[llvm] [EarlyIfConversion] Extend data dependent analysis across multiple blocks (PR #180623)

[Nathan Corbyn via llvm-commits]

================
@@ -925,31 +939,126 @@ static bool isConstantPoolLoad(const MachineInstr *MI) {
          });
 }
 
-/// Check if there are any calls in the range (From, To].
-static bool callInRange(const MachineInstr *From, const MachineInstr *To) {
-  constexpr int MaxInstructionsToCheck = 64;
-  int Count = 0;
-  auto InstrRange =
-      make_range(std::next(From->getIterator()), To->getIterator());
-  return any_of(InstrRange, [&](const MachineInstr &MI) {
-    return ++Count > MaxInstructionsToCheck || MI.isCall();
-  });
+/// Check if a call can be executed between From (where a value is loaded) and
+/// To (the condition). This is done by first scanning the instructions within
+/// From and To MBBs. If no call is found, we then scan all blocks which are
+/// dominated by From (the load) and can reach To (the condition).
+bool EarlyIfConverter::callInRange(const MachineInstr *From,
+                                   const MachineInstr *To) {
+  if (From == To)
+    return false;
+
+  assert(DomTree->dominates(From, To) && "From is expected to dominate To");
+
+  const MachineBasicBlock *FromBB = From->getParent();
+  const MachineBasicBlock *ToBB = To->getParent();
+
+  unsigned NumScanned = 0;
+  auto UpdateSearchCount = [](unsigned &NumScanned, unsigned N) {
+    NumScanned += N;
+    if (NumScanned <= MaxRegionInstrs)
+      return false;
+    LLVM_DEBUG(dbgs() << "  callInRange scanned more than " << MaxRegionInstrs
+                      << " instructions\n");
+    return true;
+  };
+  auto IsCallOrHitSearchLimit = [&UpdateSearchCount](const MachineInstr &MI,
+                                                     unsigned &NumScanned) {
+    if (UpdateSearchCount(NumScanned, 1))
+      return true;
+    if (!MI.isCall())
+      return false;
+    LLVM_DEBUG(dbgs() << "  found a call before the condition: " << MI);
+    return true;
+  };
+
+  // If From and To are in the same block, just check (From, To).
+  if (FromBB == ToBB) {
+    for (const MachineInstr &MI :
+         make_range(std::next(From->getIterator()), To->getIterator()))
+      if (IsCallOrHitSearchLimit(MI, NumScanned))
+        return true;
+    return false;
+  }
+
+  // Check (From, end of From's block] and [start of To's block, To).
+  for (const MachineInstr &MI :
+       make_range(std::next(From->getIterator()), FromBB->instr_end()))
+    if (IsCallOrHitSearchLimit(MI, NumScanned))
+      return true;
+  for (const MachineInstr &MI :
+       make_range(ToBB->instr_begin(), To->getIterator()))
+    if (IsCallOrHitSearchLimit(MI, NumScanned))
+      return true;
+
+  // Enqueued guards the traversal: the endpoint blocks are traversed through
+  // but their instructions were already handled above.
+  SmallPtrSet<const MachineBasicBlock *, 16> Enqueued = {ToBB};
+  SmallVector<const MachineBasicBlock *, 16> Worklist;
+  auto Enqueue = [&](const MachineBasicBlock *BB) {
+    if (Enqueued.insert(BB).second)
+      Worklist.push_back(BB);
+  };
+
+  for (const MachineBasicBlock *Pred : ToBB->predecessors())
+    Enqueue(Pred);
+
+  while (!Worklist.empty()) {
+    // Count every block we visit towards the limit, even if
+    // this block is not dominated by From. On very wide CFG's this could
+    // potentially cause us to miss cases where the load is very close to the
+    // condition in terms of maximum instructions, but because we walk many
+    // short predecessor basic blocks we will bail out of the scan before
+    // hitting it.
+    if (UpdateSearchCount(NumScanned, 1))
+      return true;
----------------
cofibrant wrote:

I'm not entirely clear what this is modelling. If we only enqueue the predecessors that from dominates, we can maintain the invariant that `From` dominates all of the blocks in the worklist. In conjunction with making sure that `From` is enqueued, this will remove the need for the check below and avoid the trap the comment above describes.

https://github.com/llvm/llvm-project/pull/180623