[PATCH] D113293: [SimplifyCFG] Add early bailout if Use is not in same BB.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: spatel, RKSimon, mkazantsev, lebedev.ri.
Herald added a subscriber: hiraditya.
fhahn requested review of this revision.
Herald added a project: LLVM.

Without this patch, passingValueIsAlwaysUndefined will iterate over all
instructions from I to the end of the basic block, even if the use is
outside the block.

This patch adds an early bail out, if the use instruction is outside I's
BB. This can greatly reduce compile-time in cases where very large basic
blocks are involved, with a large number of PHI nodes and incoming
values.

Note that the refactoring makes the handling of the case where I is a
phi and Use is in PHI more explicit  as well: for phi nodes, we can also
directly bail out. In the existing code, we would iterate until we reach
the end and return false.

I could add a test case that shows the compile-time improvement, but it
would be quite large.

Based on an earlier patch by Matt Wala.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113293

Files:
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp


Index: llvm/lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -6529,17 +6529,22 @@
     // Only look at the first use, avoid hurting compile time with long uselists
     User *Use = *I->user_begin();
 
+    auto *UseI = dyn_cast<Instruction>(Use);
+    // Bail out if Use is not in the same BB as I or UseI == I or UseI comes
+    // before I in the block. The latter two can be the case if UseI is a PHI
+    // node.
+    if (!UseI || UseI->getParent() != I->getParent() || UseI == I ||
+        UseI->comesBefore(I))
+      return false;
+
     // Now make sure that there are no instructions in between that can alter
     // control flow (eg. calls)
-    for (BasicBlock::iterator
-             i = ++BasicBlock::iterator(I),
-             UI = BasicBlock::iterator(dyn_cast<Instruction>(Use));
-         i != UI; ++i) {
-      if (i == I->getParent()->end())
-        return false;
-      if (!isGuaranteedToTransferExecutionToSuccessor(&*i))
-        return false;
-    }
+    auto InstrRange =
+        make_range(std::next(I->getIterator()), UseI->getIterator());
+    if (any_of(InstrRange, [](Instruction &I) {
+          return !isGuaranteedToTransferExecutionToSuccessor(&I);
+        }))
+      return false;
 
     // Look through GEPs. A load from a GEP derived from NULL is still undefined
     if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Use))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113293.385101.patch
Type: text/x-patch
Size: 1525 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211105/845e839f/attachment.bin>