[InstCombine] add control for SimplifyDemandedVectorElts depth limit

[Prince781]

Allows customizing the depth of the recursive search on vectors that InstCombine does when looking for unused elements.

We find it helpful to be able to customize this for compile time reasons.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Princeton Ferro (Prince781)

Changes

Allows customizing the depth of the recursive search on vectors that InstCombine does when looking for unused elements.

We find it helpful to be able to customize this for compile time reasons.

---

Full diff: https://github.com/llvm/llvm-project/pull/113717.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp (+7-1)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index 8ca705ae1d364d..5eb807dcb76cef 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -30,6 +30,12 @@ static cl::opt<bool>
                              "SimplifyDemandedBits() are consistent"),
                     cl::Hidden, cl::init(false));
 
+static cl::opt<unsigned> SimplifyDemandedVectorEltsDepthLimit(
+    "instcombine-simplify-vector-elts-depth",
+    cl::desc(
+        "Depth limit when simplifying vector instructions and their operands"),
+    cl::Hidden, cl::init(10));
+
 /// Check to see if the specified operand of the specified instruction is a
 /// constant integer. If so, check to see if there are any bits set in the
 /// constant that are not demanded. If so, shrink the constant and return true.
@@ -1432,7 +1438,7 @@ Value *InstCombinerImpl::SimplifyDemandedVectorElts(Value *V,
   }
 
   // Limit search depth.
-  if (Depth == 10)
+  if (Depth == SimplifyDemandedVectorEltsDepthLimit)
     return nullptr;
 
   if (!AllowMultipleUsers) {

[goldsteinn]

LGTM although there may be some contention about adding another CLI option, so wait a few days to push to gives others a chance to comment.

[Prince781]

Ping.

[nikic]

What is the depth cutoff that you plan to use? And do you have a sharable test case demonstrating the compile-time issue?

I'm not opposed to having the option, but at the same time it would be nice to address the issue upstream, as other people may also encounter the issue.

[Prince781]

Hi @nikic,

What is the depth cutoff that you plan to use?

1, although we may have other values we'd like to use.

And do you have a sharable test case demonstrating the compile-time issue?

I'm not able to share anything yet, unfortunately. We have a few internal kernels that see a lot of improvement when this aspect of InstCombine is dialed down.

I'm not opposed to having the option, but at the same time it would be nice to address the issue upstream, as other people may also encounter the issue.

I agree. I'll actually be sending a few more patches to improve InstCombine compile time with vector operations soon.