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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:6530
     // Only look at the first use, avoid hurting compile time with long uselists
-    User *Use = *I->user_begin();
+    auto *Use = dyn_cast<Instruction>(*I->user_begin());
+    // Bail out if Use is not in the same BB as I or Use == I or Use comes
----------------
nikic wrote:
> Can be `cast`, user of instruction is always instruction.
updated, thanks!


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:6544
+          return !isGuaranteedToTransferExecutionToSuccessor(&I);
+        }))
+      return false;
----------------
nikic wrote:
> We have isGuaranteedToTransferExecutionToSuccessor() variants that accept iterators in ValueTracking, and also enforce a ScanLimit (default 32). That should ensure that there are no pathological cases even if they are in the same block.
I did that with the following results: http://llvm-compile-time-tracker.com/compare.php?from=29c1c85c4e1dfe663bd9e7424d021283f9dc3254&to=2c6b1eed0c7f8e933e7666bc50eb390bbfccc039&stat=instructions

I'd like to keep this out of the current change though, as this may impact the generated code (there are a few binary changes even for CTMark), while the current patch should be NFC with respect to codegen.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113293/new/

https://reviews.llvm.org/D113293