be15dfa88fb1 broke GlobalISel's usage of getSetCCInverse() which currently
appears to be limited to our out-of-tree backend. GlobalISel doesn't use
EVT's and isn't able to derive them from the information it has as it
doesn't distinguish between integer and floating point types (that
distinction is made by operations rather than values). Bring back the
bool version of getSetCCInverse() in a way that doesn't break the intent
of be15dfa88fb1 but also allows GlobalISel to continue using it.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Could we invert the boolean flag to be isFloat? I fear that calling it isInteger, will lead the the same problems that I tried to fix in that commit (calling .isInteger() returns false for pointers).
GlobalISel's LLT type doesn't have an isInteger() and distinguishes scalars from pointers so I think that's less of a risk for GlobalISel. For SelectionDAG, my guard against misuse is moving it to the GlobalISel namespace so that SelectionDAG code can't call it by accident.
That said, I don't mind inverting it in principle but one risk I see with that is that clangs fixits will direct downstream people to add the namespace but won't point out that the bool is inverted. We could name it isIntegral, isIntegerLike, isIntegerComparison, etc. if that helps.
Could we an overload that takes LLT instead and keep the boolean flag internal to the .cpp?
Never mind, I saw now that LLT doesn't store int/float information. Seems fine to me but I either would either use isFloat or adjust the name of the boolean to isIntegerLike. Ideally also add a comment that pointers should use the integer variant.