[PATCH] D65338: [MemorySSA] Extend allowed behavior for simplified instructions.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea created this revision.
asbirlea added a reviewer: george.burgess.iv.
Herald added subscribers: Prazek, jlebar.
Herald added a project: LLVM.

LoopRotate may simplify instructions, leading to the new instructions not having memory accesses created for them.
Allow this behavior, by allowing the new access to be null when the template is null, and looking upwards for the proper defined access when dealing with simplified instructions.


Repository:
  rL LLVM

https://reviews.llvm.org/D65338

Files:
  lib/Analysis/MemorySSA.cpp
  lib/Analysis/MemorySSAUpdater.cpp
  test/Analysis/MemorySSA/loop-rotate-simplified-clone.ll


Index: test/Analysis/MemorySSA/loop-rotate-simplified-clone.ll
===================================================================
--- /dev/null
+++ test/Analysis/MemorySSA/loop-rotate-simplified-clone.ll
@@ -0,0 +1,28 @@
+; RUN: opt -verify-memoryssa -enable-mssa-loop-dependency -loop-rotate %s -S | FileCheck %s 
+
+; CHECK-LABEL: @test()
+define dso_local void @test() {
+entry:
+  br label %preheader
+
+preheader:
+  br label %l39
+
+l39:
+  %v40 = phi float (float)* [ @foo, %preheader ], [ %v43, %crit_edge ]
+  %v41 = call float %v40(float undef)
+  %v42 = load i32, i32* undef, align 8
+  br i1 undef, label %crit_edge, label %loopexit
+
+crit_edge:
+  %v43 = load float (float)*, float (float)** undef, align 8
+  br label %l39
+
+loopexit:
+  unreachable
+}
+
+; Function Attrs: readnone
+declare dso_local float @foo(float) #0 align 32
+
+attributes #0 = { readnone }
Index: lib/Analysis/MemorySSAUpdater.cpp
===================================================================
--- lib/Analysis/MemorySSAUpdater.cpp
+++ lib/Analysis/MemorySSAUpdater.cpp
@@ -484,15 +484,29 @@
                                         const ValueToValueMapTy &VMap,
                                         PhiToDefMap &MPhiMap,
                                         bool CloneWasSimplified) {
-  auto GetNewDefiningAccess = [&](MemoryAccess *MA) -> MemoryAccess * {
+  std::function<MemoryAccess *(MemoryAccess *)> GetNewDefiningAccess =
+      [&](MemoryAccess *MA) -> MemoryAccess * {
     MemoryAccess *InsnDefining = MA;
-    if (MemoryUseOrDef *DefMUD = dyn_cast<MemoryUseOrDef>(InsnDefining)) {
+    if (MemoryDef *DefMUD = dyn_cast<MemoryDef>(InsnDefining)) {
       if (!MSSA->isLiveOnEntryDef(DefMUD)) {
         Instruction *DefMUDI = DefMUD->getMemoryInst();
         assert(DefMUDI && "Found MemoryUseOrDef with no Instruction.");
         if (Instruction *NewDefMUDI =
-                cast_or_null<Instruction>(VMap.lookup(DefMUDI)))
+                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)) {
+            // 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 = GetNewDefiningAccess(&*(--DefIt));
+          }
+        }
       }
     } else {
       MemoryPhi *DefPhi = cast<MemoryPhi>(InsnDefining);
@@ -521,7 +535,8 @@
         MemoryAccess *NewUseOrDef = MSSA->createDefinedAccess(
             NewInsn, GetNewDefiningAccess(MUD->getDefiningAccess()),
             CloneWasSimplified ? nullptr : MUD);
-        MSSA->insertIntoListsForBlock(NewUseOrDef, NewBB, MemorySSA::End);
+        if (NewUseOrDef)
+          MSSA->insertIntoListsForBlock(NewUseOrDef, NewBB, MemorySSA::End);
       }
     }
   }
Index: lib/Analysis/MemorySSA.cpp
===================================================================
--- lib/Analysis/MemorySSA.cpp
+++ lib/Analysis/MemorySSA.cpp
@@ -1690,10 +1690,11 @@
                                                const MemoryUseOrDef *Template) {
   assert(!isa<PHINode>(I) && "Cannot create a defined access for a PHI");
   MemoryUseOrDef *NewAccess = createNewAccess(I, AA, Template);
-  assert(
-      NewAccess != nullptr &&
-      "Tried to create a memory access for a non-memory touching instruction");
-  NewAccess->setDefiningAccess(Definition);
+  if (Template)
+    assert(NewAccess != nullptr && "Tried to create a memory access for a "
+                                   "non-memory touching instruction");
+  if (NewAccess)
+    NewAccess->setDefiningAccess(Definition);
   return NewAccess;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65338.211961.patch
Type: text/x-patch
Size: 4126 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190726/02f00ba1/attachment.bin>