[PATCH] D63355: [MemorySSA] Don't use template when the clone is a simplified instruction.

[Alina Sbirlea via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363597: [MemorySSA] Don't use template when the clone is a simplified instruction. (authored by asbirlea, committed by ).

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63355/new/

https://reviews.llvm.org/D63355

Files:
  llvm/trunk/include/llvm/Analysis/MemorySSAUpdater.h
  llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
  llvm/trunk/test/Analysis/MemorySSA/loop-rotate-inv-template.ll


Index: llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
===================================================================
--- llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
+++ llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
@@ -482,7 +482,8 @@
 
 void MemorySSAUpdater::cloneUsesAndDefs(BasicBlock *BB, BasicBlock *NewBB,
                                         const ValueToValueMapTy &VMap,
-                                        PhiToDefMap &MPhiMap) {
+                                        PhiToDefMap &MPhiMap,
+                                        bool CloneWasSimplified) {
   auto GetNewDefiningAccess = [&](MemoryAccess *MA) -> MemoryAccess * {
     MemoryAccess *InsnDefining = MA;
     if (MemoryUseOrDef *DefMUD = dyn_cast<MemoryUseOrDef>(InsnDefining)) {
@@ -512,10 +513,14 @@
       // instructions. This occurs in LoopRotate when cloning instructions
       // from the old header to the old preheader. The cloned instruction may
       // also be a simplified Value, not an Instruction (see LoopRotate).
+      // Also in LoopRotate, even when it's an instruction, due to it being
+      // simplified, it may be a Use rather than a Def, so we cannot use MUD as
+      // template. Calls coming from updateForClonedBlockIntoPred, ensure this.
       if (Instruction *NewInsn =
               dyn_cast_or_null<Instruction>(VMap.lookup(Insn))) {
         MemoryAccess *NewUseOrDef = MSSA->createDefinedAccess(
-            NewInsn, GetNewDefiningAccess(MUD->getDefiningAccess()), MUD);
+            NewInsn, GetNewDefiningAccess(MUD->getDefiningAccess()),
+            CloneWasSimplified ? nullptr : MUD);
         MSSA->insertIntoListsForBlock(NewUseOrDef, NewBB, MemorySSA::End);
       }
     }
@@ -645,10 +650,13 @@
   // Defs from BB being used in BB will be replaced with the cloned defs from
   // VM. The uses of BB's Phi (if it exists) in BB will be replaced by the
   // incoming def into the Phi from P1.
+  // Instructions cloned into the predecessor are in practice sometimes
+  // simplified, so disable the use of the template, and create an access from
+  // scratch.
   PhiToDefMap MPhiMap;
   if (MemoryPhi *MPhi = MSSA->getMemoryAccess(BB))
     MPhiMap[MPhi] = MPhi->getIncomingValueForBlock(P1);
-  cloneUsesAndDefs(BB, P1, VM, MPhiMap);
+  cloneUsesAndDefs(BB, P1, VM, MPhiMap, /*CloneWasSimplified=*/true);
 }
 
 template <typename Iter>
Index: llvm/trunk/include/llvm/Analysis/MemorySSAUpdater.h
===================================================================
--- llvm/trunk/include/llvm/Analysis/MemorySSAUpdater.h
+++ llvm/trunk/include/llvm/Analysis/MemorySSAUpdater.h
@@ -287,8 +287,14 @@
   // not necessarily be MemoryPhis themselves, they may be MemoryDefs. As such,
   // the map is between MemoryPhis and MemoryAccesses, where the MemoryAccesses
   // may be MemoryPhis or MemoryDefs and not MemoryUses.
+  // If CloneWasSimplified = true, the clone was exact. Otherwise, assume that
+  // the clone involved simplifications that may have: (1) turned a MemoryUse
+  // into an instruction that MemorySSA has no representation for, or (2) turned
+  // a MemoryDef into a MemoryUse or an instruction that MemorySSA has no
+  // representation for. No other cases are supported.
   void cloneUsesAndDefs(BasicBlock *BB, BasicBlock *NewBB,
-                        const ValueToValueMapTy &VMap, PhiToDefMap &MPhiMap);
+                        const ValueToValueMapTy &VMap, PhiToDefMap &MPhiMap,
+                        bool CloneWasSimplified = false);
   template <typename Iter>
   void privateUpdateExitBlocksForClonedLoop(ArrayRef<BasicBlock *> ExitBlocks,
                                             Iter ValuesBegin, Iter ValuesEnd,
Index: llvm/trunk/test/Analysis/MemorySSA/loop-rotate-inv-template.ll
===================================================================
--- llvm/trunk/test/Analysis/MemorySSA/loop-rotate-inv-template.ll
+++ llvm/trunk/test/Analysis/MemorySSA/loop-rotate-inv-template.ll
@@ -0,0 +1,27 @@
+; RUN: opt -disable-output -loop-rotate -enable-mssa-loop-dependency -verify-memoryssa %s
+; REQUIRES: asserts
+
+; Function Attrs: nounwind
+define dso_local void @bar() local_unnamed_addr #0 align 32 {
+entry:
+  br label %looplabel.exit.i
+
+looplabel.exit.i: ; preds = %if.end.i, %entry
+  %0 = phi i1 (i32*, i32*)* [ @foo, %entry ], [ undef, %if.end.i ]
+  %call3.i.i = call zeroext i1 %0(i32* nonnull dereferenceable(16) undef, i32* nonnull undef)
+  br i1 %call3.i.i, label %if.end.i, label %label.exit
+
+if.end.i:                                         ; preds = %looplabel.exit.i
+  %tobool.i = icmp eq i32* undef, null
+  br label %looplabel.exit.i
+
+label.exit: ; preds = %looplabel.exit.i
+  ret void
+}
+
+; Function Attrs: readonly
+declare dso_local i1 @foo(i32*, i32*) #1 align 32
+
+attributes #0 = { nounwind }
+attributes #1 = { readonly }
+


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63355.205144.patch
Type: text/x-patch
Size: 4847 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190617/a8fb5300/attachment.bin>