[llvm] Fix Issue 65107 (PR #78444)

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


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

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.

>From 5ca2246acea87d4946d1e6bac35a50a5c3261830 Mon Sep 17 00:00:00 2001
From: Hanbum Park <kese111 at gmail.com>
Date: Wed, 17 Jan 2024 21:18:43 +0900
Subject: [PATCH 1/2] [InstCombine] Tests instruction sink to unreachable block

This patch add tests that instruction sink to unreachable block has
work as well. some tests will fail and showing LLVM ERROR message.
---
 .../InstCombine/sink_to_unreachable.ll        | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/llvm/test/Transforms/InstCombine/sink_to_unreachable.ll b/llvm/test/Transforms/InstCombine/sink_to_unreachable.ll
index e788b634da8868b..193b0d5a7040930 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
+}

>From bb85732a2f025d18452637159779bc483470febe Mon Sep 17 00:00:00 2001
From: Hanbum Park <kese111 at gmail.com>
Date: Wed, 17 Jan 2024 21:19:35 +0900
Subject: [PATCH 2/2] [InstCombine] Handle instruction sink to unreachable
 block (#65107)

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.
---
 .../InstCombine/InstCombineInternal.h         |  2 +-
 .../InstCombine/InstructionCombining.cpp      | 25 ++++++++++++++++---
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index 21c61bd990184d5..5a1e17269465101 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 7f2018b3a19958f..f4411e52376ff37 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;
+          }
+        }
       }
     }
 



More information about the llvm-commits mailing list