[PATCH] D87297: [GlobalISel] Add bailout thresholds to CSEMIRBuilder::dominates() and the localizer.

[Amara Emerson via Phabricator via llvm-commits]

aemerson created this revision.
aemerson added reviewers: paquette, aditya_nandakumar, arsenm, qcolombet, dsanders.
aemerson added a project: LLVM.
Herald added subscribers: hiraditya, rovka.
aemerson requested review of this revision.
Herald added a subscriber: wdng.

dominates(A, B) tries to determine dominance by walking an iterator from the beginning of the block potentially until the end. In extremely large BBs this can result in very long compile times, since this is called often from the artifact combiner.

The localizer's intra-block localization phase does a similar thing and walks through each instruction in the block top-down, checking if next instruction is the user set of the localizing instruction. Add a bailout threshold here too.

Longer term we may want to explore algorithmic changes to the code to alleviate these issues, perhaps by using lazy instruction numbering within blocks.

I arrived at these thresholds by building the test suite including SPEC2006, and choosing thresholds that were high enough that they wouldn't fire, going one order of magnitude higher.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87297

Files:
  llvm/include/llvm/CodeGen/GlobalISel/CSEMIRBuilder.h
  llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
  llvm/lib/CodeGen/GlobalISel/Localizer.cpp


Index: llvm/lib/CodeGen/GlobalISel/Localizer.cpp
===================================================================
--- llvm/lib/CodeGen/GlobalISel/Localizer.cpp
+++ llvm/lib/CodeGen/GlobalISel/Localizer.cpp
@@ -14,6 +14,7 @@
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/TargetLowering.h"
+#include "llvm/IR/Constants.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/Debug.h"
 
@@ -21,6 +22,12 @@
 
 using namespace llvm;
 
+static cl::opt<unsigned>
+    IntraBlockScanThreshold("localizer-intrablock-scan-threshold",
+                            cl::desc("Max number of unrelated insts to scan "
+                                     "over during intra-block localization."),
+                            cl::init(100000), cl::Hidden);
+
 char Localizer::ID = 0;
 INITIALIZE_PASS_BEGIN(Localizer, DEBUG_TYPE,
                       "Move/duplicate certain instructions close to their use",
@@ -149,9 +156,22 @@
       continue;
 
     MachineBasicBlock::iterator II(MI);
+    // Once again don't try to scan over every instruction past a threshold.
+    // For some pathological cases with extremely large basic blocks, don't try
+    // to scan beyond a certain threshold, since this is potentially O(N^2).
+    unsigned Count = 0;
+    bool ExitedEarly = false;
     ++II;
-    while (II != MBB.end() && !Users.count(&*II))
+    Count = 0;
+    while (II != MBB.end() && !Users.count(&*II)) {
+      if (++Count > IntraBlockScanThreshold) {
+        ExitedEarly = true;
+        break;
+      }
       ++II;
+    }
+    if (ExitedEarly)
+      continue;
 
     LLVM_DEBUG(dbgs() << "Intra-block: moving " << *MI << " before " << *&*II
                       << "\n");
Index: llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
===================================================================
--- llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
+++ llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
@@ -17,6 +17,12 @@
 
 using namespace llvm;
 
+static cl::opt<unsigned>
+    DominatesSearchThreshold("csemib-dom-threshold",
+                             cl::desc("Max number of instructions to scan for "
+                                      "CSEMIRBuilder inst dominance checks"),
+                             cl::init(10000), cl::Hidden);
+
 bool CSEMIRBuilder::dominates(MachineBasicBlock::const_iterator A,
                               MachineBasicBlock::const_iterator B) const {
   auto MBBEnd = getMBB().end();
@@ -26,8 +32,11 @@
          "Iterators should be in same block");
   const MachineBasicBlock *BBA = A->getParent();
   MachineBasicBlock::const_iterator I = BBA->begin();
-  for (; &*I != A && &*I != B; ++I)
-    ;
+  for (unsigned Count = 0; &*I != A && &*I != B; ++I, ++Count) {
+    if (Count > DominatesSearchThreshold)
+      return false; // Conservatively return false.
+  }
+
   return &*I == A;
 }
 
Index: llvm/include/llvm/CodeGen/GlobalISel/CSEMIRBuilder.h
===================================================================
--- llvm/include/llvm/CodeGen/GlobalISel/CSEMIRBuilder.h
+++ llvm/include/llvm/CodeGen/GlobalISel/CSEMIRBuilder.h
@@ -32,7 +32,10 @@
 class CSEMIRBuilder : public MachineIRBuilder {
 
   /// Returns true if A dominates B (within the same basic block).
-  /// Both iterators must be in the same basic block.
+  /// Both iterators must be in the same basic block. Note that if there are
+  /// more than a set threshold of instructions scanned during computation,
+  /// this will return false and exit early. Therefore, returning false does
+  /// not necessarily imply that A does *not* dominate B.
   //
   // TODO: Another approach for checking dominance is having two iterators and
   // making them go towards each other until they meet or reach begin/end. Which


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87297.290523.patch
Type: text/x-patch
Size: 3811 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200908/612affda/attachment.bin>