[llvm] r347979 - [LoopSimplifyCFG] Update MemorySSA in terminator folding. PR39783

[Max Kazantsev via llvm-commits]

Author: mkazantsev
Date: Fri Nov 30 02:06:23 2018
New Revision: 347979

URL: http://llvm.org/viewvc/llvm-project?rev=347979&view=rev
Log:
[LoopSimplifyCFG] Update MemorySSA in terminator folding. PR39783

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!

Differential Revision: https://reviews.llvm.org/D55050
Reviewed By: asbirlea

Modified:
    llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
    llvm/trunk/test/Transforms/LoopSimplifyCFG/pr39783.ll

Modified: llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp?rev=347979&r1=347978&r2=347979&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp Fri Nov 30 02:06:23 2018
@@ -42,7 +42,7 @@ using namespace llvm;
 #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 @@ private:
   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 @@ private:
           // 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 @@ private:
       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 @@ private:
   }
 
 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 @@ public:
 
 /// 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 @@ static bool constantFoldTerminators(Loop
   if (!L.getLoopLatch())
     return false;
 
-  ConstantTerminatorFoldingImpl BranchFolder(L, LI, DT);
+  ConstantTerminatorFoldingImpl BranchFolder(L, LI, DT, MSSAU);
   return BranchFolder.run();
 }
 
@@ -410,7 +417,7 @@ static bool simplifyLoopCFG(Loop &L, Dom
   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);

Modified: llvm/trunk/test/Transforms/LoopSimplifyCFG/pr39783.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopSimplifyCFG/pr39783.ll?rev=347979&r1=347978&r2=347979&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/LoopSimplifyCFG/pr39783.ll (original)
+++ llvm/trunk/test/Transforms/LoopSimplifyCFG/pr39783.ll Fri Nov 30 02:06:23 2018
@@ -1,4 +1,3 @@
-; XFAIL: *
 ; REQUIRES: asserts
 ; RUN: opt -march=z13 -S -loop-simplifycfg -enable-mssa-loop-dependency -enable-loop-simplifycfg-term-folding 2>&1 < %s | FileCheck %s
 target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"