[PATCH] D89818: [Compile Time] Make it possible to skip redundant debuginst removal

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau created this revision.
Herald added subscribers: llvm-commits, zzheng, hiraditya.
Herald added a project: LLVM.
ctetreau requested review of this revision.

MergeBlockIntoPredecessor unconditionally calls RemoveRedundantDbgInstrs,
which iterates twice over the instructions of the basic block.
MergeBlockIntoPredecessor is itself called in a hot inner loop,
and as a result, RemoveRedundantDbgInstrs was taking 98% of
the runtime in some cases, and causing 10 minute compile times in some
downstream workloads.

Make the call to RemoveRedundantDbgInstrs optional in
MergeBlockIntoPredecessor, and call RemoveRedundantDbgInstrs after the
loop is unrolled in LoopUnrollPass. This reduces the runtime usage of
RemoveRedundantDbgInstrs to 3%. There are places it can be moved where
the usage will be lower, but moving it into a function called
"simplifyLoopAfterUnroll" seemed a good fit.

See: Bug 47746


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89818

Files:
  llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
  llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
  llvm/lib/Transforms/Utils/LoopUnroll.cpp


Index: llvm/lib/Transforms/Utils/LoopUnroll.cpp
===================================================================
--- llvm/lib/Transforms/Utils/LoopUnroll.cpp
+++ llvm/lib/Transforms/Utils/LoopUnroll.cpp
@@ -234,6 +234,7 @@
       if (isInstructionTriviallyDead(Inst))
         BB->getInstList().erase(Inst);
     }
+    RemoveRedundantDbgInstrs(BB);
   }
 
   // TODO: after peeling or unrolling, previously loop variant conditions are
@@ -862,7 +863,8 @@
     if (Term && Term->isUnconditional()) {
       BasicBlock *Dest = Term->getSuccessor(0);
       BasicBlock *Fold = Dest->getUniquePredecessor();
-      if (MergeBlockIntoPredecessor(Dest, &DTU, LI)) {
+      if (MergeBlockIntoPredecessor(Dest, &DTU, LI, nullptr, nullptr, false,
+                                    true)) {
         // Dest has been folded into Fold. Update our worklists accordingly.
         std::replace(Latches.begin(), Latches.end(), Dest, Fold);
         UnrolledLoopBlocks.erase(std::remove(UnrolledLoopBlocks.begin(),
Index: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
===================================================================
--- llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -172,7 +172,8 @@
 bool llvm::MergeBlockIntoPredecessor(BasicBlock *BB, DomTreeUpdater *DTU,
                                      LoopInfo *LI, MemorySSAUpdater *MSSAU,
                                      MemoryDependenceResults *MemDep,
-                                     bool PredecessorWithTwoSuccessors) {
+                                     bool PredecessorWithTwoSuccessors,
+                                     bool SkipRedundantDebugValElimination) {
   if (BB->hasAddressTaken())
     return false;
 
@@ -285,10 +286,12 @@
   // Add unreachable to now empty BB.
   new UnreachableInst(BB->getContext(), BB);
 
-  // Eliminate duplicate/redundant dbg.values. This seems to be a good place to
-  // do that since we might end up with redundant dbg.values describing the
-  // entry PHI node post-splice.
-  RemoveRedundantDbgInstrs(PredBB);
+  if (!SkipRedundantDebugValElimination) {
+    // Eliminate duplicate/redundant dbg.values. This seems to be a good place
+    // to do that since we might end up with redundant dbg.values describing the
+    // entry PHI node post-splice.
+    RemoveRedundantDbgInstrs(PredBB);
+  }
 
   // Inherit predecessors name if it exists.
   if (!PredBB->hasName())
Index: llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
===================================================================
--- llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
+++ llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
@@ -95,7 +95,8 @@
                                LoopInfo *LI = nullptr,
                                MemorySSAUpdater *MSSAU = nullptr,
                                MemoryDependenceResults *MemDep = nullptr,
-                               bool PredecessorWithTwoSuccessors = false);
+                               bool PredecessorWithTwoSuccessors = false,
+                               bool SkipRedundantDebugValElimination = false);
 
 /// Merge block(s) sucessors, if possible. Return true if at least two
 /// of the blocks were merged together.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89818.299447.patch
Type: text/x-patch
Size: 3252 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201020/0d6717f7/attachment.bin>