diff --git a/llvm/include/llvm/Analysis/MemorySSAUpdater.h b/llvm/include/llvm/Analysis/MemorySSAUpdater.h --- a/llvm/include/llvm/Analysis/MemorySSAUpdater.h +++ b/llvm/include/llvm/Analysis/MemorySSAUpdater.h @@ -174,36 +174,33 @@ // 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 --- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp +++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp @@ -1897,7 +1897,7 @@ auto *LastDef = cast(Updater.getMemorySSA()->getMemoryAccess(Malloc)); auto *NewAccess = - Updater.createMemoryAccessAfter(cast(Calloc), LastDef, + Updater.createMemoryAccessAfter(cast(Calloc), nullptr, LastDef); auto *NewAccessMD = cast(NewAccess); Updater.insertDef(NewAccessMD, /*RenameUses=*/true); diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -1416,16 +1416,8 @@ 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(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(NewAccess)) MSSAU->insertDef(NewDef, /*RenameUses=*/true); else @@ -2023,14 +2015,12 @@ } } - // 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(FirstNonDom)) : MSSAU->createMemoryAccessInBB( - NewS, MSSAU->getMemorySSA()->getLiveOnEntryDef(), + NewS, nullptr, NewS->getParent(), MemorySSA::BeforeTerminator); MSSAU->insertDef(cast(NewDef), /*RenameUses=*/false); diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp --- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp +++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp @@ -357,21 +357,13 @@ // 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( MSSAU->getMemorySSA()->getMemoryAccess(&*BI)); - if (CurrentAcc) { + if (CurrentAcc) MemInsertPoint = CurrentAcc; - if (auto *CurrentDef = dyn_cast(CurrentAcc)) - LastMemDef = CurrentDef; - } // Calls that only access inaccessible memory do not block merging // accessible stores. @@ -475,16 +467,13 @@ if (!Range.TheStores.empty()) AMemSet->setDebugLoc(Range.TheStores[0]->getDebugLoc()); - assert(LastMemDef && MemInsertPoint && - "Both LastMemDef and MemInsertPoint need to be set"); auto *NewDef = cast(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 @@ auto *LastDef = cast(MSSAU->getMemorySSA()->getMemoryAccess(SI)); - auto *NewAccess = MSSAU->createMemoryAccessAfter(M, LastDef, LastDef); + auto *NewAccess = MSSAU->createMemoryAccessAfter(M, nullptr, LastDef); MSSAU->insertDef(cast(NewAccess), /*RenameUses=*/true); eraseInstruction(SI); @@ -814,7 +803,7 @@ // store, so we do not need to rename uses. auto *StoreDef = cast(MSSA->getMemoryAccess(SI)); auto *NewAccess = MSSAU->createMemoryAccessBefore( - M, StoreDef->getDefiningAccess(), StoreDef); + M, nullptr, StoreDef); MSSAU->insertDef(cast(NewAccess), /*RenameUses=*/false); eraseInstruction(SI); @@ -1203,7 +1192,7 @@ assert(isa(MSSAU->getMemorySSA()->getMemoryAccess(M))); auto *LastDef = cast(MSSAU->getMemorySSA()->getMemoryAccess(M)); - auto *NewAccess = MSSAU->createMemoryAccessAfter(NewM, LastDef, LastDef); + auto *NewAccess = MSSAU->createMemoryAccessAfter(NewM, nullptr, LastDef); MSSAU->insertDef(cast(NewAccess), /*RenameUses=*/true); // Remove the instruction we're replacing. @@ -1315,7 +1304,7 @@ auto *LastDef = cast(MSSAU->getMemorySSA()->getMemoryAccess(MemCpy)); auto *NewAccess = MSSAU->createMemoryAccessBefore( - NewMemSet, LastDef->getDefiningAccess(), LastDef); + NewMemSet, nullptr, LastDef); MSSAU->insertDef(cast(NewAccess), /*RenameUses=*/true); eraseInstruction(MemSet); @@ -1420,7 +1409,7 @@ CopySize, MemCpy->getDestAlign()); auto *LastDef = cast(MSSAU->getMemorySSA()->getMemoryAccess(MemCpy)); - auto *NewAccess = MSSAU->createMemoryAccessAfter(NewM, LastDef, LastDef); + auto *NewAccess = MSSAU->createMemoryAccessAfter(NewM, nullptr, LastDef); MSSAU->insertDef(cast(NewAccess), /*RenameUses=*/true); return true; @@ -1610,8 +1599,7 @@ 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(StartMA), /*RenameUses=*/true); // Create a new lifetime end marker after the last user of src or alloca @@ -1623,10 +1611,7 @@ 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(EndMA), /*RenameUses=*/true); } @@ -1674,7 +1659,7 @@ auto *LastDef = cast(MSSAU->getMemorySSA()->getMemoryAccess(M)); auto *NewAccess = - MSSAU->createMemoryAccessAfter(NewM, LastDef, LastDef); + MSSAU->createMemoryAccessAfter(NewM, nullptr, LastDef); MSSAU->insertDef(cast(NewAccess), /*RenameUses=*/true); eraseInstruction(M);