[PATCH] D55050: [LoopSimplifyCFG] Update MemorySSA in terminator folding. PR39783

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.
mkazantsev added reviewers: jonpa, anna, fedor.sergeev.
Herald added subscribers: george.burgess.iv, Prazek.

Terminator folding transform lacks MemorySSA update for memory Phis,
while they exist within MemorySSA analysis. They need exactly the same
type of updates as regular Phis. Failing to update them properly ends up
with inconsistent MemorySSA and manifests in various assertion failures.

This patch adds Memory Phi updates to this transform.

Thanks to @jonpa for finding this!


https://reviews.llvm.org/D55050

Files:
  lib/Transforms/Scalar/LoopSimplifyCFG.cpp
  test/Transforms/LoopSimplifyCFG/pr39783_1.ll
  test/Transforms/LoopSimplifyCFG/pr39783_2.ll


Index: test/Transforms/LoopSimplifyCFG/pr39783_2.ll
===================================================================
--- test/Transforms/LoopSimplifyCFG/pr39783_2.ll
+++ test/Transforms/LoopSimplifyCFG/pr39783_2.ll
@@ -1,4 +1,3 @@
-; XFAIL: *
 ; RUN: opt  -S -march=z13 -O3 -enable-simple-loop-unswitch -enable-mssa-loop-dependency -enable-loop-simplifycfg-term-folding 2>&1 < %s | FileCheck %s
 ; CHECK-LABEL: @safe_sub_func_uint64_t_u_u(
 
Index: test/Transforms/LoopSimplifyCFG/pr39783_1.ll
===================================================================
--- test/Transforms/LoopSimplifyCFG/pr39783_1.ll
+++ test/Transforms/LoopSimplifyCFG/pr39783_1.ll
@@ -1,4 +1,3 @@
-; XFAIL: *
 ; RUN: opt -march=z13 -S -O3 -enable-simple-loop-unswitch -enable-mssa-loop-dependency -enable-loop-simplifycfg-term-folding 2>&1 < %s | FileCheck %s
 ; CHECK-LABEL: @main(
 
Index: lib/Transforms/Scalar/LoopSimplifyCFG.cpp
===================================================================
--- lib/Transforms/Scalar/LoopSimplifyCFG.cpp
+++ lib/Transforms/Scalar/LoopSimplifyCFG.cpp
@@ -42,7 +42,7 @@
 #define DEBUG_TYPE "loop-simplifycfg"
 
 static cl::opt<bool> EnableTermFolding("enable-loop-simplifycfg-term-folding",
-                                       cl::init(false));
+                                       cl::init(true));
 
 STATISTIC(NumTerminatorsFolded,
           "Number of terminators folded to unconditional branches");
@@ -83,6 +83,7 @@
   Loop &L;
   LoopInfo &LI;
   DominatorTree &DT;
+  MemorySSAUpdater *MSSAU;
 
   // Whether or not the current loop will still exist after terminator constant
   // folding will be done. In theory, there are two ways how it can happen:
@@ -257,6 +258,8 @@
           // the one-input Phi because it is a LCSSA Phi.
           bool PreserveLCSSAPhi = !L.contains(Succ);
           Succ->removePredecessor(BB, PreserveLCSSAPhi);
+          if (MSSAU)
+            MSSAU->removeEdge(BB, Succ);
         } else
           ++TheOnlySuccDuplicates;
 
@@ -267,6 +270,8 @@
       bool PreserveLCSSAPhi = !L.contains(TheOnlySucc);
       for (unsigned Dup = 1; Dup < TheOnlySuccDuplicates; ++Dup)
         TheOnlySucc->removePredecessor(BB, PreserveLCSSAPhi);
+      if (MSSAU && TheOnlySuccDuplicates > 1)
+        MSSAU->removeDuplicatePhiEdgesBetween(BB, TheOnlySucc);
 
       IRBuilder<> Builder(BB->getContext());
       Instruction *Term = BB->getTerminator();
@@ -282,8 +287,9 @@
   }
 
 public:
-  ConstantTerminatorFoldingImpl(Loop &L, LoopInfo &LI, DominatorTree &DT)
-      : L(L), LI(LI), DT(DT) {}
+  ConstantTerminatorFoldingImpl(Loop &L, LoopInfo &LI, DominatorTree &DT,
+                                MemorySSAUpdater *MSSAU)
+      : L(L), LI(LI), DT(DT), MSSAU(MSSAU) {}
   bool run() {
     assert(L.getLoopLatch() && "Should be single latch!");
 
@@ -364,7 +370,8 @@
 
 /// Turn branches and switches with known constant conditions into unconditional
 /// branches.
-static bool constantFoldTerminators(Loop &L, DominatorTree &DT, LoopInfo &LI) {
+static bool constantFoldTerminators(Loop &L, DominatorTree &DT, LoopInfo &LI,
+                                    MemorySSAUpdater *MSSAU) {
   if (!EnableTermFolding)
     return false;
 
@@ -373,7 +380,7 @@
   if (!L.getLoopLatch())
     return false;
 
-  ConstantTerminatorFoldingImpl BranchFolder(L, LI, DT);
+  ConstantTerminatorFoldingImpl BranchFolder(L, LI, DT, MSSAU);
   return BranchFolder.run();
 }
 
@@ -410,7 +417,7 @@
   bool Changed = false;
 
   // Constant-fold terminators with known constant conditions.
-  Changed |= constantFoldTerminators(L, DT, LI);
+  Changed |= constantFoldTerminators(L, DT, LI, MSSAU);
 
   // Eliminate unconditional branches by merging blocks into their predecessors.
   Changed |= mergeBlocksIntoPredecessors(L, DT, LI, MSSAU);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55050.175829.patch
Type: text/x-patch
Size: 3796 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181129/d9c51f5c/attachment.bin>