Sink: Handle instruction sink when a user is dead

[cfang]

Summary

The current instruction sink pass uses findNearestCommonDominator of all users to find block to sink the instruction to.
However, a user may be in a dead block, which will result in unexpected behavior.

This patch handles such cases by skipping dead blocks. In addition, if findNearestCommonDominator fails somehow,
we bail out instruction sinking.

[MaskRay]

llvm/lib/Transforms/Scalar/Sink.cpp

150

After D88758, the return value cannot be nullptr.

[cfang]

remove checking for findNearestCommonDominator failures.

[cfang]

llvm/lib/Transforms/Scalar/Sink.cpp

150

Done. Remove the checking.

[MaskRay]

'unreachable' may be more appropriate than 'dead'.

Please mention that this fixes PR47415 ( https://bugs.llvm.org/show_bug.cgi?id=47415)

In addition, if findNearestCommonDominator fails somehow, we bail out instruction sinking.

This sentence should be deleted.

llvm/lib/Transforms/Scalar/Sink.cpp

static bool SinkInstruction(Instruction *Inst,
   // decide.
   BasicBlock *SuccToSinkTo = nullptr;
 
   // Find the nearest common dominator of all users as the candidate.
   BasicBlock *BB = Inst->getParent();
   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);
     }
     if (SuccToSinkTo)
       SuccToSinkTo = DT.findNearestCommonDominator(SuccToSinkTo, UseBlock);
     else
       SuccToSinkTo = UseBlock;
     // The current basic block needs to dominate the candidate.
     if (!DT.dominates(BB, SuccToSinkTo))
       return false;
   }
 
   if (SuccToSinkTo) {
     // The nearest common dominator may be in a parent loop of BB, which may not
     // be beneficial. Find an ancestor.

llvm/test/Transforms/Sink/dead-user.ll

+; Compiler should not be broken with a dead user.
+; RUN: opt -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) {
+bb.0:
+  %conv = sext i16 %p1 to i32
+  br i1 undef, label %bb.1, label %bb.3
+
+bb.1:                                             ; preds = %bb.0
+  br label %bb.2
+
+bb.2:                                             ; preds = %bb.2, %bb.1
+  %and.2 = and i32 undef, %conv
+  br label %bb.2
+
+bb.3:                                             ; preds = %bb.3, %bb.0
+  %and.3 = and i32 undef, %conv
+  br label %bb.3
+
+dead:                                             ; preds = %dead
+  %and.dead = and i32 undef, %conv
+  br label %dead
+}