[llvm] Fix Issue 65107 (PR #78444)

via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 17 05:31:59 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: hanbeom (ParkHanbum)

<details>
<summary>Changes</summary>

After we move instruction to an unreachable basic block, we need to
handle it appropriately. if not,
`removeAllNonTerminatorAndEHPadInstructions` is called within
`prepareWorklist` preparing for the second combining process, and
instructions in the unreachable block are processed.

in this case, we are considered to have modified instruction, so
message `LLVM ERROR: Instruction Combining did not reach a fixpoint
after 1 iterations` is showing and exits.

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


3 Files Affected:

- (modified) llvm/lib/Transforms/InstCombine/InstCombineInternal.h (+1-1) 
- (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+22-3) 
- (modified) llvm/test/Transforms/InstCombine/sink_to_unreachable.ll (+72) 


``````````diff
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index 21c61bd990184d..5a1e1726946510 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -736,7 +736,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
   bool removeInstructionsBeforeUnreachable(Instruction &I);
   void addDeadEdge(BasicBlock *From, BasicBlock *To,
                    SmallVectorImpl<BasicBlock *> &Worklist);
-  void handleUnreachableFrom(Instruction *I,
+  bool handleUnreachableFrom(Instruction *I,
                              SmallVectorImpl<BasicBlock *> &Worklist);
   void handlePotentiallyDeadBlocks(SmallVectorImpl<BasicBlock *> &Worklist);
   void handlePotentiallyDeadSuccessors(BasicBlock *BB, BasicBlock *LiveSucc);
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 7f2018b3a19958..f4411e52376ff3 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3082,22 +3082,23 @@ void InstCombinerImpl::addDeadEdge(BasicBlock *From, BasicBlock *To,
 
 // Under the assumption that I is unreachable, remove it and following
 // instructions. Changes are reported directly to MadeIRChange.
-void InstCombinerImpl::handleUnreachableFrom(
+bool InstCombinerImpl::handleUnreachableFrom(
     Instruction *I, SmallVectorImpl<BasicBlock *> &Worklist) {
+  bool Changed = false;
   BasicBlock *BB = I->getParent();
   for (Instruction &Inst : make_early_inc_range(
            make_range(std::next(BB->getTerminator()->getReverseIterator()),
                       std::next(I->getReverseIterator())))) {
     if (!Inst.use_empty() && !Inst.getType()->isTokenTy()) {
       replaceInstUsesWith(Inst, PoisonValue::get(Inst.getType()));
-      MadeIRChange = true;
+      Changed = true;
     }
     if (Inst.isEHPad() || Inst.getType()->isTokenTy())
       continue;
     // RemoveDIs: erase debug-info on this instruction manually.
     Inst.dropDbgValues();
     eraseInstFromFunction(Inst);
-    MadeIRChange = true;
+    Changed  = true;
   }
 
   // RemoveDIs: to match behaviour in dbg.value mode, drop debug-info on
@@ -3107,6 +3108,8 @@ void InstCombinerImpl::handleUnreachableFrom(
   // Handle potentially dead successors.
   for (BasicBlock *Succ : successors(BB))
     addDeadEdge(BB, Succ, Worklist);
+
+  return MadeIRChange = (Changed | MadeIRChange);
 }
 
 void InstCombinerImpl::handlePotentiallyDeadBlocks(
@@ -4396,6 +4399,22 @@ bool InstCombinerImpl::run() {
         for (Use &U : I->operands())
           if (Instruction *OpI = dyn_cast<Instruction>(U.get()))
             Worklist.push(OpI);
+
+        // When we sink instruction, we must consider the case
+        // where the baisc block to which the instruction is moved
+        // is unreachable. Otherwise, a problem occurs where the
+        // sinked instruction removes the instruction from the
+        // second loop.
+        if (all_of(predecessors(UserParent), [&](BasicBlock *Pred) {
+              return DeadEdges.contains({Pred, UserParent}) ||
+                     DT.dominates(UserParent, Pred);
+            })) {
+          SmallVector<BasicBlock *> DeadBlockCandidates;
+          if (handleUnreachableFrom(I, DeadBlockCandidates)) {
+            handlePotentiallyDeadBlocks(DeadBlockCandidates);
+            continue;
+          }
+        }
       }
     }
 
diff --git a/llvm/test/Transforms/InstCombine/sink_to_unreachable.ll b/llvm/test/Transforms/InstCombine/sink_to_unreachable.ll
index e788b634da8868..193b0d5a704093 100644
--- a/llvm/test/Transforms/InstCombine/sink_to_unreachable.ll
+++ b/llvm/test/Transforms/InstCombine/sink_to_unreachable.ll
@@ -157,3 +157,75 @@ bb3:
   %p = phi i32 [0, %bb1], [%a, %bb2]
   ret i32 %p
 }
+
+define i1 @src_sink_and_remove_bb(i16 %0) {
+; CHECK-LABEL: @src_sink_and_remove_bb(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[BB1:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br i1 true, label [[BB1]], label [[UNREACHABLE:%.*]]
+; CHECK:       unreachable:
+; CHECK-NEXT:    ret i1 poison
+;
+entry:
+  br label %bb1
+bb1:
+  %unknown = icmp ult i16 255, %0
+  br i1 true, label %bb1, label %unreachable
+unreachable:
+  ret i1 %unknown
+}
+
+define i1 @src_sink_and_remove_entry(i16 %0) {
+; CHECK-LABEL: @src_sink_and_remove_entry(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[BB1:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br i1 true, label [[BB1]], label [[UNREACHABLE:%.*]]
+; CHECK:       unreachable:
+; CHECK-NEXT:    ret i1 poison
+;
+entry:
+  %unknown = icmp ult i16 255, %0
+  br label %bb1
+bb1:
+  br i1 true, label %bb1, label %unreachable
+unreachable:
+  ret i1 %unknown
+}
+
+define i1 @src_sink_and_remove(i8 %0) {
+; CHECK-LABEL: @src_sink_and_remove(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[BB1:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br i1 false, label [[UNREACHABLE:%.*]], label [[BB1]]
+; CHECK:       unreachable:
+; CHECK-NEXT:    ret i1 poison
+;
+entry:
+  %unknown = icmp ult i8 255, %0
+  br label %bb1
+bb1:
+  br i1 false, label %unreachable, label %bb1
+unreachable:
+  ret i1 %unknown
+}
+
+define i1 @src_sink_and_combine(i8 %0) {
+; CHECK-LABEL: @src_sink_and_combine(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[BB1:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    br i1 false, label [[BB1]], label [[UNREACHABLE:%.*]]
+; CHECK:       unreachable:
+; CHECK-NEXT:    ret i1 false
+;
+entry:
+  br label %bb1
+bb1:
+  %unknown = icmp ult i8 255, %0
+  br i1 false, label %bb1, label %unreachable
+unreachable:
+  ret i1 %unknown
+}

``````````

</details>


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


More information about the llvm-commits mailing list