Skip to content
This repository has been archived by the owner on Feb 5, 2019. It is now read-only.

Commit

Permalink
Apply fix from bug 36184
Browse files Browse the repository at this point in the history
Review is at https://reviews.llvm.org/D42833 and hasn't landed yet, but let's
test it out!

https://bugs.llvm.org/show_bug.cgi?id=36184
  • Loading branch information
alexcrichton committed Feb 9, 2018
1 parent 36c8f31 commit 9f81bea
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 8 deletions.
31 changes: 23 additions & 8 deletions lib/Transforms/Scalar/LICM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ static bool hoist(Instruction &I, const DominatorTree *DT, const Loop *CurLoop,
const LoopSafetyInfo *SafetyInfo,
OptimizationRemarkEmitter *ORE);
static bool sink(Instruction &I, LoopInfo *LI, DominatorTree *DT,
const Loop *CurLoop, const LoopSafetyInfo *SafetyInfo,
const Loop *CurLoop, LoopSafetyInfo *SafetyInfo,
OptimizationRemarkEmitter *ORE, bool FreeInLoop);
static bool isSafeToExecuteUnconditionally(Instruction &Inst,
const DominatorTree *DT,
Expand Down Expand Up @@ -855,10 +855,16 @@ static Instruction *sinkThroughTriviallyReplacablePHI(
return New;
}

static bool canSplitPredecessors(PHINode *PN) {
static bool canSplitPredecessors(PHINode *PN, LoopSafetyInfo *SafetyInfo) {
BasicBlock *BB = PN->getParent();
if (!BB->canSplitPredecessors())
return false;
// FIXME: it's not impossible to split LandingPad blocks, but if BlockColors
// already exist it require updating BlockColors for all offspring blocks
// accordingly. By skipping such corner case, we can make updating BlockColors
// after splitting predecessor fairly simple.
if (!SafetyInfo->BlockColors.empty() && BB->getFirstNonPHI()->isEHPad())
return false;
for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI) {
BasicBlock *BBPred = *PI;
if (isa<IndirectBrInst>(BBPred->getTerminator()))
Expand All @@ -868,7 +874,8 @@ static bool canSplitPredecessors(PHINode *PN) {
}

static void splitPredecessorsOfLoopExit(PHINode *PN, DominatorTree *DT,
LoopInfo *LI, const Loop *CurLoop) {
LoopInfo *LI, const Loop *CurLoop,
LoopSafetyInfo *SafetyInfo) {
#ifndef NDEBUG
SmallVector<BasicBlock *, 32> ExitBlocks;
CurLoop->getUniqueExitBlocks(ExitBlocks);
Expand Down Expand Up @@ -910,13 +917,21 @@ static void splitPredecessorsOfLoopExit(PHINode *PN, DominatorTree *DT,
// LE:
// %p = phi [%p1, %LE.split], [%p2, %LE.split2]
//
auto &BlockColors = SafetyInfo->BlockColors;
SmallSetVector<BasicBlock *, 8> PredBBs(pred_begin(ExitBB), pred_end(ExitBB));
while (!PredBBs.empty()) {
BasicBlock *PredBB = *PredBBs.begin();
assert(CurLoop->contains(PredBB) &&
"Expect all predecessors are in the loop");
if (PN->getBasicBlockIndex(PredBB) >= 0)
SplitBlockPredecessors(ExitBB, PredBB, ".split.loop.exit", DT, LI, true);
if (PN->getBasicBlockIndex(PredBB) >= 0) {
BasicBlock *NewPred = SplitBlockPredecessors(
ExitBB, PredBB, ".split.loop.exit", DT, LI, true);
// Since we do not allow splitting EH-block with BlockColors in
// canSplitPredecessors(), we can simply assign predecessor's color to
// the new block.
if (!BlockColors.empty())
BlockColors[NewPred] = BlockColors[PredBB];
}
PredBBs.remove(PredBB);
}
}
Expand All @@ -927,7 +942,7 @@ static void splitPredecessorsOfLoopExit(PHINode *PN, DominatorTree *DT,
/// position, and may either delete it or move it to outside of the loop.
///
static bool sink(Instruction &I, LoopInfo *LI, DominatorTree *DT,
const Loop *CurLoop, const LoopSafetyInfo *SafetyInfo,
const Loop *CurLoop, LoopSafetyInfo *SafetyInfo,
OptimizationRemarkEmitter *ORE, bool FreeInLoop) {
DEBUG(dbgs() << "LICM sinking instruction: " << I << "\n");
ORE->emit([&]() {
Expand Down Expand Up @@ -975,12 +990,12 @@ static bool sink(Instruction &I, LoopInfo *LI, DominatorTree *DT,
if (isTriviallyReplacablePHI(*PN, I))
continue;

if (!canSplitPredecessors(PN))
if (!canSplitPredecessors(PN, SafetyInfo))
return Changed;

// Split predecessors of the PHI so that we can make users trivially
// replacable.
splitPredecessorsOfLoopExit(PN, DT, LI, CurLoop);
splitPredecessorsOfLoopExit(PN, DT, LI, CurLoop, SafetyInfo);

// Should rebuild the iterators, as they may be invalidated by
// splitPredecessorsOfLoopExit().
Expand Down
61 changes: 61 additions & 0 deletions test/Transforms/LICM/sinking.ll
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,67 @@ try.cont:
ret void
}

; The sinkable call should be sunk into an exit block split. After splitting
; the exit block, BlockColor for new blocks should be added properly so
; that we should be able to access valid ColorVector.
;
; CHECK-LABEL:@test21_pr36184
; CHECK-LABEL: Loop
; CHECK-NOT: %sinkableCall
; CHECK-LABEL:Out.split.loop.exit
; CHECK: %sinkableCall
define i32 @test21_pr36184(i8* %P) personality i32 (...)* @__CxxFrameHandler3 {
entry:
br label %loop.ph

loop.ph:
br label %Loop

Loop:
%sinkableCall = call i32 @strlen( i8* %P ) readonly
br i1 undef, label %ContLoop, label %Out

ContLoop:
br i1 undef, label %Loop, label %Out

Out:
%idx = phi i32 [ %sinkableCall, %Loop ], [0, %ContLoop ]
ret i32 %idx
}

; We do not support splitting a landingpad block if BlockColors is not empty.
; CHECK-LABEL: @test22
; CHECK-LABEL: while.body2
; CHECK-LABEL: %mul
; CHECK-NOT: lpadBB.split{{.*}}
define void @test22(i1 %b, i32 %v1, i32 %v2) personality i32 (...)* @__CxxFrameHandler3 {
entry:
br label %while.cond
while.cond:
br i1 %b, label %try.cont, label %while.body

while.body:
invoke void @may_throw()
to label %while.body2 unwind label %lpadBB

while.body2:
%v = call i32 @getv()
%mul = mul i32 %v, %v2
invoke void @may_throw2()
to label %while.cond unwind label %lpadBB
lpadBB:
%.lcssa1 = phi i32 [ 0, %while.body ], [ %mul, %while.body2 ]
landingpad { i8*, i32 }
catch i8* null
br label %lpadBBSucc1

lpadBBSucc1:
ret void

try.cont:
ret void
}

declare void @may_throw()
declare void @may_throw2()
declare i32 @__CxxFrameHandler3(...)
Expand Down

0 comments on commit 9f81bea

Please sign in to comment.