[llvm] [MSSA] Don't check non-AA memory effects when using template access (PR #76142)

[Alina Sbirlea via llvm-commits]

https://github.com/alinas commented:

I think this makes sense as the simple way to address this.
There's also the option to generalize what to do in the case of accesses that are optimized further.
I *think* that's possible but I may be missing edge cases not covered by current tests. Something like this resolves the testcases in this PR:
```
diff --git llvm/lib/Analysis/MemorySSAUpdater.cpp llvm/lib/Analysis/MemorySSAUpdater.cpp
index 9ad60f774e9f..0e533cb54f15 100644
--- llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -578,18 +578,9 @@ static MemoryAccess *getNewDefiningAccessForClone(MemoryAccess *MA,
       if (Instruction *NewDefMUDI =
               cast_or_null<Instruction>(VMap.lookup(DefMUDI))) {
         InsnDefining = MSSA->getMemoryAccess(NewDefMUDI);
-        if (!CloneWasSimplified)
-          assert(InsnDefining && "Defining instruction cannot be nullptr.");
-        else if (!InsnDefining || isa<MemoryUse>(InsnDefining)) {
+        if (!InsnDefining || isa<MemoryUse>(InsnDefining)) {
           // The clone was simplified, it's no longer a MemoryDef, look up.
-          auto DefIt = DefMUD->getDefsIterator();
-          // Since simplified clones only occur in single block cloning, a
-          // previous definition must exist, otherwise NewDefMUDI would not
-          // have been found in VMap.
-          assert(DefIt != MSSA->getBlockDefs(DefMUD->getBlock())->begin() &&
-                 "Previous def must exist");
-          InsnDefining = getNewDefiningAccessForClone(
-              &*(--DefIt), VMap, MPhiMap, CloneWasSimplified, MSSA);
+          InsnDefining = getNewDefiningAccessForClone(DefMUD->getDefiningAccess(), VMap, MPhiMap, CloneWasSimplified, MSSA);
         }
       }
     }
@@ -626,7 +617,7 @@ void MemorySSAUpdater::cloneUsesAndDefs(BasicBlock *BB, BasicBlock *NewBB,
             getNewDefiningAccessForClone(MUD->getDefiningAccess(), VMap,
                                          MPhiMap, CloneWasSimplified, MSSA),
             /*Template=*/CloneWasSimplified ? nullptr : MUD,
-            /*CreationMustSucceed=*/CloneWasSimplified ? false : true);
+            /*CreationMustSucceed=*/false);
         if (NewUseOrDef)
           MSSA->insertIntoListsForBlock(NewUseOrDef, NewBB, MemorySSA::End);
       }
```
But needs cleanup then of "CloneSimplified" that's now losing its purpose.

If you move fwd with creating accesses for non-memory affecting instructions, a follow up optimization could be to remember those accesses that were still created using a template, then call `removeMemoryAccess` (leaving optimizePhis to false) on those, after cloning and new phi assignment is complete. This should redirect the defining accesses properly.

https://github.com/llvm/llvm-project/pull/76142