[PATCH] D63026: [InstCombine] Fold icmp eq/ne (and %x, signbit), 0 -> %x s>=/s< 0 earlier

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Yep, first fold looks good, some nits.



================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:1643-1644
 
   if (!And->hasOneUse())
     return nullptr;
 
----------------
Move after the newly added code, this restriction does not apply for the pattern in question.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:1654
+
+    // ((X & ~7) == 0) --> X < 8
+    // If X is (BinOp Y, C3), allow other rules to fold C3 with C2.
----------------
`X u< 8`


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:1654-1662
+    // ((X & ~7) == 0) --> X < 8
+    // If X is (BinOp Y, C3), allow other rules to fold C3 with C2.
+    if (!match(X, m_c_BinOp(m_Value(), m_Constant())) &&
+        (~(*C2) + 1).isPowerOf2()) {
+      Constant *NegBOC =
+          ConstantExpr::getNeg(cast<Constant>(And->getOperand(1)));
+      auto NewPred = isICMP_NE ? ICmpInst::ICMP_UGE : ICmpInst::ICMP_ULT;
----------------
lebedev.ri wrote:
> `X u< 8`
I'm sorry that i keep nagging, but this is a second, not unrelated, but separate fold.
Please put it into a separate review :)


================
Comment at: test/Transforms/InstCombine/shl-and-signbit-icmpeq-zero.ll:125
 
 ; Fold happened
 define i1 @scalar_shl_and_signbit_eq_extra_use_shl(i32 %x, i32 %y, i32 %z, i32* %p) {
----------------
It didn't though (:


================
Comment at: test/Transforms/InstCombine/shl-and-signbit-icmpeq-zero.ll:142
 
 ; Not fold
 define i1 @scalar_shl_and_signbit_eq_extra_use_and(i32 %x, i32 %y, i32 %z, i32* %p) {
----------------
Similarly, should fold.


================
Comment at: test/Transforms/InstCombine/shl-and-signbit-icmpeq-zero.ll:160
 
 ; Not fold
 define i1 @scalar_shl_and_signbit_eq_extra_use_shl_and(i32 %x, i32 %y, i32 %z, i32* %p, i32* %q) {
----------------
Here too


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63026/new/

https://reviews.llvm.org/D63026