[InstCombine] fold (icmp eq/ne (or disjoint x, C0), C1) -> (icmp eq/ne x, C0^C1)

[goldsteinn]

• [InstCombine] Add tests for folding (icmp eq/ne (or disjoint x, C0), C1); NFC
• [InstCombine] fold (icmp eq/ne (or disjoint x, C0), C1) -> (icmp eq/ne x, C0^C1)

Proof: https://alive2.llvm.org/ce/z/m3xoo_

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes

• [InstCombine] Add tests for folding (icmp eq/ne (or disjoint x, C0), C1); NFC
• [InstCombine] fold (icmp eq/ne (or disjoint x, C0), C1) -> (icmp eq/ne x, C0^C1)

Proof: https://alive2.llvm.org/ce/z/m3xoo_

---

Full diff: https://github.com/llvm/llvm-project/pull/87734.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+11)
• (modified) llvm/test/Transforms/InstCombine/icmp-or.ll (+46)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index db302d7e526844..03a6e7a555c3f7 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/PatternMatch.h"
 #include "llvm/Support/KnownBits.h"
@@ -2049,6 +2050,16 @@ Instruction *InstCombinerImpl::foldICmpOrConstant(ICmpInst &Cmp,
   }
 
   Value *OrOp0 = Or->getOperand(0), *OrOp1 = Or->getOperand(1);
+
+  // (icmp eq/ne (or disjoint x, C0), C1)
+  //    -> (icmp eq/ne x, C0^C1)
+  if (Cmp.isEquality() && match(OrOp1, m_ImmConstant()) &&
+      cast<PossiblyDisjointInst>(Or)->isDisjoint()) {
+    Constant *NewC = ConstantExpr::getXor(
+        cast<Constant>(OrOp1), ConstantInt::get(OrOp1->getType(), C));
+    return new ICmpInst(Pred, OrOp0, NewC);
+  }
+
   const APInt *MaskC;
   if (match(OrOp1, m_APInt(MaskC)) && Cmp.isEquality()) {
     if (*MaskC == C && (C + 1).isPowerOf2()) {
diff --git a/llvm/test/Transforms/InstCombine/icmp-or.ll b/llvm/test/Transforms/InstCombine/icmp-or.ll
index 922845c1e7e2d8..65d90104cfeeee 100644
--- a/llvm/test/Transforms/InstCombine/icmp-or.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-or.ll
@@ -951,3 +951,49 @@ define i1 @icmp_or_xor_with_sub_3_6(i64 %x1, i64 %y1, i64 %x2, i64 %y2, i64 %x3,
   %cmp = icmp eq i64 %or1, 0
   ret i1 %cmp
 }
+
+
+define i1 @or_disjoint_with_constants(i8 %x) {
+; CHECK-LABEL: @or_disjoint_with_constants(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i8 [[TMP1:%.*]], 18
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %or = or disjoint i8 %x, 1
+  %cmp = icmp eq i8 %or, 19
+  ret i1 %cmp
+}
+
+
+define i1 @or_disjoint_with_constants2(i8 %x) {
+; CHECK-LABEL: @or_disjoint_with_constants2(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i8 [[TMP1:%.*]], 66
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %or = or disjoint i8 %x, 5
+  %cmp = icmp ne i8 %or, 71
+  ret i1 %cmp
+}
+
+
+define i1 @or_disjoint_with_constants_fail_missing_const1(i8 %x, i8 %y) {
+; CHECK-LABEL: @or_disjoint_with_constants_fail_missing_const1(
+; CHECK-NEXT:    [[OR:%.*]] = or disjoint i8 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i8 [[OR]], 19
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %or = or disjoint i8 %x, %y
+  %cmp = icmp eq i8 %or, 19
+  ret i1 %cmp
+}
+
+define i1 @or_disjoint_with_constants_fail_missing_const2(i8 %x, i8 %y) {
+; CHECK-LABEL: @or_disjoint_with_constants_fail_missing_const2(
+; CHECK-NEXT:    [[OR:%.*]] = or disjoint i8 [[X:%.*]], 19
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i8 [[OR]], [[Y:%.*]]
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %or = or disjoint i8 %x, 19
+  %cmp = icmp eq i8 %or, %y
+  ret i1 %cmp
+}
+

[dtcxzyw]

I am not sure whether we should put the logic in InstCombinerImpl::foldICmpBinOpEqualityWithConstant or not.

[goldsteinn]

We have some other equality folds here, I think we should move them all when/if we move them.

[goldsteinn]

We can't move w.o a non-trivial refactor. Otherwise we hit the X | C == C1 -> X & ~C == C1 ^ C0 fold first.

[AtariDreams]

I don't know if this is equivalent to

and tmp, -2

this clears the last bit only

which means 18 and 19 will both result in icmp eq i8 being true.

But when the and is eliminated, that doesn't work

[goldsteinn]

The previous fold covered more cases than where necessary. the disjoint implies that x != 19 (otherwise x | 1 wouldn't be disjoint).

[AtariDreams]

Given how the test cases show variables and not just constants, can this fold apply to variables too?

[XChy]

If we phrase (or disjoint x, C0) == C1 as x + C0 == C1, this fold is just like x + C0 == C1 -> X == C1 - C0: https://alive2.llvm.org/ce/z/aRk27g
Or we should put it in

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

Line 3441 in c91254d

case Instruction::Add: {

, so that we could handle it easier?

[goldsteinn]

Given how the test cases show variables and not just constants, can this fold apply to variables too?

I don't think the fold is profitable if we can't constant fold the xor.

[goldsteinn]

If we phrase (or disjoint x, C0) == C1 as x + C0 == C1, this fold is just like x + C0 == C1 -> X == C1 - C0: https://alive2.llvm.org/ce/z/aRk27g Or we should put it in

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp

Line 3441 in c91254d

case Instruction::Add: {

, so that we could handle it easier?

I don't think so for two reasons.

1. This only count's for disjoint or so we need special logic for fallthrough which I'm not sure would be simpler to implement.
2. the add case avoid multi-use. This is pretty specific to add b.c folding for multi-use messes up some loop optimizations. We don't want/need the same restriction for disjoint or.

[XChy]

LGTM

[nikic]

LGTM

[nikic]

Use IRBuilder for this please, to save me the trouble of replacing it later.