[llvm] abe4677 - [InstCombine] Fix infinite loop due to incorrect `DoesConsume` (#82973)

[via llvm-commits]

Author: Yingwei Zheng
Date: 2024-03-05T03:51:06+08:00
New Revision: abe4677d9f8ab82f722f29dbf57952052accd70b

URL: https://github.com/llvm/llvm-project/commit/abe4677d9f8ab82f722f29dbf57952052accd70b
DIFF: https://github.com/llvm/llvm-project/commit/abe4677d9f8ab82f722f29dbf57952052accd70b.diff

LOG: [InstCombine] Fix infinite loop due to incorrect `DoesConsume` (#82973)

When a call to `getFreelyInvertedImpl` with a select/phi node fails,
`DoesConsume` should not be changed.

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

Added: 
    llvm/test/Transforms/InstCombine/pr82877.ll

Modified: 
    llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 87c8dca7efed89..80ce0c9275b2cb 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2341,11 +2341,13 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
                   !shouldAvoidAbsorbingNotIntoSelect(*cast<SelectInst>(V));
   // Selects/min/max with invertible operands are freely invertible
   if (IsSelect || match(V, m_MaxOrMin(m_Value(A), m_Value(B)))) {
+    bool LocalDoesConsume = DoesConsume;
     if (!getFreelyInvertedImpl(B, B->hasOneUse(), /*Builder*/ nullptr,
-                               DoesConsume, Depth))
+                               LocalDoesConsume, Depth))
       return nullptr;
     if (Value *NotA = getFreelyInvertedImpl(A, A->hasOneUse(), Builder,
-                                            DoesConsume, Depth)) {
+                                            LocalDoesConsume, Depth)) {
+      DoesConsume = LocalDoesConsume;
       if (Builder != nullptr) {
         Value *NotB = getFreelyInvertedImpl(B, B->hasOneUse(), Builder,
                                             DoesConsume, Depth);
@@ -2361,12 +2363,13 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
   }
 
   if (PHINode *PN = dyn_cast<PHINode>(V)) {
+    bool LocalDoesConsume = DoesConsume;
     SmallVector<std::pair<Value *, BasicBlock *>, 8> IncomingValues;
     for (Use &U : PN->operands()) {
       BasicBlock *IncomingBlock = PN->getIncomingBlock(U);
       Value *NewIncomingVal = getFreelyInvertedImpl(
           U.get(), /*WillInvertAllUses=*/false,
-          /*Builder=*/nullptr, DoesConsume, MaxAnalysisRecursionDepth - 1);
+          /*Builder=*/nullptr, LocalDoesConsume, MaxAnalysisRecursionDepth - 1);
       if (NewIncomingVal == nullptr)
         return nullptr;
       // Make sure that we can safely erase the original PHI node.
@@ -2375,6 +2378,8 @@ Value *InstCombiner::getFreelyInvertedImpl(Value *V, bool WillInvertAllUses,
       if (Builder != nullptr)
         IncomingValues.emplace_back(NewIncomingVal, IncomingBlock);
     }
+
+    DoesConsume = LocalDoesConsume;
     if (Builder != nullptr) {
       IRBuilderBase::InsertPointGuard Guard(*Builder);
       Builder->SetInsertPoint(PN);

diff  --git a/llvm/test/Transforms/InstCombine/pr82877.ll b/llvm/test/Transforms/InstCombine/pr82877.ll
new file mode 100644
index 00000000000000..8594bb68b8e4cc
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/pr82877.ll
@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -S -passes=instcombine < %s | FileCheck %s
+
+define i64 @func(i32 %p, i1 %cmp1) {
+; CHECK-LABEL: define i64 @func(
+; CHECK-SAME: i32 [[P:%.*]], i1 [[CMP1:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[NOT:%.*]] = xor i32 [[P]], -1
+; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
+; CHECK:       for.body:
+; CHECK-NEXT:    [[P0:%.*]] = phi i32 [ [[NOT]], [[ENTRY:%.*]] ], [ [[CONV:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[COND:%.*]] = select i1 [[CMP1]], i32 0, i32 -1231558963
+; CHECK-NEXT:    [[XOR:%.*]] = xor i32 [[COND]], [[P0]]
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp ne i32 [[XOR]], 1
+; CHECK-NEXT:    [[CONV]] = zext i1 [[CMP2]] to i32
+; CHECK-NEXT:    br i1 [[CMP2]], label [[FOR_BODY]], label [[FOR_EXIT:%.*]]
+; CHECK:       for.exit:
+; CHECK-NEXT:    ret i64 0
+;
+entry:
+  %not = xor i32 %p, -1
+  br label %for.body
+
+for.body:
+  %p0 = phi i32 [ %not, %entry ], [ %conv, %for.body ]
+  %cond = select i1 %cmp1, i32 0, i32 -1231558963
+  %xor = xor i32 %cond, %p0
+  %cmp2 = icmp ne i32 %xor, 1
+  %conv = zext i1 %cmp2 to i32
+  br i1 %cmp2, label %for.body, label %for.exit
+
+for.exit:
+  ret i64 0
+}