[llvm] 2a2b426 - [Sink] Fix bugs of sinking unreachable BB from phi (#68576)

[via llvm-commits]

Author: XChy
Date: 2023-10-09T22:34:44+08:00
New Revision: 2a2b426f13dfd33c7495da1c54ab9d1a8e625d87

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

LOG: [Sink] Fix bugs of sinking unreachable BB from phi (#68576)

Resolve #68529.
Sink pass doesn't consider whether Basicblock from phi is unreachable.
This patch moves unreachability check after checking phi.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/Sink.cpp
    llvm/test/Transforms/Sink/dead-user.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/Sink.cpp b/llvm/lib/Transforms/Scalar/Sink.cpp
index ed768deacd069b9..46bcfd6b41ce4f4 100644
--- a/llvm/lib/Transforms/Scalar/Sink.cpp
+++ b/llvm/lib/Transforms/Scalar/Sink.cpp
@@ -130,15 +130,16 @@ static bool SinkInstruction(Instruction *Inst,
   for (Use &U : Inst->uses()) {
     Instruction *UseInst = cast<Instruction>(U.getUser());
     BasicBlock *UseBlock = UseInst->getParent();
-    // Don't worry about dead users.
-    if (!DT.isReachableFromEntry(UseBlock))
-      continue;
     if (PHINode *PN = dyn_cast<PHINode>(UseInst)) {
       // PHI nodes use the operand in the predecessor block, not the block with
       // the PHI.
       unsigned Num = PHINode::getIncomingValueNumForOperand(U.getOperandNo());
       UseBlock = PN->getIncomingBlock(Num);
     }
+    // Don't worry about dead users.
+    if (!DT.isReachableFromEntry(UseBlock))
+      continue;
+
     if (SuccToSinkTo)
       SuccToSinkTo = DT.findNearestCommonDominator(SuccToSinkTo, UseBlock);
     else

diff  --git a/llvm/test/Transforms/Sink/dead-user.ll b/llvm/test/Transforms/Sink/dead-user.ll
index 9478e29cc12ef98..91e61b43ca3912d 100644
--- a/llvm/test/Transforms/Sink/dead-user.ll
+++ b/llvm/test/Transforms/Sink/dead-user.ll
@@ -1,26 +1,25 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
 ; Compiler should not be broken with a dead user.
 ; RUN: opt -passes=sink -S < %s | FileCheck %s
 
-; CHECK-LABEL: @test(
-; CHECK:       bb.0:
-; CHECK-NEXT:    %conv = sext i16 %p1 to i32
-; CHECK-NEXT:    br i1 undef, label %bb.1, label %bb.3
-
-; CHECK:       bb.1:                                             ; preds = %bb.0
-; CHECK-NEXT:    br label %bb.2
-
-; CHECK:       bb.2:                                             ; preds = %bb.2, %bb.1
-; CHECK-NEXT:    %and.2 = and i32 undef, %conv
-; CHECK-NEXT:    br label %bb.2
-
-; CHECK:       bb.3:                                             ; preds = %bb.3, %bb.0
-; CHECK-NEXT:    %and.3 = and i32 undef, %conv
-; CHECK-NEXT:    br label %bb.3
-
-; CHECK:       dead:                                             ; preds = %dead
-; CHECK-NEXT:    %and.dead = and i32 undef, %conv
-; CHECK-NEXT:    br label %dead
 define void @test(i16 %p1) {
+; CHECK-LABEL: define void @test(
+; CHECK-SAME: i16 [[P1:%.*]]) {
+; CHECK-NEXT:  bb.0:
+; CHECK-NEXT:    [[CONV:%.*]] = sext i16 [[P1]] to i32
+; CHECK-NEXT:    br i1 undef, label [[BB_1:%.*]], label [[BB_3:%.*]]
+; CHECK:       bb.1:
+; CHECK-NEXT:    br label [[BB_2:%.*]]
+; CHECK:       bb.2:
+; CHECK-NEXT:    [[AND_2:%.*]] = and i32 undef, [[CONV]]
+; CHECK-NEXT:    br label [[BB_2]]
+; CHECK:       bb.3:
+; CHECK-NEXT:    [[AND_3:%.*]] = and i32 undef, [[CONV]]
+; CHECK-NEXT:    br label [[BB_3]]
+; CHECK:       dead:
+; CHECK-NEXT:    [[AND_DEAD:%.*]] = and i32 undef, [[CONV]]
+; CHECK-NEXT:    br label [[DEAD:%.*]]
+;
 bb.0:
   %conv = sext i16 %p1 to i32
   br i1 undef, label %bb.1, label %bb.3
@@ -40,3 +39,34 @@ dead:                                             ; preds = %dead
   %and.dead = and i32 undef, %conv
   br label %dead
 }
+
+define i32 @dead_from_phi(i32 %a) {
+; CHECK-LABEL: define i32 @dead_from_phi(
+; CHECK-SAME: i32 [[A:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[DOTNOT:%.*]] = icmp eq i32 [[A]], 0
+; CHECK-NEXT:    br i1 [[DOTNOT]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[B:%.*]] = and i32 undef, 65535
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       dead:
+; CHECK-NEXT:    br label [[IF_END]]
+; CHECK:       if.end:
+; CHECK-NEXT:    [[DOT0:%.*]] = phi i32 [ [[A]], [[ENTRY:%.*]] ], [ [[B]], [[IF_THEN]] ], [ [[B]], [[DEAD:%.*]] ]
+; CHECK-NEXT:    ret i32 [[DOT0]]
+;
+entry:
+  %.not = icmp eq i32 %a, 0
+  br i1 %.not, label %if.end, label %if.then
+
+if.then:                                                ; preds = %1
+  %b = and i32 undef, 65535
+  br label %if.end
+
+dead:                                                ; No predecessors!
+  br label %if.end
+
+if.end:                                                ; preds = %4, %if.then, %1
+  %.0 = phi i32 [ %a, %entry ], [ %b, %if.then ], [ %b, %dead ]
+  ret i32 %.0
+}