[llvm] [GVN] MemorySSA for GVN: embed the memory state in symbolic expressions (PR #123218)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 16 08:29:42 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Antonio Frighetto (antoniofrighetto)
<details>
<summary>Changes</summary>
While migrating towards MemorySSA, account for the memory state modeled by MemorySSA by hashing it, when computing the symbolic expressions for the memory operations. Likewise, when phi-translating while walking the CFG for PRE possibilities, see if the value number of an operand may be refined with one of the value from the incoming edges of the MemoryPhi associated to the current phi.
---
Original patch: https://reviews.llvm.org/D115160.
Minor additions wrt the original version encompass comments and not including the refactoring of createCallExpr (now it should be taking into account the attributes of the call-site as well, not sure if it’s worth the refactor AFAICT).
---
Full diff: https://github.com/llvm/llvm-project/pull/123218.diff
2 Files Affected:
- (modified) llvm/include/llvm/Transforms/Scalar/GVN.h (+8)
- (modified) llvm/lib/Transforms/Scalar/GVN.cpp (+87-6)
``````````diff
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index c8be390799836e..dd47cc917370e2 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -172,6 +172,10 @@ class GVNPass : public PassInfoMixin<GVNPass> {
// Value number to PHINode mapping. Used for phi-translate in scalarpre.
DenseMap<uint32_t, PHINode *> NumberingPhi;
+ // Value number to BasicBlock mapping. Used for phi-translate across
+ // MemoryPhis.
+ DenseMap<uint32_t, BasicBlock *> NumberingBB;
+
// Cache for phi-translate in scalarpre.
using PhiTranslateMap =
DenseMap<std::pair<uint32_t, const BasicBlock *>, uint32_t>;
@@ -179,6 +183,7 @@ class GVNPass : public PassInfoMixin<GVNPass> {
AAResults *AA = nullptr;
MemoryDependenceResults *MD = nullptr;
+ MemorySSA *MSSA = nullptr;
DominatorTree *DT = nullptr;
uint32_t nextValueNumber = 1;
@@ -189,12 +194,14 @@ class GVNPass : public PassInfoMixin<GVNPass> {
Expression createExtractvalueExpr(ExtractValueInst *EI);
Expression createGEPExpr(GetElementPtrInst *GEP);
uint32_t lookupOrAddCall(CallInst *C);
+ uint32_t lookupOrAddLoadStore(Instruction *I);
uint32_t phiTranslateImpl(const BasicBlock *BB, const BasicBlock *PhiBlock,
uint32_t Num, GVNPass &Gvn);
bool areCallValsEqual(uint32_t Num, uint32_t NewNum, const BasicBlock *Pred,
const BasicBlock *PhiBlock, GVNPass &Gvn);
std::pair<uint32_t, bool> assignExpNewValueNum(Expression &exp);
bool areAllValsInBB(uint32_t num, const BasicBlock *BB, GVNPass &Gvn);
+ void addMemoryStateToExp(Instruction *I, Expression &E);
public:
ValueTable();
@@ -217,6 +224,7 @@ class GVNPass : public PassInfoMixin<GVNPass> {
void setAliasAnalysis(AAResults *A) { AA = A; }
AAResults *getAliasAnalysis() const { return AA; }
void setMemDep(MemoryDependenceResults *M) { MD = M; }
+ void setMemorySSA(MemorySSA *M) { MSSA = M; }
void setDomTree(DominatorTree *D) { DT = D; }
uint32_t getNextUnusedValueNumber() { return nextValueNumber; }
void verifyRemoved(const Value *) const;
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 31af2d8a617b63..b0c01bc31c0a8f 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -476,6 +476,27 @@ void GVNPass::ValueTable::add(Value *V, uint32_t num) {
NumberingPhi[num] = PN;
}
+// Include the incoming memory state into the hash of the expression for the
+// given instruction. If the incoming memory state is:
+// * LiveOnEntry, add the value number of the entry block,
+// * a MemoryPhi, add the value number of the basic block corresponding to that
+// MemoryPhi,
+// * a MemoryDef, add the value number of the memory setting instruction.
+void GVNPass::ValueTable::addMemoryStateToExp(Instruction *I, Expression &E) {
+ assert(MSSA && "addMemoryStateToExp should not be called without MemorySSA");
+ assert(MSSA->getMemoryAccess(I) && "Instruction does not access memory");
+ MemoryAccess *MA = MSSA->getSkipSelfWalker()->getClobberingMemoryAccess(I);
+
+ uint32_t N = 0;
+ if (isa<MemoryPhi>(MA))
+ N = lookupOrAdd(MA->getBlock());
+ else if (MSSA->isLiveOnEntryDef(MA))
+ N = lookupOrAdd(&I->getFunction()->getEntryBlock());
+ else
+ N = lookupOrAdd(cast<MemoryDef>(MA)->getMemoryInst());
+ E.varargs.push_back(N);
+}
+
uint32_t GVNPass::ValueTable::lookupOrAddCall(CallInst *C) {
// FIXME: Currently the calls which may access the thread id may
// be considered as not accessing the memory. But this is
@@ -596,10 +617,37 @@ uint32_t GVNPass::ValueTable::lookupOrAddCall(CallInst *C) {
return v;
}
+ if (MSSA && AA->onlyReadsMemory(C)) {
+ Expression exp = createExpr(C);
+ addMemoryStateToExp(C, exp);
+ uint32_t e = assignExpNewValueNum(exp).first;
+ valueNumbering[C] = e;
+ return e;
+ }
+
valueNumbering[C] = nextValueNumber;
return nextValueNumber++;
}
+/// Returns the value number for the specified load or store instruction.
+uint32_t GVNPass::ValueTable::lookupOrAddLoadStore(Instruction *I) {
+ if (!MSSA) {
+ valueNumbering[I] = nextValueNumber;
+ return nextValueNumber++;
+ }
+
+ Expression E;
+ E.type = I->getType();
+ E.opcode = I->getOpcode();
+ for (Use &Op : I->operands())
+ E.varargs.push_back(lookupOrAdd(Op));
+ addMemoryStateToExp(I, E);
+
+ uint32_t N = assignExpNewValueNum(E).first;
+ valueNumbering[I] = N;
+ return N;
+}
+
/// Returns true if a value number exists for the specified value.
bool GVNPass::ValueTable::exists(Value *V) const {
return valueNumbering.contains(V);
@@ -615,6 +663,8 @@ uint32_t GVNPass::ValueTable::lookupOrAdd(Value *V) {
auto *I = dyn_cast<Instruction>(V);
if (!I) {
valueNumbering[V] = nextValueNumber;
+ if (MSSA && isa<BasicBlock>(V))
+ NumberingBB[nextValueNumber] = cast<BasicBlock>(V);
return nextValueNumber++;
}
@@ -674,6 +724,9 @@ uint32_t GVNPass::ValueTable::lookupOrAdd(Value *V) {
valueNumbering[V] = nextValueNumber;
NumberingPhi[nextValueNumber] = cast<PHINode>(V);
return nextValueNumber++;
+ case Instruction::Load:
+ case Instruction::Store:
+ return lookupOrAddLoadStore(I);
default:
valueNumbering[V] = nextValueNumber;
return nextValueNumber++;
@@ -711,6 +764,7 @@ void GVNPass::ValueTable::clear() {
valueNumbering.clear();
expressionNumbering.clear();
NumberingPhi.clear();
+ NumberingBB.clear();
PhiTranslateTable.clear();
nextValueNumber = 1;
Expressions.clear();
@@ -725,6 +779,8 @@ void GVNPass::ValueTable::erase(Value *V) {
// If V is PHINode, V <--> value number is an one-to-one mapping.
if (isa<PHINode>(V))
NumberingPhi.erase(Num);
+ else if (isa<BasicBlock>(V))
+ NumberingBB.erase(Num);
}
/// verifyRemoved - Verify that the value is removed from all internal data
@@ -2294,15 +2350,39 @@ bool GVNPass::ValueTable::areCallValsEqual(uint32_t Num, uint32_t NewNum,
uint32_t GVNPass::ValueTable::phiTranslateImpl(const BasicBlock *Pred,
const BasicBlock *PhiBlock,
uint32_t Num, GVNPass &Gvn) {
+ // See if we can refine the value number by looking at the PN incoming value
+ // for the given predecessor.
if (PHINode *PN = NumberingPhi[Num]) {
- for (unsigned i = 0; i != PN->getNumIncomingValues(); ++i) {
- if (PN->getParent() == PhiBlock && PN->getIncomingBlock(i) == Pred)
- if (uint32_t TransVal = lookup(PN->getIncomingValue(i), false))
- return TransVal;
- }
+ if (PN->getParent() == PhiBlock)
+ for (unsigned i = 0; i != PN->getNumIncomingValues(); ++i)
+ if (PN->getIncomingBlock(i) == Pred)
+ if (uint32_t TransVal = lookup(PN->getIncomingValue(i), false))
+ return TransVal;
return Num;
}
+ if (BasicBlock *BB = NumberingBB[Num]) {
+ assert(MSSA && "NumberingBB is non-empty only when using MemorySSA");
+ // Value numbers of basic blocks are used to represent memory state in
+ // load/store instructions and read-only function calls when said state is
+ // set by a MemoryPhi.
+ if (BB != PhiBlock)
+ return Num;
+ MemoryPhi *MPhi = MSSA->getMemoryAccess(BB);
+ for (unsigned i = 0, N = MPhi->getNumIncomingValues(); i != N; ++i) {
+ if (MPhi->getIncomingBlock(i) != Pred)
+ continue;
+ MemoryAccess *MA = MPhi->getIncomingValue(i);
+ if (auto *PredPhi = dyn_cast<MemoryPhi>(MA))
+ return lookupOrAdd(PredPhi->getBlock());
+ if (MSSA->isLiveOnEntryDef(MA))
+ return lookupOrAdd(&BB->getParent()->getEntryBlock());
+ return lookupOrAdd(cast<MemoryUseOrDef>(MA)->getMemoryInst());
+ }
+ llvm_unreachable(
+ "CFG/MemorySSA mismatch: predecessor not found among incoming blocks");
+ }
+
// If there is any value related with Num is defined in a BB other than
// PhiBlock, it cannot depend on a phi in PhiBlock without going through
// a backedge. We can do an early exit in that case to save compile time.
@@ -2337,7 +2417,7 @@ uint32_t GVNPass::ValueTable::phiTranslateImpl(const BasicBlock *Pred,
}
if (uint32_t NewNum = expressionNumbering[Exp]) {
- if (Exp.opcode == Instruction::Call && NewNum != Num)
+ if (!MSSA && Exp.opcode == Instruction::Call && NewNum != Num)
return areCallValsEqual(Num, NewNum, Pred, PhiBlock, Gvn) ? NewNum : Num;
return NewNum;
}
@@ -2738,6 +2818,7 @@ bool GVNPass::runImpl(Function &F, AssumptionCache &RunAC, DominatorTree &RunDT,
ICF = &ImplicitCFT;
this->LI = &LI;
VN.setMemDep(MD);
+ VN.setMemorySSA(MSSA);
ORE = RunORE;
InvalidBlockRPONumbers = true;
MemorySSAUpdater Updater(MSSA);
``````````
</details>
https://github.com/llvm/llvm-project/pull/123218
More information about the llvm-commits
mailing list