[llvm] [InstCombine] Fix infinite combine loop in evaluateInDifferentType (PR #202572)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

<details>
<summary>Changes</summary>

The implementation assumes that all original uses inside visited instructions would get removed as part of changing the type. However, this is not true for uses in select conditions, as only the value operands change type in that case. Bail out if we encounter uses in select conditions to avoid this.

Fixes https://github.com/llvm/llvm-project/issues/173148.

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


2 Files Affected:

- (modified) llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp (+11-2) 
- (added) llvm/test/Transforms/InstCombine/issue173148-sext-phi-select-infloop.ll (+26) 


``````````diff
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
index ca42263f47fa6..f1907fdb044a1 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -412,8 +412,17 @@ class TypeEvaluationHelper {
     // done and and ready to have a positive verdict, we should double-check all
     // of the pending users and ensure that we visited them. allPendingVisited
     // predicate checks exactly that.
-    if (!I->hasOneUse())
-      llvm::append_range(Pending, I->users());
+    if (!I->hasOneUse()) {
+      for (Use &U : I->uses()) {
+        // For most instruction, evaluating them in a different type will change
+        // the type of all operands. This is not the case for select conditions.
+        // Make sure we don't retain an extra use via the select condition.
+        if (isa<SelectInst>(U.getUser()) && U.getOperandNo() == 0)
+          return false;
+
+        Pending.push_back(U.getUser());
+      }
+    }
 
     const bool Result = Pred(V, Ty);
     // We have to set result this way and not via It because Pred is recursive
diff --git a/llvm/test/Transforms/InstCombine/issue173148-sext-phi-select-infloop.ll b/llvm/test/Transforms/InstCombine/issue173148-sext-phi-select-infloop.ll
new file mode 100644
index 0000000000000..43b6d42c951aa
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/issue173148-sext-phi-select-infloop.ll
@@ -0,0 +1,26 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+; Test for https://github.com/llvm/llvm-project/issues/173148.
+; Make sure this does not result in an infinite combine loop.
+
+target datalayout = "n8:16:32:64"
+
+define i8 @test() {
+; CHECK-LABEL: define i8 @test() {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    br label %[[LOOP:.*]]
+; CHECK:       [[LOOP]]:
+; CHECK-NEXT:    br label %[[LOOP]]
+;
+entry:
+  br label %loop
+
+loop:
+  %phi = phi i16 [ %select, %loop ], [ 0, %entry ]
+  %phi2 = phi i1 [ %select3, %loop ], [ false, %entry ]
+  %add = add i16 %phi, -1
+  %select = select i1 %phi2, i16 %phi, i16 %add
+  %select3 = select i1 %phi2, i1 false, i1 true
+  br label %loop
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/202572