[llvm] 9d2f8ec - [MSSAU] Clarify that the defining access does not matter (NFC)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 16 00:00:40 PDT 2023
Author: Nikita Popov
Date: 2023-08-16T09:00:32+02:00
New Revision: 9d2f8ecac88c73242c99e7b353ac985fc57f1ed3
URL: https://github.com/llvm/llvm-project/commit/9d2f8ecac88c73242c99e7b353ac985fc57f1ed3
DIFF: https://github.com/llvm/llvm-project/commit/9d2f8ecac88c73242c99e7b353ac985fc57f1ed3.diff
LOG: [MSSAU] Clarify that the defining access does not matter (NFC)
New memory accesses are usually inserted by using one of the
createMemoryAccessXYZ() methods followed by insertUse() or
insertDef(). createMemoryAccessXYZ() accepts a defining access,
however this defining access will always be overwritten by
insertUse() / insertDef().
Update the documentation to clarify this, and stop passing
Definition to createMemoryAccessXYZ() if it's followed by
insertUse/insertDef.
Alternatively, we could also make insertUse / insertDef keep the
defining access if it is specified, and only recompute it if it's
missing.
Differential Revision: https://reviews.llvm.org/D157979
Added:
Modified:
llvm/include/llvm/Analysis/MemorySSAUpdater.h
llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
llvm/lib/Transforms/Scalar/GVN.cpp
llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/MemorySSAUpdater.h b/llvm/include/llvm/Analysis/MemorySSAUpdater.h
index 2bcd1a46287164..d4da3ef1146db7 100644
--- a/llvm/include/llvm/Analysis/MemorySSAUpdater.h
+++ b/llvm/include/llvm/Analysis/MemorySSAUpdater.h
@@ -174,36 +174,33 @@ class MemorySSAUpdater {
// the edge cases right, and the above calls already operate in near-optimal
// time bounds.
- /// Create a MemoryAccess in MemorySSA at a specified point in a block,
- /// with a specified clobbering definition.
+ /// Create a MemoryAccess in MemorySSA at a specified point in a block.
+ ///
+ /// When used by itself, this method will only insert the new MemoryAccess
+ /// into the access list, but not make any other changes, such as inserting
+ /// MemoryPHI nodes, or updating users to point to the new MemoryAccess. You
+ /// must specify a correct Definition in this case.
+ ///
+ /// Usually, this API is instead combined with insertUse() or insertDef(),
+ /// which will perform all the necessary MSSA updates. If these APIs are used,
+ /// then nullptr can be used as Definition, as the correct defining access
+ /// will be automatically determined.
///
- /// Returns the new MemoryAccess.
- /// This should be called when a memory instruction is created that is being
- /// used to replace an existing memory instruction. It will *not* create PHI
- /// nodes, or verify the clobbering definition. The insertion place is used
- /// solely to determine where in the memoryssa access lists the instruction
- /// will be placed. The caller is expected to keep ordering the same as
- /// instructions.
- /// It will return the new MemoryAccess.
/// Note: If a MemoryAccess already exists for I, this function will make it
/// inaccessible and it *must* have removeMemoryAccess called on it.
MemoryAccess *createMemoryAccessInBB(Instruction *I, MemoryAccess *Definition,
const BasicBlock *BB,
MemorySSA::InsertionPlace Point);
- /// Create a MemoryAccess in MemorySSA before or after an existing
- /// MemoryAccess.
- ///
- /// Returns the new MemoryAccess.
- /// This should be called when a memory instruction is created that is being
- /// used to replace an existing memory instruction. It will *not* create PHI
- /// nodes, or verify the clobbering definition.
+ /// Create a MemoryAccess in MemorySSA before an existing MemoryAccess.
///
- /// Note: If a MemoryAccess already exists for I, this function will make it
- /// inaccessible and it *must* have removeMemoryAccess called on it.
+ /// See createMemoryAccessInBB() for usage details.
MemoryUseOrDef *createMemoryAccessBefore(Instruction *I,
MemoryAccess *Definition,
MemoryUseOrDef *InsertPt);
+ /// Create a MemoryAccess in MemorySSA after an existing MemoryAccess.
+ ///
+ /// See createMemoryAccessInBB() for usage details.
MemoryUseOrDef *createMemoryAccessAfter(Instruction *I,
MemoryAccess *Definition,
MemoryAccess *InsertPt);
diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
index d3fbe49439a80b..6fd3f5ce753ce8 100644
--- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -1897,7 +1897,7 @@ struct DSEState {
auto *LastDef =
cast<MemoryDef>(Updater.getMemorySSA()->getMemoryAccess(Malloc));
auto *NewAccess =
- Updater.createMemoryAccessAfter(cast<Instruction>(Calloc), LastDef,
+ Updater.createMemoryAccessAfter(cast<Instruction>(Calloc), nullptr,
LastDef);
auto *NewAccessMD = cast<MemoryDef>(NewAccess);
Updater.insertDef(NewAccessMD, /*RenameUses=*/true);
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 09438433077dd2..8b9f89cfe755d6 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -1416,16 +1416,8 @@ void GVNPass::eliminatePartiallyRedundantLoad(
Load->getSyncScopeID(), UnavailableBlock->getTerminator());
NewLoad->setDebugLoc(Load->getDebugLoc());
if (MSSAU) {
- auto *MSSA = MSSAU->getMemorySSA();
- // Get the defining access of the original load or use the load if it is a
- // MemoryDef (e.g. because it is volatile). The inserted loads are
- // guaranteed to load from the same definition.
- auto *LoadAcc = MSSA->getMemoryAccess(Load);
- auto *DefiningAcc =
- isa<MemoryDef>(LoadAcc) ? LoadAcc : LoadAcc->getDefiningAccess();
auto *NewAccess = MSSAU->createMemoryAccessInBB(
- NewLoad, DefiningAcc, NewLoad->getParent(),
- MemorySSA::BeforeTerminator);
+ NewLoad, nullptr, NewLoad->getParent(), MemorySSA::BeforeTerminator);
if (auto *NewDef = dyn_cast<MemoryDef>(NewAccess))
MSSAU->insertDef(NewDef, /*RenameUses=*/true);
else
@@ -2023,14 +2015,12 @@ bool GVNPass::processAssumeIntrinsic(AssumeInst *IntrinsicI) {
}
}
- // This added store is to null, so it will never executed and we can
- // just use the LiveOnEntry def as defining access.
auto *NewDef =
FirstNonDom ? MSSAU->createMemoryAccessBefore(
- NewS, MSSAU->getMemorySSA()->getLiveOnEntryDef(),
+ NewS, nullptr,
const_cast<MemoryUseOrDef *>(FirstNonDom))
: MSSAU->createMemoryAccessInBB(
- NewS, MSSAU->getMemorySSA()->getLiveOnEntryDef(),
+ NewS, nullptr,
NewS->getParent(), MemorySSA::BeforeTerminator);
MSSAU->insertDef(cast<MemoryDef>(NewDef), /*RenameUses=*/false);
diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
index 1dc306f282b7e4..6015bdf88a62ea 100644
--- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -357,21 +357,13 @@ Instruction *MemCpyOptPass::tryMergingIntoMemset(Instruction *StartInst,
// Keeps track of the last memory use or def before the insertion point for
// the new memset. The new MemoryDef for the inserted memsets will be inserted
- // after MemInsertPoint. It points to either LastMemDef or to the last user
- // before the insertion point of the memset, if there are any such users.
+ // after MemInsertPoint.
MemoryUseOrDef *MemInsertPoint = nullptr;
- // Keeps track of the last MemoryDef between StartInst and the insertion point
- // for the new memset. This will become the defining access of the inserted
- // memsets.
- MemoryDef *LastMemDef = nullptr;
for (++BI; !BI->isTerminator(); ++BI) {
auto *CurrentAcc = cast_or_null<MemoryUseOrDef>(
MSSAU->getMemorySSA()->getMemoryAccess(&*BI));
- if (CurrentAcc) {
+ if (CurrentAcc)
MemInsertPoint = CurrentAcc;
- if (auto *CurrentDef = dyn_cast<MemoryDef>(CurrentAcc))
- LastMemDef = CurrentDef;
- }
// Calls that only access inaccessible memory do not block merging
// accessible stores.
@@ -475,16 +467,13 @@ Instruction *MemCpyOptPass::tryMergingIntoMemset(Instruction *StartInst,
if (!Range.TheStores.empty())
AMemSet->setDebugLoc(Range.TheStores[0]->getDebugLoc());
- assert(LastMemDef && MemInsertPoint &&
- "Both LastMemDef and MemInsertPoint need to be set");
auto *NewDef =
cast<MemoryDef>(MemInsertPoint->getMemoryInst() == &*BI
? MSSAU->createMemoryAccessBefore(
- AMemSet, LastMemDef, MemInsertPoint)
+ AMemSet, nullptr, MemInsertPoint)
: MSSAU->createMemoryAccessAfter(
- AMemSet, LastMemDef, MemInsertPoint));
+ AMemSet, nullptr, MemInsertPoint));
MSSAU->insertDef(NewDef, /*RenameUses=*/true);
- LastMemDef = NewDef;
MemInsertPoint = NewDef;
// Zap all the stores.
@@ -693,7 +682,7 @@ bool MemCpyOptPass::processStoreOfLoad(StoreInst *SI, LoadInst *LI,
auto *LastDef =
cast<MemoryDef>(MSSAU->getMemorySSA()->getMemoryAccess(SI));
- auto *NewAccess = MSSAU->createMemoryAccessAfter(M, LastDef, LastDef);
+ auto *NewAccess = MSSAU->createMemoryAccessAfter(M, nullptr, LastDef);
MSSAU->insertDef(cast<MemoryDef>(NewAccess), /*RenameUses=*/true);
eraseInstruction(SI);
@@ -814,7 +803,7 @@ bool MemCpyOptPass::processStore(StoreInst *SI, BasicBlock::iterator &BBI) {
// store, so we do not need to rename uses.
auto *StoreDef = cast<MemoryDef>(MSSA->getMemoryAccess(SI));
auto *NewAccess = MSSAU->createMemoryAccessBefore(
- M, StoreDef->getDefiningAccess(), StoreDef);
+ M, nullptr, StoreDef);
MSSAU->insertDef(cast<MemoryDef>(NewAccess), /*RenameUses=*/false);
eraseInstruction(SI);
@@ -1203,7 +1192,7 @@ bool MemCpyOptPass::processMemCpyMemCpyDependence(MemCpyInst *M,
assert(isa<MemoryDef>(MSSAU->getMemorySSA()->getMemoryAccess(M)));
auto *LastDef = cast<MemoryDef>(MSSAU->getMemorySSA()->getMemoryAccess(M));
- auto *NewAccess = MSSAU->createMemoryAccessAfter(NewM, LastDef, LastDef);
+ auto *NewAccess = MSSAU->createMemoryAccessAfter(NewM, nullptr, LastDef);
MSSAU->insertDef(cast<MemoryDef>(NewAccess), /*RenameUses=*/true);
// Remove the instruction we're replacing.
@@ -1315,7 +1304,7 @@ bool MemCpyOptPass::processMemSetMemCpyDependence(MemCpyInst *MemCpy,
auto *LastDef =
cast<MemoryDef>(MSSAU->getMemorySSA()->getMemoryAccess(MemCpy));
auto *NewAccess = MSSAU->createMemoryAccessBefore(
- NewMemSet, LastDef->getDefiningAccess(), LastDef);
+ NewMemSet, nullptr, LastDef);
MSSAU->insertDef(cast<MemoryDef>(NewAccess), /*RenameUses=*/true);
eraseInstruction(MemSet);
@@ -1420,7 +1409,7 @@ bool MemCpyOptPass::performMemCpyToMemSetOptzn(MemCpyInst *MemCpy,
CopySize, MemCpy->getDestAlign());
auto *LastDef =
cast<MemoryDef>(MSSAU->getMemorySSA()->getMemoryAccess(MemCpy));
- auto *NewAccess = MSSAU->createMemoryAccessAfter(NewM, LastDef, LastDef);
+ auto *NewAccess = MSSAU->createMemoryAccessAfter(NewM, nullptr, LastDef);
MSSAU->insertDef(cast<MemoryDef>(NewAccess), /*RenameUses=*/true);
return true;
@@ -1610,8 +1599,7 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
Builder.SetInsertPoint(FirstUser->getParent(), FirstUser->getIterator());
auto *Start = Builder.CreateLifetimeStart(SrcAlloca, AllocaSize);
auto *FirstMA = MSSA->getMemoryAccess(FirstUser);
- auto *StartMA = MSSAU->createMemoryAccessBefore(
- Start, FirstMA->getDefiningAccess(), FirstMA);
+ auto *StartMA = MSSAU->createMemoryAccessBefore(Start, nullptr, FirstMA);
MSSAU->insertDef(cast<MemoryDef>(StartMA), /*RenameUses=*/true);
// Create a new lifetime end marker after the last user of src or alloca
@@ -1623,10 +1611,7 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
Builder.SetInsertPoint(LastUser->getParent(), ++LastUser->getIterator());
auto *End = Builder.CreateLifetimeEnd(SrcAlloca, AllocaSize);
auto *LastMA = MSSA->getMemoryAccess(LastUser);
- // FIXME: the second argument should be LastMA if LastMA is MemoryDef, but
- // that's updated by insertDef.
- auto *EndMA = MSSAU->createMemoryAccessAfter(
- End, LastMA->getDefiningAccess(), LastMA);
+ auto *EndMA = MSSAU->createMemoryAccessAfter(End, nullptr, LastMA);
MSSAU->insertDef(cast<MemoryDef>(EndMA), /*RenameUses=*/true);
}
@@ -1674,7 +1659,7 @@ bool MemCpyOptPass::processMemCpy(MemCpyInst *M, BasicBlock::iterator &BBI) {
auto *LastDef =
cast<MemoryDef>(MSSAU->getMemorySSA()->getMemoryAccess(M));
auto *NewAccess =
- MSSAU->createMemoryAccessAfter(NewM, LastDef, LastDef);
+ MSSAU->createMemoryAccessAfter(NewM, nullptr, LastDef);
MSSAU->insertDef(cast<MemoryDef>(NewAccess), /*RenameUses=*/true);
eraseInstruction(M);
More information about the llvm-commits
mailing list