Match icmp eq (trunc (lsr A, BW), (ashr (trunc A), BW-1)), which checks the top BW/2 + 1 bits are all the same. Create "A >=s INT_MIN && A <=s INT_MAX", which we generate as "icmp ult (add A, 2^BW-1), 2^BW" to skip a few steps of instcombining.
https://alive2.llvm.org/ce/z/NjH6Ty
https://alive2.llvm.org/ce/z/_fEQ9P
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | ||
---|---|---|
4596 | We don't need the commutative matcher (and the regression test shows this). That's because we always canonicalize the 'ashr' as op0 and 'trunc' as op1 based on getComplexity. Actually, we don't need to rematch the icmp at all if we handle both predicates - see next comment. | |
4600 | We should handle ICMP_NE for symmetry. Flip from ULT to UGE? |
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp | ||
---|---|---|
4596 | Oh that's good to hear. Yeah the only reason for starting the match at the ICmp was to get commutativity through the m_c_ICmp. |
"A >s INT_MIN && A <s INT_MAX"
>=s and <=s, in the commit message and the inline comment?
The half-width truncate clause seems too restrictive. I assume we want to catch a case like this too:
https://alive2.llvm.org/ce/z/UAQ4yQ
Yes, that was all wrong. If we're not doing a half-width truncate, the bits aren't lined up consecutively, so it probably doesn't matter:
https://alive2.llvm.org/ce/z/YdoXeg
Yeah, that was what I thought about the now-half-width-cases. The maths doesn't work at as well, and they at least feel like a different case to me.
I agree that the double-the-bitwidth precondition is needed *for this pattern*, but there are other possible cases:
https://alive2.llvm.org/ce/z/RjUMzM (double)
https://alive2.llvm.org/ce/z/Jd3aHC (more than double => sext)
https://alive2.llvm.org/ce/z/CS2xkv (less than double => trunc)
lsr -> lshr