[PATCH] D126236: [CaptureTracking] Increase limit but use it for all visited uses.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: nikic, aeubanks, reames, jdoerfert.
Herald added a subscriber: hiraditya.
Herald added a project: All.
fhahn requested review of this revision.
Herald added a project: LLVM.

Currently the MaxUsesToExplore limit only applies to the number of users
per value, not the total number of users to explore.

The current limit of 20 pessimizes IR with opaque pointers in some
cases. Without opaque pointers, we have deeper pointer def-use chains in
general due to extra bitcasts and geps for structs with index 0.

With opaque pointers the def-use chain is not as deep but wider, due to
bitcasts & 0-geps missing.

To improve the situation for opaque pointers, this patch does 2 things:

1. Apply the limit to the total number of uses visited. From the wording in the description of the option it seems like this may be the original intention. With the current implementation we could still end up walking a lot of uses.
2. Increase the limit to 100. This is quite arbitrary, but enables a good number of additional optimizations.

Those adjustments have a noticeable compile-time impact though. In part
that is likely due to additional transformations (and conversely
the current baseline misses optimizations after switching to opaque
pointers).

Limit=100:

- NewPM-O3: +0.15%
- NewPM-ReleaseThinLTO: +0.86%
- NewPM-ReleaseLTO-g: +0.44%

https://llvm-compile-time-tracker.com/compare.php?from=8bfccb963b3519393c0266b452a115a4bb46d207&to=818719fad01d472412c963629671a81a8703b25b&stat=instructions

Limit=60:

- NewPM-O3: +0.14%
- NewPM-ReleaseThinLTO: +0.41%
- NewPM-ReleaseLTO-g: +0.21%

https://llvm-compile-time-tracker.com/compare.php?from=aeb19817d66f1a15754163c7f48e01e9ebdd6d45&to=520563fdc146319aae90d06f88d87f2e9e1247b7&stat=instructions

Limit=40:

- NewPM-O3: +0.11%
- NewPM-ReleaseThinLTO: +0.12%
- NewPM-ReleaseLTO-g: +0.09%

https://llvm-compile-time-tracker.com/compare.php?from=aeb19817d66f1a15754163c7f48e01e9ebdd6d45&to=c9182576e9fe3f1c84a71479665aef91a416318c&stat=instructions

I'll add a test if/once we converge on agreement. I'd be more than happy to
discuss alternatives as well


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126236

Files:
  llvm/lib/Analysis/CaptureTracking.cpp


Index: llvm/lib/Analysis/CaptureTracking.cpp
===================================================================
--- llvm/lib/Analysis/CaptureTracking.cpp
+++ llvm/lib/Analysis/CaptureTracking.cpp
@@ -45,9 +45,9 @@
 /// use it where possible. The caching version can use much higher limit or
 /// don't have this cap at all.
 static cl::opt<unsigned>
-DefaultMaxUsesToExplore("capture-tracking-max-uses-to-explore", cl::Hidden,
-                        cl::desc("Maximal number of uses to explore."),
-                        cl::init(20));
+    DefaultMaxUsesToExplore("capture-tracking-max-uses-to-explore", cl::Hidden,
+                            cl::desc("Maximal number of uses to explore."),
+                            cl::init(40));
 
 unsigned llvm::getDefaultMaxUsesToExploreForCaptureTracking() {
   return DefaultMaxUsesToExplore;
@@ -444,8 +444,8 @@
   Worklist.reserve(getDefaultMaxUsesToExploreForCaptureTracking());
   SmallSet<const Use *, 20> Visited;
 
+  unsigned Count = 0;
   auto AddUses = [&](const Value *V) {
-    unsigned Count = 0;
     for (const Use &U : V->uses()) {
       // If there are lots of uses, conservatively say that the value
       // is captured to avoid taking too much compile time.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126236.431458.patch
Type: text/x-patch
Size: 1232 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220523/e199406e/attachment.bin>