[llvm-branch-commits] Transforms: Have CSE/GVN/LICM check isProfitableToHoist() before hoisting. (PR #141325)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri May 23 20:15:35 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Peter Collingbourne (pcc)
<details>
<summary>Changes</summary>
LICM hoists instructions into the preheader, and CSE and GVN
can effectively perform a hoist by replacing an instruction with
one from another basic block, all of which can lead to the same
kinds of pessimizations that isProfitableToHoist() is intended to
prevent. Therefore, call the hook from all these passes.
This will be tested in the subsequent change that adds a previously
hoistable case and verifies that no optimization pass hoists it.
---
Full diff: https://github.com/llvm/llvm-project/pull/141325.diff
5 Files Affected:
- (modified) llvm/include/llvm/Transforms/Scalar/GVN.h (+4-1)
- (modified) llvm/include/llvm/Transforms/Utils/LoopUtils.h (+4-4)
- (modified) llvm/lib/Transforms/Scalar/EarlyCSE.cpp (+10)
- (modified) llvm/lib/Transforms/Scalar/GVN.cpp (+18-2)
- (modified) llvm/lib/Transforms/Scalar/LICM.cpp (+11-10)
``````````diff
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index 25b042eebf664..b7a2ced3a7a21 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -53,6 +53,7 @@ class NonLocalDepResult;
class OptimizationRemarkEmitter;
class PHINode;
class TargetLibraryInfo;
+class TargetTransformInfo;
class Value;
/// A private "module" namespace for types and utilities used by GVN. These
/// are implementation details and should not be used by clients.
@@ -226,6 +227,7 @@ class GVNPass : public PassInfoMixin<GVNPass> {
MemoryDependenceResults *MD = nullptr;
DominatorTree *DT = nullptr;
const TargetLibraryInfo *TLI = nullptr;
+ const TargetTransformInfo *TTI = nullptr;
AssumptionCache *AC = nullptr;
SetVector<BasicBlock *> DeadBlocks;
OptimizationRemarkEmitter *ORE = nullptr;
@@ -318,7 +320,8 @@ class GVNPass : public PassInfoMixin<GVNPass> {
using UnavailBlkVect = SmallVector<BasicBlock *, 64>;
bool runImpl(Function &F, AssumptionCache &RunAC, DominatorTree &RunDT,
- const TargetLibraryInfo &RunTLI, AAResults &RunAA,
+ const TargetLibraryInfo &RunTLI,
+ const TargetTransformInfo &RunTTI, AAResults &RunAA,
MemoryDependenceResults *RunMD, LoopInfo &LI,
OptimizationRemarkEmitter *ORE, MemorySSA *MSSA = nullptr);
diff --git a/llvm/include/llvm/Transforms/Utils/LoopUtils.h b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
index 416a0a70325d1..4fa8445ce5cc8 100644
--- a/llvm/include/llvm/Transforms/Utils/LoopUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
@@ -175,10 +175,10 @@ bool sinkRegionForLoopNest(DomTreeNode *, AAResults *, LoopInfo *,
/// \p AllowSpeculation is whether values should be hoisted even if they are not
/// guaranteed to execute in the loop, but are safe to speculatively execute.
bool hoistRegion(DomTreeNode *, AAResults *, LoopInfo *, DominatorTree *,
- AssumptionCache *, TargetLibraryInfo *, Loop *,
- MemorySSAUpdater &, ScalarEvolution *, ICFLoopSafetyInfo *,
- SinkAndHoistLICMFlags &, OptimizationRemarkEmitter *, bool,
- bool AllowSpeculation);
+ AssumptionCache *, TargetLibraryInfo *, TargetTransformInfo *,
+ Loop *, MemorySSAUpdater &, ScalarEvolution *,
+ ICFLoopSafetyInfo *, SinkAndHoistLICMFlags &,
+ OptimizationRemarkEmitter *, bool, bool AllowSpeculation);
/// Return true if the induction variable \p IV in a Loop whose latch is
/// \p LatchBlock would become dead if the exit test \p Cond were removed.
diff --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index 1e378369166c0..16faf72c488fe 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -1534,6 +1534,16 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
}
// See if the instruction has an available value. If so, use it.
if (Value *V = AvailableValues.lookup(&Inst)) {
+ // If this CSE would effectively hoist the instruction and that's known
+ // to be unprofitable, don't do it. Remember the instruction so that it
+ // can be used by other instructions in the same block.
+ if (auto *ReplI = dyn_cast<Instruction>(V)) {
+ if (ReplI->getParent() != Inst.getParent() &&
+ !TTI.isProfitableToHoist(&Inst)) {
+ AvailableValues.insert(&Inst, &Inst);
+ continue;
+ }
+ }
LLVM_DEBUG(dbgs() << "EarlyCSE CSE: " << Inst << " to: " << *V
<< '\n');
if (!DebugCounter::shouldExecute(CSECounter)) {
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 77ca14f88a834..0cb38bcfff2d4 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -42,6 +42,7 @@
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/Analysis/PHITransAddr.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
+#include "llvm/Analysis/TargetTransformInfo.h"
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/IR/Attributes.h"
#include "llvm/IR/BasicBlock.h"
@@ -832,6 +833,7 @@ PreservedAnalyses GVNPass::run(Function &F, FunctionAnalysisManager &AM) {
auto &AC = AM.getResult<AssumptionAnalysis>(F);
auto &DT = AM.getResult<DominatorTreeAnalysis>(F);
auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
+ auto &TTI = AM.getResult<TargetIRAnalysis>(F);
auto &AA = AM.getResult<AAManager>(F);
auto *MemDep =
isMemDepEnabled() ? &AM.getResult<MemoryDependenceAnalysis>(F) : nullptr;
@@ -843,7 +845,7 @@ PreservedAnalyses GVNPass::run(Function &F, FunctionAnalysisManager &AM) {
MSSA = &AM.getResult<MemorySSAAnalysis>(F);
}
auto &ORE = AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
- bool Changed = runImpl(F, AC, DT, TLI, AA, MemDep, LI, &ORE,
+ bool Changed = runImpl(F, AC, DT, TLI, TTI, AA, MemDep, LI, &ORE,
MSSA ? &MSSA->getMSSA() : nullptr);
if (!Changed)
return PreservedAnalyses::all();
@@ -2719,6 +2721,16 @@ bool GVNPass::processInstruction(Instruction *I) {
return false;
}
+ // If this GVN would effectively hoist the instruction and that's known to be
+ // unprofitable, don't do it. Remember the instruction so that it can be used
+ // by other instructions in the same block.
+ if (auto *ReplI = dyn_cast<Instruction>(I)) {
+ if (ReplI->getParent() != I->getParent() && !TTI->isProfitableToHoist(I)) {
+ LeaderTable.insert(Num, I, I->getParent());
+ return false;
+ }
+ }
+
// Remove it!
patchAndReplaceAllUsesWith(I, Repl);
if (MD && Repl->getType()->isPtrOrPtrVectorTy())
@@ -2729,13 +2741,15 @@ bool GVNPass::processInstruction(Instruction *I) {
/// runOnFunction - This is the main transformation entry point for a function.
bool GVNPass::runImpl(Function &F, AssumptionCache &RunAC, DominatorTree &RunDT,
- const TargetLibraryInfo &RunTLI, AAResults &RunAA,
+ const TargetLibraryInfo &RunTLI,
+ const TargetTransformInfo &RunTTI, AAResults &RunAA,
MemoryDependenceResults *RunMD, LoopInfo &LI,
OptimizationRemarkEmitter *RunORE, MemorySSA *MSSA) {
AC = &RunAC;
DT = &RunDT;
VN.setDomTree(DT);
TLI = &RunTLI;
+ TTI = &RunTTI;
VN.setAliasAnalysis(&RunAA);
MD = RunMD;
ImplicitControlFlowTracking ImplicitCFT;
@@ -3295,6 +3309,7 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
F, getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F),
getAnalysis<DominatorTreeWrapperPass>().getDomTree(),
getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(F),
+ getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F),
getAnalysis<AAResultsWrapperPass>().getAAResults(),
Impl.isMemDepEnabled()
? &getAnalysis<MemoryDependenceWrapperPass>().getMemDep()
@@ -3308,6 +3323,7 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
AU.addRequired<AssumptionCacheTracker>();
AU.addRequired<DominatorTreeWrapperPass>();
AU.addRequired<TargetLibraryInfoWrapperPass>();
+ AU.addRequired<TargetTransformInfoWrapperPass>();
AU.addRequired<LoopInfoWrapperPass>();
if (Impl.isMemDepEnabled())
AU.addRequired<MemoryDependenceWrapperPass>();
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index 62ef40c686874..ae5c4387a49c3 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -467,9 +467,9 @@ bool LoopInvariantCodeMotion::runOnLoop(Loop *L, AAResults *AA, LoopInfo *LI,
MSSAU, &SafetyInfo, Flags, ORE);
Flags.setIsSink(false);
if (Preheader)
- Changed |= hoistRegion(DT->getNode(L->getHeader()), AA, LI, DT, AC, TLI, L,
- MSSAU, SE, &SafetyInfo, Flags, ORE, LoopNestMode,
- LicmAllowSpeculation);
+ Changed |= hoistRegion(DT->getNode(L->getHeader()), AA, LI, DT, AC, TLI,
+ TTI, L, MSSAU, SE, &SafetyInfo, Flags, ORE,
+ LoopNestMode, LicmAllowSpeculation);
// Now that all loop invariants have been removed from the loop, promote any
// memory references to scalars that we can.
@@ -873,9 +873,9 @@ class ControlFlowHoister {
///
bool llvm::hoistRegion(DomTreeNode *N, AAResults *AA, LoopInfo *LI,
DominatorTree *DT, AssumptionCache *AC,
- TargetLibraryInfo *TLI, Loop *CurLoop,
- MemorySSAUpdater &MSSAU, ScalarEvolution *SE,
- ICFLoopSafetyInfo *SafetyInfo,
+ TargetLibraryInfo *TLI, TargetTransformInfo *TTI,
+ Loop *CurLoop, MemorySSAUpdater &MSSAU,
+ ScalarEvolution *SE, ICFLoopSafetyInfo *SafetyInfo,
SinkAndHoistLICMFlags &Flags,
OptimizationRemarkEmitter *ORE, bool LoopNestMode,
bool AllowSpeculation) {
@@ -911,11 +911,12 @@ bool llvm::hoistRegion(DomTreeNode *N, AAResults *AA, LoopInfo *LI,
// TODO: It may be safe to hoist if we are hoisting to a conditional block
// and we have accurately duplicated the control flow from the loop header
// to that block.
- if (CurLoop->hasLoopInvariantOperands(&I) &&
+ if (TTI->isProfitableToHoist(&I) &&
+ CurLoop->hasLoopInvariantOperands(&I) &&
canSinkOrHoistInst(I, AA, DT, CurLoop, MSSAU, true, Flags, ORE) &&
- isSafeToExecuteUnconditionally(
- I, DT, TLI, CurLoop, SafetyInfo, ORE,
- Preheader->getTerminator(), AC, AllowSpeculation)) {
+ isSafeToExecuteUnconditionally(I, DT, TLI, CurLoop, SafetyInfo, ORE,
+ Preheader->getTerminator(), AC,
+ AllowSpeculation)) {
hoist(I, DT, CurLoop, CFH.getOrCreateHoistedBlock(BB), SafetyInfo,
MSSAU, SE, ORE);
HoistedInstructions.push_back(&I);
``````````
</details>
https://github.com/llvm/llvm-project/pull/141325
More information about the llvm-branch-commits
mailing list