[llvm] 6e9363c - [CaptureTracking] Only check reachability for capture candidates

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2021-05-15T22:57:56+02:00
New Revision: 6e9363c94230a8427c11c2d13b7c65164370ffae

URL: https://github.com/llvm/llvm-project/commit/6e9363c94230a8427c11c2d13b7c65164370ffae
DIFF: https://github.com/llvm/llvm-project/commit/6e9363c94230a8427c11c2d13b7c65164370ffae.diff

LOG: [CaptureTracking] Only check reachability for capture candidates

Reachability queries are very expensive, and currently performed
for each instruction we look at, even though most of them will
not lead to a capture and are thus ultimately irrelevant. It is
more efficient to walk a few unnecessary instructions than to
perform unnecessary reachability queries.

Theoretically, this may produce worse results, because the additional
instructions considered may cause us to hit the use count limit
earlier. In practice, this does not appear to be a problem, e.g.
on test-suite O3 we report only one more captured-before with this
change, with no resulting codegen differences.

This makes PointerMayBeCapturedBefore() significantly cheaper in
practice, hopefully allowing it to be used in more places.

Added: 
    

Modified: 
    llvm/lib/Analysis/CaptureTracking.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/CaptureTracking.cpp b/llvm/lib/Analysis/CaptureTracking.cpp
index 4ad9b4b9c7db..e14f6f25dc18 100644
--- a/llvm/lib/Analysis/CaptureTracking.cpp
+++ b/llvm/lib/Analysis/CaptureTracking.cpp
@@ -106,7 +106,8 @@ namespace {
     void tooManyUses() override { Captured = true; }
 
     bool isSafeToPrune(Instruction *I) {
-      assert(I != BeforeHere && "Should have been handled earlier");
+      if (BeforeHere == I)
+        return !IncludeI;
 
       BasicBlock *BB = I->getParent();
       // We explore this usage only if the usage can reach "BeforeHere".
@@ -152,17 +153,15 @@ namespace {
       return false;
     }
 
-    bool shouldExplore(const Use *U) override {
+    bool captured(const Use *U) override {
       Instruction *I = cast<Instruction>(U->getUser());
+      if (isa<ReturnInst>(I) && !ReturnCaptures)
+        return false;
 
-      if (BeforeHere == I)
-        return IncludeI;
-
-      return !isSafeToPrune(I);
-    }
-
-    bool captured(const Use *U) override {
-      if (isa<ReturnInst>(U->getUser()) && !ReturnCaptures)
+      // Check isSafeToPrune() here rather than in shouldExplore() to avoid
+      // an expensive reachability query for every instruction we look at.
+      // Instead we only do one for actual capturing candidates.
+      if (isSafeToPrune(I))
         return false;
 
       Captured = true;