[llvm] r374177 - [MemorySSA] Make the use of moveAllAfterMergeBlocks consistent.

Alina Sbirlea via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 9 08:54:24 PDT 2019


Author: asbirlea
Date: Wed Oct  9 08:54:24 2019
New Revision: 374177

URL: http://llvm.org/viewvc/llvm-project?rev=374177&view=rev
Log:
[MemorySSA] Make the use of moveAllAfterMergeBlocks consistent.

Summary:
The rule for the moveAllAfterMergeBlocks API si for all instructions
from `From` to have been moved to `To`, while keeping the CFG edges (and
block terminators) unchanged.
Update all the callsites for moveAllAfterMergeBlocks to follow this.

Pending follow-up: since the same behavior is needed everytime, merge
all callsites into one. The common denominator may be the call to
`MergeBlockIntoPredecessor`.

Resolves PR43569.

Reviewers: george.burgess.iv

Subscribers: Prazek, sanjoy.google, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68659

Added:
    llvm/trunk/test/Analysis/MemorySSA/pr43569.ll
Modified:
    llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
    llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp
    llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp
    llvm/trunk/lib/Transforms/Utils/LoopRotationUtils.cpp

Modified: llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp?rev=374177&r1=374176&r2=374177&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp (original)
+++ llvm/trunk/lib/Analysis/MemorySSAUpdater.cpp Wed Oct  9 08:54:24 2019
@@ -1159,25 +1159,32 @@ void MemorySSAUpdater::moveAllAccesses(B
   if (!Accs)
     return;
 
+  assert(Start->getParent() == To && "Incorrect Start instruction");
   MemoryAccess *FirstInNew = nullptr;
   for (Instruction &I : make_range(Start->getIterator(), To->end()))
     if ((FirstInNew = MSSA->getMemoryAccess(&I)))
       break;
-  if (!FirstInNew)
-    return;
+  if (FirstInNew) {
+    auto *MUD = cast<MemoryUseOrDef>(FirstInNew);
+    do {
+      auto NextIt = ++MUD->getIterator();
+      MemoryUseOrDef *NextMUD = (!Accs || NextIt == Accs->end())
+                                    ? nullptr
+                                    : cast<MemoryUseOrDef>(&*NextIt);
+      MSSA->moveTo(MUD, To, MemorySSA::End);
+      // Moving MUD from Accs in the moveTo above, may delete Accs, so we need
+      // to retrieve it again.
+      Accs = MSSA->getWritableBlockAccesses(From);
+      MUD = NextMUD;
+    } while (MUD);
+  }
 
-  auto *MUD = cast<MemoryUseOrDef>(FirstInNew);
-  do {
-    auto NextIt = ++MUD->getIterator();
-    MemoryUseOrDef *NextMUD = (!Accs || NextIt == Accs->end())
-                                  ? nullptr
-                                  : cast<MemoryUseOrDef>(&*NextIt);
-    MSSA->moveTo(MUD, To, MemorySSA::End);
-    // Moving MUD from Accs in the moveTo above, may delete Accs, so we need to
-    // retrieve it again.
-    Accs = MSSA->getWritableBlockAccesses(From);
-    MUD = NextMUD;
-  } while (MUD);
+  // If all accesses were moved and only a trivial Phi remains, we try to remove
+  // that Phi. This is needed when From is going to be deleted.
+  auto *Defs = MSSA->getWritableBlockDefs(From);
+  if (Defs && !Defs->empty())
+    if (auto *Phi = dyn_cast<MemoryPhi>(&*Defs->begin()))
+      tryRemoveTrivialPhi(Phi);
 }
 
 void MemorySSAUpdater::moveAllAfterSpliceBlocks(BasicBlock *From,
@@ -1193,7 +1200,7 @@ void MemorySSAUpdater::moveAllAfterSplic
 
 void MemorySSAUpdater::moveAllAfterMergeBlocks(BasicBlock *From, BasicBlock *To,
                                                Instruction *Start) {
-  assert(From->getSinglePredecessor() == To &&
+  assert(From->getUniquePredecessor() == To &&
          "From block is expected to have a single predecessor (To).");
   moveAllAccesses(From, To, Start);
   for (BasicBlock *Succ : successors(From))

Modified: llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp?rev=374177&r1=374176&r2=374177&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp Wed Oct  9 08:54:24 2019
@@ -1629,15 +1629,27 @@ void LoopUnswitch::SimplifyCode(std::vec
           ReplaceUsesOfWith(PN, PN->getIncomingValue(0), Worklist, L, LPM,
                             MSSAU.get());
 
-        // If Succ has any successors with PHI nodes, update them to have
-        // entries coming from Pred instead of Succ.
-        Succ->replaceAllUsesWith(Pred);
+        Instruction *STI = Succ->getTerminator();
+        Instruction *Start = &*Succ->begin();
+        // If there's nothing to move, mark the starting instruction as the last
+        // instruction in the block.
+        if (Start == STI)
+          Start = BI;
 
         // Move all of the successor contents from Succ to Pred.
         Pred->getInstList().splice(BI->getIterator(), Succ->getInstList(),
-                                   Succ->begin(), Succ->end());
+                                   Succ->begin(), STI->getIterator());
         if (MSSAU)
-          MSSAU->moveAllAfterMergeBlocks(Succ, Pred, BI);
+          MSSAU->moveAllAfterMergeBlocks(Succ, Pred, Start);
+
+        // Move terminator instruction from Succ now, we're deleting BI below.
+        // FIXME: remove BI first might be more intuitive.
+        Pred->getInstList().splice(Pred->end(), Succ->getInstList());
+
+        // If Succ has any successors with PHI nodes, update them to have
+        // entries coming from Pred instead of Succ.
+        Succ->replaceAllUsesWith(Pred);
+
         LPM->deleteSimpleAnalysisValue(BI, L);
         RemoveFromWorklist(BI, Worklist);
         BI->eraseFromParent();

Modified: llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp?rev=374177&r1=374176&r2=374177&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/BasicBlockUtils.cpp Wed Oct  9 08:54:24 2019
@@ -227,17 +227,29 @@ bool llvm::MergeBlockIntoPredecessor(Bas
     Updates.push_back({DominatorTree::Delete, PredBB, BB});
   }
 
-  if (MSSAU)
-    MSSAU->moveAllAfterMergeBlocks(BB, PredBB, &*(BB->begin()));
+  Instruction *PTI = PredBB->getTerminator();
+  Instruction *STI = BB->getTerminator();
+  Instruction *Start = &*BB->begin();
+  // If there's nothing to move, mark the starting instruction as the last
+  // instruction in the block.
+  if (Start == STI)
+    Start = PTI;
 
-  // Delete the unconditional branch from the predecessor...
-  PredBB->getInstList().pop_back();
+  // Move all definitions in the successor to the predecessor...
+  PredBB->getInstList().splice(PTI->getIterator(), BB->getInstList(),
+                               BB->begin(), STI->getIterator());
+
+  if (MSSAU)
+    MSSAU->moveAllAfterMergeBlocks(BB, PredBB, Start);
 
   // Make all PHI nodes that referred to BB now refer to Pred as their
   // source...
   BB->replaceAllUsesWith(PredBB);
 
-  // Move all definitions in the successor to the predecessor...
+  // Delete the unconditional branch from the predecessor...
+  PredBB->getInstList().pop_back();
+
+  // Move terminator instruction and add unreachable to now empty BB.
   PredBB->getInstList().splice(PredBB->end(), BB->getInstList());
   new UnreachableInst(BB->getContext(), BB);
 
@@ -274,11 +286,10 @@ bool llvm::MergeBlockIntoPredecessor(Bas
            "applying corresponding DTU updates.");
     DTU->applyUpdatesPermissive(Updates);
     DTU->deleteBB(BB);
-  }
-
-  else {
+  } else {
     BB->eraseFromParent(); // Nuke BB if DTU is nullptr.
   }
+
   return true;
 }
 

Modified: llvm/trunk/lib/Transforms/Utils/LoopRotationUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LoopRotationUtils.cpp?rev=374177&r1=374176&r2=374177&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/LoopRotationUtils.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/LoopRotationUtils.cpp Wed Oct  9 08:54:24 2019
@@ -615,8 +615,13 @@ bool LoopRotate::simplifyLoopLatch(Loop
   LLVM_DEBUG(dbgs() << "Folding loop latch " << Latch->getName() << " into "
                     << LastExit->getName() << "\n");
 
+  Instruction *FirstLatchInst = &*Latch->begin();
+  // If there's nothing to move, mark the starting instruction as the last
+  // instruction in the block.
+  if (FirstLatchInst == Jmp)
+    FirstLatchInst = BI;
+
   // Hoist the instructions from Latch into LastExit.
-  Instruction *FirstLatchInst = &*(Latch->begin());
   LastExit->getInstList().splice(BI->getIterator(), Latch->getInstList(),
                                  Latch->begin(), Jmp->getIterator());
 

Added: llvm/trunk/test/Analysis/MemorySSA/pr43569.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/MemorySSA/pr43569.ll?rev=374177&view=auto
==============================================================================
--- llvm/trunk/test/Analysis/MemorySSA/pr43569.ll (added)
+++ llvm/trunk/test/Analysis/MemorySSA/pr43569.ll Wed Oct  9 08:54:24 2019
@@ -0,0 +1,49 @@
+; RUN: opt -pgo-kind=pgo-instr-gen-pipeline -aa-pipeline=default -passes="default<O3>" -enable-nontrivial-unswitch -S < %s | FileCheck %s
+; REQUIRES: asserts
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at __profn_c = private constant [1 x i8] c"c"
+ at b = common dso_local global i32 0, align 4
+ at a = common dso_local global i16 0, align 2
+
+; CHECK-LABEL: @c()
+; Function Attrs: nounwind uwtable
+define dso_local void @c() #0 {
+entry:
+  call void @llvm.instrprof.increment(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @__profn_c, i32 0, i32 0), i64 68269137, i32 3, i32 0)
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.end, %entry
+  call void @llvm.instrprof.increment(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @__profn_c, i32 0, i32 0), i64 68269137, i32 3, i32 1)
+  store i32 0, i32* @b, align 4
+  br label %for.cond1
+
+for.cond1:                                        ; preds = %for.inc, %for.cond
+  %0 = load i32, i32* @b, align 4
+  %1 = load i16, i16* @a, align 2
+  %conv = sext i16 %1 to i32
+  %cmp = icmp slt i32 %0, %conv
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.cond1
+  call void @llvm.instrprof.increment(i8* getelementptr inbounds ([1 x i8], [1 x i8]* @__profn_c, i32 0, i32 0), i64 68269137, i32 3, i32 2)
+  br label %for.inc
+
+for.inc:                                          ; preds = %for.body
+  %2 = load i32, i32* @b, align 4
+  %inc = add nsw i32 %2, 1
+  store i32 %inc, i32* @b, align 4
+  br label %for.cond1
+
+for.end:                                          ; preds = %for.cond1
+  br label %for.cond
+}
+
+; Function Attrs: nounwind
+declare void @llvm.instrprof.increment(i8*, i64, i32, i32) #1
+
+attributes #0 = { nounwind uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind }
+




More information about the llvm-commits mailing list