Index: include/llvm/Transforms/Utils/LoopUtils.h =================================================================== --- include/llvm/Transforms/Utils/LoopUtils.h +++ include/llvm/Transforms/Utils/LoopUtils.h @@ -473,16 +473,11 @@ /// getAnalysisUsage. void getLoopAnalysisUsage(AnalysisUsage &AU); -/// Returns true if the hoister and sinker can handle this instruction. -/// If SafetyInfo is null, we are checking for sinking instructions from -/// preheader to loop body (no speculation). -/// If SafetyInfo is not null, we are checking for hoisting/sinking -/// instructions from loop body to preheader/exit. Check if the instruction -/// can execute specultatively. -/// +/// Return true if its safe to hoist or sink the instruction. bool canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT, Loop *CurLoop, AliasSetTracker *CurAST, - LoopSafetyInfo *SafetyInfo); + LoopSafetyInfo *SafetyInfo = nullptr, + bool IsHoist = false); } #endif Index: lib/Transforms/Scalar/LICM.cpp =================================================================== --- lib/Transforms/Scalar/LICM.cpp +++ lib/Transforms/Scalar/LICM.cpp @@ -101,6 +101,8 @@ const LoopInfo *LI, const LoopSafetyInfo *SafetyInfo); +static bool IsSimpleInst(Instruction &I); + namespace { struct LoopInvariantCodeMotion { bool runOnLoop(Loop *L, AliasAnalysis *AA, LoopInfo *LI, DominatorTree *DT, @@ -363,7 +365,7 @@ // operands of the instruction are loop invariant. // if (isNotUsedInLoop(I, CurLoop, SafetyInfo) && - canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST, SafetyInfo)) { + canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST)) { ++II; Changed |= sink(I, LI, DT, CurLoop, CurAST, SafetyInfo); } @@ -416,10 +418,8 @@ // is safe to hoist the instruction. // if (CurLoop->hasLoopInvariantOperands(&I) && - canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST, SafetyInfo) && - isSafeToExecuteUnconditionally( - I, DT, CurLoop, SafetyInfo, - CurLoop->getLoopPreheader()->getTerminator())) + canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST, SafetyInfo, + true /*Hoist*/)) Changed |= hoist(I, DT, CurLoop, SafetyInfo); } @@ -462,13 +462,40 @@ SafetyInfo->BlockColors = colorEHFunclets(*Fn); } -bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT, +bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, + DominatorTree *DT, Loop *CurLoop, AliasSetTracker *CurAST, - LoopSafetyInfo *SafetyInfo) { - // Loads have extra constraints we have to verify before we can hoist them. + LoopSafetyInfo *SafetyInfo, bool IsHoist) { + // In case of hoist, we need to check isSafeToExecuteUnconditionally. + // i.e. as we cant hoist an instruction that could fault out of a loop + // body (e.g. what if the loop body is never executed.) unless the + // instruction meets the conditions in isSafeToExecuteUnconditionally. + // + // In case of sink, its always correct to sink these instructions to + // basic blocks dominated by the current block. + bool SafeToToExecuteUnconditionally = !IsHoist || + isSafeToExecuteUnconditionally(I, DT, CurLoop, SafetyInfo, + CurLoop->getLoopPreheader()->getTerminator()); + + // Early exit if the instruction can not be moved safely. + if (!SafeToToExecuteUnconditionally) + return false; + + // For simple instructions, we can simply move them. + if (IsSimpleInst(I)) + return true; + + // For non-simple instructions, specifically the ones that read or + // write memory. There are extra constraints we have to verify before we can + // hoist or sink them. + // + // i.e. in order to do sinking or hoisting of loads and calls. we need to + // check whether there will be memory invalidations that could happen because + // of the sinking or hoisting. + // if (LoadInst *LI = dyn_cast(&I)) { if (!LI->isUnordered()) - return false; // Don't hoist volatile/atomic loads! + return false; // Don't hoist or sink volatile/atomic loads! // Loads from constant memory are always safe to move, even if they end up // in the same alias set as something that ends up being modified. @@ -477,7 +504,7 @@ if (LI->getMetadata(LLVMContext::MD_invariant_load)) return true; - // Don't hoist loads which have may-aliased stores in loop. + // Don't hoist or sink loads which have may-aliased stores in loop. uint64_t Size = 0; if (LI->getType()->isSized()) Size = I.getModule()->getDataLayout().getTypeStoreSize(LI->getType()); @@ -491,7 +518,7 @@ if (isa(I)) return false; - // Don't sink calls which can throw. + // Don't hoist or sink calls which can throw. if (CI->mayThrow()) return false; @@ -530,23 +557,8 @@ return false; } - // Only these instructions are hoistable/sinkable. - if (!isa(I) && !isa(I) && !isa(I) && - !isa(I) && !isa(I) && - !isa(I) && !isa(I) && - !isa(I) && !isa(I) && - !isa(I)) - return false; - - // SafetyInfo is nullptr if we are checking for sinking from preheader to - // loop body. It will be always safe as there is no speculative execution. - if (!SafetyInfo) - return true; - - // TODO: Plumb the context instruction through to make hoisting and sinking - // more powerful. Hoisting of loads already works due to the special casing - // above. - return isSafeToExecuteUnconditionally(I, DT, CurLoop, SafetyInfo, nullptr); + // We do not understand this instruction, return conservatively. + return false; } /// Returns true if a PHINode is a trivially replaceable with an @@ -672,6 +684,17 @@ return New; } +static bool IsSimpleInst(Instruction &I) { + // These instructions work on SSA values and known to be moveable. + if (isa(I) || isa(I) || isa(I) || + isa(I) || isa(I) || + isa(I) || isa(I) || + isa(I) || isa(I) || + isa(I)) + return true; + return false; +} + /// When an instruction is found to only be used outside of the loop, this /// function moves it to the exit blocks and patches up SSA form as needed. /// This method is guaranteed to remove the original instruction from its Index: lib/Transforms/Scalar/LoopSink.cpp =================================================================== --- lib/Transforms/Scalar/LoopSink.cpp +++ lib/Transforms/Scalar/LoopSink.cpp @@ -283,7 +283,7 @@ // sinked. for (auto II = Preheader->rbegin(), E = Preheader->rend(); II != E;) { Instruction *I = &*II++; - if (!canSinkOrHoistInst(*I, &AA, &DT, &L, &CurAST, nullptr)) + if (!canSinkOrHoistInst(*I, &AA, &DT, &L, &CurAST)) continue; if (sinkInstruction(L, *I, ColdLoopBBs, LoopBlockNumber, LI, DT, BFI)) Changed = true;