[InstCombine] foldOpIntoPhi should apply to icmp with non-constant operand

[rkirsling]

Alive2: https://alive2.llvm.org/ce/z/4MeCzA
Fixes #146263.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Ross Kirsling (rkirsling)

Changes

Alive2: https://alive2.llvm.org/ce/z/4MeCzA
Fixes #146263.

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+4-4)
• (added) llvm/test/Transforms/InstCombine/fold-icmp-without-constant-into-phi.ll (+98)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index c6f317a668cfe..e753af73886f3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -4305,10 +4305,6 @@ Instruction *InstCombinerImpl::foldICmpInstWithConstantNotInt(ICmpInst &I) {
     return nullptr;
 
   switch (LHSI->getOpcode()) {
-  case Instruction::PHI:
-    if (Instruction *NV = foldOpIntoPhi(I, cast<PHINode>(LHSI)))
-      return NV;
-    break;
   case Instruction::IntToPtr:
     // icmp pred inttoptr(X), null -> icmp pred X, 0
     if (RHSC->isNullValue() &&
@@ -7699,6 +7695,10 @@ Instruction *InstCombinerImpl::visitICmpInst(ICmpInst &I) {
   if (Instruction *New = foldSignBitTest(I))
     return New;
 
+  if (auto *PN = dyn_cast<PHINode>(Op0))
+    if (Instruction *NV = foldOpIntoPhi(I, PN))
+      return NV;
+
   if (Instruction *Res = foldICmpInstWithConstantNotInt(I))
     return Res;
 
diff --git a/llvm/test/Transforms/InstCombine/fold-icmp-without-constant-into-phi.ll b/llvm/test/Transforms/InstCombine/fold-icmp-without-constant-into-phi.ll
new file mode 100644
index 0000000000000..7c0c1a39d00f7
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/fold-icmp-without-constant-into-phi.ll
@@ -0,0 +1,98 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+
+define i1 @test(i1 %cond, i64 %left, i64 %right) {
+; CHECK-LABEL: define i1 @test(
+; CHECK-SAME: i1 [[COND:%.*]], i64 [[LEFT:%.*]], i64 [[RIGHT:%.*]]) {
+; CHECK-NEXT:  [[START:.*:]]
+; CHECK-NEXT:    br i1 [[COND]], label %[[COND_TRUE:.*]], label %[[COND_FALSE:.*]]
+; CHECK:       [[COND_TRUE]]:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i64 [[RIGHT]], [[LEFT]]
+; CHECK-NEXT:    br i1 [[CMP]], label %[[END:.*]], label %[[COND_FALSE]]
+; CHECK:       [[COND_FALSE]]:
+; CHECK-NEXT:    br label %[[END]]
+; CHECK:       [[END]]:
+; CHECK-NEXT:    ret i1 false
+;
+start:
+  br i1 %cond, label %cond.true, label %cond.false
+
+cond.true:
+  %cmp = icmp sgt i64 %right, %left
+  br i1 %cmp, label %end, label %cond.false
+
+cond.false:
+  %left_or_right = phi i64 [ %left, %start ], [ %right, %cond.true ]
+  %false = icmp sgt i64 %left_or_right, %left
+  br label %end
+
+end:
+  %result = phi i1 [ false, %cond.true ], [ %false, %cond.false ]
+  ret i1 %result
+}
+
+define i1 @test_with_wrong_icmp(i1 %cond, i64 %left, i64 %right) {
+; CHECK-LABEL: define i1 @test_with_wrong_icmp(
+; CHECK-SAME: i1 [[COND:%.*]], i64 [[LEFT:%.*]], i64 [[RIGHT:%.*]]) {
+; CHECK-NEXT:  [[START:.*]]:
+; CHECK-NEXT:    br i1 [[COND]], label %[[COND_TRUE:.*]], label %[[COND_FALSE:.*]]
+; CHECK:       [[COND_TRUE]]:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i64 [[RIGHT]], [[LEFT]]
+; CHECK-NEXT:    br i1 [[CMP]], label %[[END:.*]], label %[[COND_FALSE]]
+; CHECK:       [[COND_FALSE]]:
+; CHECK-NEXT:    [[LEFT_OR_RIGHT:%.*]] = phi i64 [ [[LEFT]], %[[START]] ], [ [[RIGHT]], %[[COND_TRUE]] ]
+; CHECK-NEXT:    [[FALSE:%.*]] = icmp sge i64 [[LEFT_OR_RIGHT]], [[LEFT]]
+; CHECK-NEXT:    br label %[[END]]
+; CHECK:       [[END]]:
+; CHECK-NEXT:    [[RESULT:%.*]] = phi i1 [ false, %[[COND_TRUE]] ], [ [[FALSE]], %[[COND_FALSE]] ]
+; CHECK-NEXT:    ret i1 [[RESULT]]
+;
+start:
+  br i1 %cond, label %cond.true, label %cond.false
+
+cond.true:
+  %cmp = icmp sgt i64 %right, %left
+  br i1 %cmp, label %end, label %cond.false
+
+cond.false:
+  %left_or_right = phi i64 [ %left, %start ], [ %right, %cond.true ]
+  %false = icmp sge i64 %left_or_right, %left
+  br label %end
+
+end:
+  %result = phi i1 [ false, %cond.true ], [ %false, %cond.false ]
+  ret i1 %result
+}
+
+define i1 @test_with_unfoldable_phi(i1 %cond, i64 %left, i64 %right) {
+; CHECK-LABEL: define i1 @test_with_unfoldable_phi(
+; CHECK-SAME: i1 [[COND:%.*]], i64 [[LEFT:%.*]], i64 [[RIGHT:%.*]]) {
+; CHECK-NEXT:  [[START:.*]]:
+; CHECK-NEXT:    br i1 [[COND]], label %[[COND_TRUE:.*]], label %[[COND_FALSE:.*]]
+; CHECK:       [[COND_TRUE]]:
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i64 [[RIGHT]], [[LEFT]]
+; CHECK-NEXT:    br i1 [[CMP]], label %[[END:.*]], label %[[COND_FALSE]]
+; CHECK:       [[COND_FALSE]]:
+; CHECK-NEXT:    [[LEFT_OR_RIGHT:%.*]] = phi i64 [ [[LEFT]], %[[START]] ], [ [[RIGHT]], %[[COND_TRUE]] ]
+; CHECK-NEXT:    [[FALSE:%.*]] = icmp sgt i64 [[LEFT_OR_RIGHT]], [[LEFT]]
+; CHECK-NEXT:    br label %[[END]]
+; CHECK:       [[END]]:
+; CHECK-NEXT:    [[RESULT:%.*]] = phi i1 [ false, %[[COND_TRUE]] ], [ [[FALSE]], %[[COND_FALSE]] ]
+; CHECK-NEXT:    ret i1 [[RESULT]]
+;
+start:
+  br i1 %cond, label %cond.true, label %cond.false
+
+cond.true:
+  %cmp = icmp slt i64 %right, %left         ; slt instead of sgt here
+  br i1 %cmp, label %end, label %cond.false
+
+cond.false:
+  %left_or_right = phi i64 [ %left, %start ], [ %right, %cond.true ]
+  %false = icmp sgt i64 %left_or_right, %left
+  br label %end
+
+end:
+  %result = phi i1 [ false, %cond.true ], [ %false, %cond.false ]
+  ret i1 %result
+}

[nikic]

Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=2d424e633349fc0e22a1242978989c82aa4895d4&to=994805f5827e4c577aed4852e3a126a09f846fef&stat=instructions:u Mostly neutral, but has a large impact on sqlite3.