[PATCH] D75032: [MachineVerifier] Doing ::calcRegsPassed in RPO: ~35% faster MV

[Roman Tereshin via Phabricator via llvm-commits]

rtereshin created this revision.
rtereshin added reviewers: bogner, stoklund, rudkx, qcolombet.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

Depending on the target, test suite, pipeline config and perhaps other
factors machine verifier when forced on with -verify-machineinstrs can
increase compile time 2-2.5 times over (Release, Asserts On), taking up
~60% of the time. An invaluable tool, it significantly slows down
machine verifier-enabled testing.

Nearly 75% of its time MachineVerifier spends in the calcRegsPassed
method. It's a classic forward dataflow analysis executed over sets, but
visiting MBBs in arbitrary order. We switch that to RPO here.

This speeds up MachineVerifier by about 35%, decreasing the overall
compile time with -verify-machineinstrs by 20-25% or so.

calcRegsPassed itself gets 2x faster here.

All measured on a large suite of shaders targeting a number of GPUs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75032

Files:
  llvm/lib/CodeGen/MachineVerifier.cpp


Index: llvm/lib/CodeGen/MachineVerifier.cpp
===================================================================
--- llvm/lib/CodeGen/MachineVerifier.cpp
+++ llvm/lib/CodeGen/MachineVerifier.cpp
@@ -24,6 +24,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/DepthFirstIterator.h"
+#include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetOperations.h"
 #include "llvm/ADT/SmallPtrSet.h"
@@ -2126,34 +2127,48 @@
 // can pass through an MBB live, but may not be live every time. It is assumed
 // that all vregsPassed sets are empty before the call.
 void MachineVerifier::calcRegsPassed() {
+  // This is a forward dataflow, doing it in RPO. A standard map serves as a
+  // priority (by RPO number) queue, deduplicating worklist, and an RPO number
+  // to MBB mapping all at once with the same as alternatives' time complexity.
+  // Constant factor could be on a bigger side, but this map takes only ~0.5%
+  // of total machine verifier time, so I'm sure we're fine.
+  std::map<unsigned, const MachineBasicBlock *> RPOWorklist;
+  DenseMap<const MachineBasicBlock *, unsigned> RPONumbers;
+  if (MF->empty()) {
+    // ReversePostOrderTraversal doesn't handle empty functions.
+    return;
+  }
+  for (const MachineBasicBlock *MBB :
+       ReversePostOrderTraversal<const MachineFunction *>(MF)) {
+    // Careful with the evaluation order, fetch next number before allocating.
+    unsigned Number = RPONumbers.size();
+    RPONumbers[MBB] = Number;
+  }
   // First push live-out regs to successors' vregsPassed. Remember the MBBs that
   // have any vregsPassed.
-  SmallPtrSet<const MachineBasicBlock*, 8> todo;
-  for (const auto &MBB : *MF) {
+  for (const MachineBasicBlock &MBB : *MF) {
     BBInfo &MInfo = MBBInfoMap[&MBB];
     if (!MInfo.reachable)
       continue;
-    for (MachineBasicBlock::const_succ_iterator SuI = MBB.succ_begin(),
-           SuE = MBB.succ_end(); SuI != SuE; ++SuI) {
-      BBInfo &SInfo = MBBInfoMap[*SuI];
+    for (const MachineBasicBlock *Succ : MBB.successors()) {
+      BBInfo &SInfo = MBBInfoMap[Succ];
       if (SInfo.addPassed(MInfo.regsLiveOut))
-        todo.insert(*SuI);
+        RPOWorklist.emplace(RPONumbers[Succ], Succ);
     }
   }
 
-  // Iteratively push vregsPassed to successors. This will converge to the same
-  // final state regardless of DenseSet iteration order.
-  while (!todo.empty()) {
-    const MachineBasicBlock *MBB = *todo.begin();
-    todo.erase(MBB);
+  // Iteratively push vregsPassed to successors.
+  while (!RPOWorklist.empty()) {
+    auto Next = RPOWorklist.begin();
+    const MachineBasicBlock *MBB = Next->second;
+    RPOWorklist.erase(Next);
     BBInfo &MInfo = MBBInfoMap[MBB];
-    for (MachineBasicBlock::const_succ_iterator SuI = MBB->succ_begin(),
-           SuE = MBB->succ_end(); SuI != SuE; ++SuI) {
-      if (*SuI == MBB)
+    for (const MachineBasicBlock *Succ : MBB->successors()) {
+      if (Succ == MBB)
         continue;
-      BBInfo &SInfo = MBBInfoMap[*SuI];
+      BBInfo &SInfo = MBBInfoMap[Succ];
       if (SInfo.addPassed(MInfo.vregsPassed))
-        todo.insert(*SuI);
+        RPOWorklist.emplace(RPONumbers[Succ], Succ);
     }
   }
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75032.246153.patch
Type: text/x-patch
Size: 3241 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200224/08c58451/attachment.bin>