From 42a0730c42b74e5a9effaed4714c00b3ff9fa7a9 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Mon, 4 Jan 2016 03:37:39 +0000 Subject: [PATCH] [LICM] Make instruction sinking funclet-aware We had two bugs here: - We might try to sink into a catchswitch, causing verifier failures. - We will succeed in sinking into a cleanuppad but we didn't update the funclet operand bundle. This fixes PR26000. llvm-svn: 256728 --- .../include/llvm/Transforms/Utils/LoopUtils.h | 3 + llvm/lib/Transforms/Scalar/LICM.cpp | 91 +++++++++++++++---- llvm/test/Transforms/LICM/funclet.ll | 67 ++++++++++++++ llvm/test/Transforms/LICM/sinking.ll | 4 +- 4 files changed, 147 insertions(+), 18 deletions(-) create mode 100644 llvm/test/Transforms/LICM/funclet.ll diff --git a/llvm/include/llvm/Transforms/Utils/LoopUtils.h b/llvm/include/llvm/Transforms/Utils/LoopUtils.h index 17aaee03e4a8..2cfacb650ff5 100644 --- a/llvm/include/llvm/Transforms/Utils/LoopUtils.h +++ b/llvm/include/llvm/Transforms/Utils/LoopUtils.h @@ -16,6 +16,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/Analysis/AliasAnalysis.h" +#include "llvm/Analysis/EHPersonalities.h" #include "llvm/IR/Dominators.h" #include "llvm/IR/IRBuilder.h" @@ -39,6 +40,8 @@ struct LICMSafetyInfo { bool MayThrow; // The current loop contains an instruction which // may throw. bool HeaderMayThrow; // Same as previous, but specific to loop header + // Used to update funclet bundle operands. + DenseMap BlockColors; LICMSafetyInfo() : MayThrow(false), HeaderMayThrow(false) {} }; diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp index 6d70cdc3ade2..5d49a0234231 100644 --- a/llvm/lib/Transforms/Scalar/LICM.cpp +++ b/llvm/lib/Transforms/Scalar/LICM.cpp @@ -75,10 +75,12 @@ DisablePromotion("disable-licm-promotion", cl::Hidden, cl::desc("Disable memory promotion in LICM pass")); static bool inSubLoop(BasicBlock *BB, Loop *CurLoop, LoopInfo *LI); -static bool isNotUsedInLoop(const Instruction &I, const Loop *CurLoop); +static bool isNotUsedInLoop(const Instruction &I, const Loop *CurLoop, + const LICMSafetyInfo *SafetyInfo); static bool hoist(Instruction &I, BasicBlock *Preheader); static bool sink(Instruction &I, const LoopInfo *LI, const DominatorTree *DT, - const Loop *CurLoop, AliasSetTracker *CurAST ); + const Loop *CurLoop, AliasSetTracker *CurAST, + const LICMSafetyInfo *SafetyInfo); static bool isGuaranteedToExecute(const Instruction &Inst, const DominatorTree *DT, const Loop *CurLoop, @@ -92,10 +94,10 @@ static bool isSafeToExecuteUnconditionally(const Instruction &Inst, static bool pointerInvalidatedByLoop(Value *V, uint64_t Size, const AAMDNodes &AAInfo, AliasSetTracker *CurAST); -static Instruction *CloneInstructionInExitBlock(const Instruction &I, - BasicBlock &ExitBlock, - PHINode &PN, - const LoopInfo *LI); +static Instruction * +CloneInstructionInExitBlock(Instruction &I, BasicBlock &ExitBlock, PHINode &PN, + const LoopInfo *LI, + const LICMSafetyInfo *SafetyInfo); static bool canSinkOrHoistInst(Instruction &I, AliasAnalysis *AA, DominatorTree *DT, TargetLibraryInfo *TLI, Loop *CurLoop, AliasSetTracker *CurAST, @@ -348,10 +350,10 @@ bool llvm::sinkRegion(DomTreeNode *N, AliasAnalysis *AA, LoopInfo *LI, // outside of the loop. In this case, it doesn't even matter if the // operands of the instruction are loop invariant. // - if (isNotUsedInLoop(I, CurLoop) && + if (isNotUsedInLoop(I, CurLoop, SafetyInfo) && canSinkOrHoistInst(I, AA, DT, TLI, CurLoop, CurAST, SafetyInfo)) { ++II; - Changed |= sink(I, LI, DT, CurLoop, CurAST); + Changed |= sink(I, LI, DT, CurLoop, CurAST, SafetyInfo); } } return Changed; @@ -432,6 +434,14 @@ void llvm::computeLICMSafetyInfo(LICMSafetyInfo * SafetyInfo, Loop * CurLoop) { for (BasicBlock::iterator I = (*BB)->begin(), E = (*BB)->end(); (I != E) && !SafetyInfo->MayThrow; ++I) SafetyInfo->MayThrow |= I->mayThrow(); + + // Compute funclet colors if we might sink/hoist in a function with a funclet + // personality routine. + Function *Fn = CurLoop->getHeader()->getParent(); + if (Fn->hasPersonalityFn()) + if (Constant *PersonalityFn = Fn->getPersonalityFn()) + if (isFuncletEHPersonality(classifyEHPersonality(PersonalityFn))) + SafetyInfo->BlockColors = colorEHFunclets(*Fn); } /// canSinkOrHoistInst - Return true if the hoister and sinker can handle this @@ -466,6 +476,10 @@ bool canSinkOrHoistInst(Instruction &I, AliasAnalysis *AA, DominatorTree *DT, if (isa(I)) return false; + // Don't sink calls which can throw. + if (CI->mayThrow()) + return false; + // Handle simple cases by querying alias analysis. FunctionModRefBehavior Behavior = AA->getModRefBehavior(CI); if (Behavior == FMRB_DoesNotAccessMemory) @@ -534,10 +548,24 @@ static bool isTriviallyReplacablePHI(const PHINode &PN, const Instruction &I) { /// the loop. If this is true, we can sink the instruction to the exit /// blocks of the loop. /// -static bool isNotUsedInLoop(const Instruction &I, const Loop *CurLoop) { +static bool isNotUsedInLoop(const Instruction &I, const Loop *CurLoop, + const LICMSafetyInfo *SafetyInfo) { + const auto &BlockColors = SafetyInfo->BlockColors; for (const User *U : I.users()) { const Instruction *UI = cast(U); if (const PHINode *PN = dyn_cast(UI)) { + const BasicBlock *BB = PN->getParent(); + // We cannot sink uses in catchswitches. + if (isa(BB->getTerminator())) + return false; + + // We need to sink a callsite to a unique funclet. Avoid sinking if the + // phi use is too muddled. + if (isa(I)) + if (!BlockColors.empty() && + BlockColors.find(const_cast(BB))->second.size() != 1) + return false; + // A PHI node where all of the incoming values are this instruction are // special -- they can just be RAUW'ed with the instruction and thus // don't require a use in the predecessor. This is a particular important @@ -565,11 +593,41 @@ static bool isNotUsedInLoop(const Instruction &I, const Loop *CurLoop) { return true; } -static Instruction *CloneInstructionInExitBlock(const Instruction &I, - BasicBlock &ExitBlock, - PHINode &PN, - const LoopInfo *LI) { - Instruction *New = I.clone(); +static Instruction * +CloneInstructionInExitBlock(Instruction &I, BasicBlock &ExitBlock, PHINode &PN, + const LoopInfo *LI, + const LICMSafetyInfo *SafetyInfo) { + Instruction *New; + if (auto *CI = dyn_cast(&I)) { + const auto &BlockColors = SafetyInfo->BlockColors; + + // Sinking call-sites need to be handled differently from other + // instructions. The cloned call-site needs a funclet bundle operand + // appropriate for it's location in the CFG. + SmallVector OpBundles; + for (unsigned BundleIdx = 0, BundleEnd = CI->getNumOperandBundles(); + BundleIdx != BundleEnd; ++BundleIdx) { + OperandBundleUse Bundle = CI->getOperandBundleAt(BundleIdx); + if (Bundle.getTagID() == LLVMContext::OB_funclet) + continue; + + OpBundles.emplace_back(Bundle); + } + + if (!BlockColors.empty()) { + const ColorVector &CV = BlockColors.find(&ExitBlock)->second; + assert(CV.size() == 1 && "non-unique color for exit block!"); + BasicBlock *BBColor = CV.front(); + Instruction *EHPad = BBColor->getFirstNonPHI(); + if (EHPad->isEHPad()) + OpBundles.emplace_back("funclet", EHPad); + } + + New = CallInst::Create(CI, OpBundles); + } else { + New = I.clone(); + } + ExitBlock.getInstList().insert(ExitBlock.getFirstInsertionPt(), New); if (!I.getName().empty()) New->setName(I.getName() + ".le"); @@ -601,7 +659,8 @@ static Instruction *CloneInstructionInExitBlock(const Instruction &I, /// position, and may either delete it or move it to outside of the loop. /// static bool sink(Instruction &I, const LoopInfo *LI, const DominatorTree *DT, - const Loop *CurLoop, AliasSetTracker *CurAST ) { + const Loop *CurLoop, AliasSetTracker *CurAST, + const LICMSafetyInfo *SafetyInfo) { DEBUG(dbgs() << "LICM sinking instruction: " << I << "\n"); bool Changed = false; if (isa(I)) ++NumMovedLoads; @@ -652,7 +711,7 @@ static bool sink(Instruction &I, const LoopInfo *LI, const DominatorTree *DT, New = It->second; else New = SunkCopies[ExitBlock] = - CloneInstructionInExitBlock(I, *ExitBlock, *PN, LI); + CloneInstructionInExitBlock(I, *ExitBlock, *PN, LI, SafetyInfo); PN->replaceAllUsesWith(New); PN->eraseFromParent(); diff --git a/llvm/test/Transforms/LICM/funclet.ll b/llvm/test/Transforms/LICM/funclet.ll new file mode 100644 index 000000000000..4052ef7c4d70 --- /dev/null +++ b/llvm/test/Transforms/LICM/funclet.ll @@ -0,0 +1,67 @@ +; RUN: opt < %s -licm -S | FileCheck %s + +target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32" +target triple = "i386-pc-windows-msvc18.0.0" + +define void @test1(i32* %s, i1 %b) personality i32 (...)* @__CxxFrameHandler3 { +entry: + br label %while.cond + +while.cond: ; preds = %while.body, %entry + %0 = call i32 @pure_computation() + br i1 %b, label %try.cont, label %while.body + +while.body: ; preds = %while.cond + invoke void @may_throw() + to label %while.cond unwind label %catch.dispatch + +catch.dispatch: ; preds = %while.body + %.lcssa1 = phi i32 [ %0, %while.body ] + %cs = catchswitch within none [label %catch] unwind to caller + +catch: ; preds = %catch.dispatch + %cp = catchpad within %cs [i8* null, i32 64, i8* null] + store i32 %.lcssa1, i32* %s + catchret from %cp to label %try.cont + +try.cont: ; preds = %catch, %while.cond + ret void +} + +; CHECK-LABEL: define void @test1( +; CHECK: %[[CALL:.*]] = call i32 @pure_computation() +; CHECK: phi i32 [ %[[CALL]] + +define void @test2(i32* %s, i1 %b) personality i32 (...)* @__CxxFrameHandler3 { +entry: + br label %while.cond + +while.cond: ; preds = %while.body, %entry + %0 = call i32 @pure_computation() + br i1 %b, label %try.cont, label %while.body + +while.body: ; preds = %while.cond + invoke void @may_throw() + to label %while.cond unwind label %catch.dispatch + +catch.dispatch: ; preds = %while.body + %.lcssa1 = phi i32 [ %0, %while.body ] + %cp = cleanuppad within none [] + store i32 %.lcssa1, i32* %s + cleanupret from %cp unwind to caller + +try.cont: ; preds = %catch, %while.cond + ret void +} + +; CHECK-LABEL: define void @test2( +; CHECK: %[[CP:.*]] = cleanuppad within none [] +; CHECK-NEXT: %[[CALL:.*]] = call i32 @pure_computation() [ "funclet"(token %[[CP]]) ] +; CHECK-NEXT: store i32 %[[CALL]], i32* %s +; CHECK-NEXT: cleanupret from %[[CP]] unwind to caller + +declare void @may_throw() + +declare i32 @pure_computation() nounwind argmemonly readonly + +declare i32 @__CxxFrameHandler3(...) diff --git a/llvm/test/Transforms/LICM/sinking.ll b/llvm/test/Transforms/LICM/sinking.ll index 02bf5846a64c..6e9e8d4b7b6f 100644 --- a/llvm/test/Transforms/LICM/sinking.ll +++ b/llvm/test/Transforms/LICM/sinking.ll @@ -1,6 +1,6 @@ ; RUN: opt < %s -basicaa -licm -S | FileCheck %s -declare i32 @strlen(i8*) readonly +declare i32 @strlen(i8*) readonly nounwind declare void @foo() @@ -20,7 +20,7 @@ Out: ; preds = %Loop ; CHECK-NEXT: ret i32 %A } -declare double @sin(double) readnone +declare double @sin(double) readnone nounwind ; Sink readnone function out of loop with unknown memory behavior. define double @test2(double %X) {