diff options
| -rw-r--r-- | llvm/include/llvm/Transforms/Utils/LoopUtils.h | 16 | ||||
| -rw-r--r-- | llvm/lib/Transforms/Scalar/LICM.cpp | 32 | ||||
| -rw-r--r-- | llvm/lib/Transforms/Scalar/LoopSink.cpp | 2 |
3 files changed, 25 insertions, 25 deletions
diff --git a/llvm/include/llvm/Transforms/Utils/LoopUtils.h b/llvm/include/llvm/Transforms/Utils/LoopUtils.h index eb4c99102a6..33dcc2abc81 100644 --- a/llvm/include/llvm/Transforms/Utils/LoopUtils.h +++ b/llvm/include/llvm/Transforms/Utils/LoopUtils.h @@ -497,16 +497,18 @@ Optional<unsigned> getLoopEstimatedTripCount(Loop *L); /// 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 speculatively. +/// Returns true if is legal to hoist or sink this instruction disregarding the +/// possible introduction of faults. Reasoning about potential faulting +/// instructions is the responsibility of the caller since it is challenging to +/// do efficiently from within this routine. +/// \p TargetExecutesOncePerLoop is true only when it is guaranteed that the +/// target executes at most once per execution of the loop body. This is used +/// to assess the legality of duplicating atomic loads. Generally, this is +/// true when moving out of loop and not true when moving into loops. /// If \p ORE is set use it to emit optimization remarks. bool canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT, Loop *CurLoop, AliasSetTracker *CurAST, - LoopSafetyInfo *SafetyInfo, + bool TargetExecutesOncePerLoop, OptimizationRemarkEmitter *ORE = nullptr); /// Generates an ordered vector reduction using extracts to reduce the value. diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp index 02e5839ff5b..0edba8fc993 100644 --- a/llvm/lib/Transforms/Scalar/LICM.cpp +++ b/llvm/lib/Transforms/Scalar/LICM.cpp @@ -412,7 +412,14 @@ bool llvm::sinkRegion(DomTreeNode *N, AliasAnalysis *AA, LoopInfo *LI, // bool FreeInLoop = false; if (isNotUsedOrFreeInLoop(I, CurLoop, SafetyInfo, TTI, FreeInLoop) && - canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST, SafetyInfo, ORE)) { + canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST, true, ORE) && + // FIXME: Remove the special casing here. Why do we need the + // execution check at all? We're sinking from a dominating location + // to a dominated location. + (isa<LoadInst>(I) || isa<CallInst>(I) || + // TODO: Plumb the context instruction through to make sinking + // more powerful. + isSafeToExecuteUnconditionally(I, DT, CurLoop, SafetyInfo, ORE))) { if (sink(I, LI, DT, CurLoop, SafetyInfo, ORE, FreeInLoop)) { if (!FreeInLoop) { ++II; @@ -482,7 +489,7 @@ bool llvm::hoistRegion(DomTreeNode *N, AliasAnalysis *AA, LoopInfo *LI, // if it is safe to hoist the instruction. // if (CurLoop->hasLoopInvariantOperands(&I) && - canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST, SafetyInfo, ORE) && + canSinkOrHoistInst(I, AA, DT, CurLoop, CurAST, true, ORE) && (IsMustExecute || isSafeToExecuteUnconditionally( I, DT, CurLoop, SafetyInfo, ORE, @@ -592,15 +599,12 @@ bool isHoistableAndSinkableInst(Instruction &I) { bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT, Loop *CurLoop, AliasSetTracker *CurAST, - LoopSafetyInfo *SafetyInfo, + bool TargetExecutesOncePerLoop, OptimizationRemarkEmitter *ORE) { // If we don't understand the instruction, bail early. if (!isHoistableAndSinkableInst(I)) return false; - // SafetyInfo is nullptr if we are checking for sinking from preheader to - // loop body. - const bool SinkingToLoopBody = !SafetyInfo; // Loads have extra constraints we have to verify before we can hoist them. if (LoadInst *LI = dyn_cast<LoadInst>(&I)) { if (!LI->isUnordered()) @@ -613,8 +617,8 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT, if (LI->getMetadata(LLVMContext::MD_invariant_load)) return true; - if (LI->isAtomic() && SinkingToLoopBody) - return false; // Don't sink unordered atomic loads to loop body. + if (LI->isAtomic() && !TargetExecutesOncePerLoop) + return false; // Don't risk duplicating unordered loads // This checks for an invariant.start dominating the load. if (isLoadInvariantInLoop(LI, DT, CurLoop)) @@ -685,15 +689,9 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT, return false; } - // If we are checking for sinking from preheader to loop body it will be - // always safe as there is no speculative execution. - if (SinkingToLoopBody) - 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've established mechanical ability and aliasing, it's up to the caller + // to check fault safety + return true; } /// Returns true if a PHINode is a trivially replaceable with an diff --git a/llvm/lib/Transforms/Scalar/LoopSink.cpp b/llvm/lib/Transforms/Scalar/LoopSink.cpp index 760177c9c5e..240d727d1e2 100644 --- a/llvm/lib/Transforms/Scalar/LoopSink.cpp +++ b/llvm/lib/Transforms/Scalar/LoopSink.cpp @@ -290,7 +290,7 @@ static bool sinkLoopInvariantInstructions(Loop &L, AAResults &AA, LoopInfo &LI, // No need to check for instruction's operands are loop invariant. assert(L.hasLoopInvariantOperands(I) && "Insts in a loop's preheader should have loop invariant operands!"); - if (!canSinkOrHoistInst(*I, &AA, &DT, &L, &CurAST, nullptr)) + if (!canSinkOrHoistInst(*I, &AA, &DT, &L, &CurAST, false)) continue; if (sinkInstruction(L, *I, ColdLoopBBs, LoopBlockNumber, LI, DT, BFI)) Changed = true; |
