[GlobalOpt][Flow][Quant] Fix segfault on unlowered quant.uniform types - #24824
[GlobalOpt][Flow][Quant] Fix segfault on unlowered quant.uniform types#24824FedericoBruzzone wants to merge 3 commits into
quant.uniform types#24824Conversation
|
Thanks @FedericoBruzzone, I'll give this a proper look tomorrow! Reminded me of my stale PR #22920 which was aiming to address another (narrower?) case, and ultimately I haven't found a good direction upon an apt review comment from Mahesh. |
|
Hi @AGindinson! As always, thank you so much. There's no rush 🫶 Tomorrow or later, I'll take a look at your PR to get an idea of what it's about. Sorry, but I didn't have a chance to do that before sending this (I didn't know it existed). EDIT: Unfortunately, at first glance, I don't think this PR will help #22920. This PR aims to maintain a conservative approach when working with statically unknown size types (i.e., we consider only int and float). |
|
Are CI's failures related to this PR? 🤔 |
|
@FedericoBruzzone Not sure what the Bazel issue is about, but doesn't seem to be the case on other PRs/on main. Could you maybe rebase? |
Signed-off-by: Federico Bruzzone <federico.bruzzone.i@gmail.com>
1de98cd to
0902d8f
Compare
Signed-off-by: Federico Bruzzone <federico.bruzzone.i@gmail.com>
0902d8f to
c6952c1
Compare
Signed-off-by: Federico Bruzzone <federico.bruzzone.i@gmail.com>
488f6bb to
363a085
Compare
|
@AGindinson, thanks for checking it out! |
HoistIntoGlobalsPass'sHoistableTensorTypeInterfaceunconditionally calledgetIntOrFloatBitWidth()on a tensor's element type to decide whether it is hoistable, assuming it is always a plain int/float. A!quant.uniform<...>element type is neither, sogetIntOrFloatBitWidth()crashes instead of returning a sane answer.isHoistableType/isHoistableLeafType, with this PR, check that the element type is actually an int/floatbefore computing its bit width via a new helper
hasComputableBitWidth, and conservatively treat anything else asnon-hoistable.
Additonally,
quant.uniformtypes leak into the pipeline becausequantis a transitive type dependency of TOSA, not because we have any lowering support forquant.qcast/quant.dcast/quant.scastAFAIK. TOSA's own conversion tolinalg/arithis expected to fully resolve these beforeFlow.VerifyInputLegalityPassalready enforces that for those some dialects,quantwas simply missing from the list, so IR where these ops survive pastGlobalOptimizationwould segfault later.Fixes #24814