Skip to content

[InstSimplify] Check call args are constant first to save ~0.04% instrs - #240676

Open
rnk wants to merge 5 commits into
llvm:mainfrom
rnk:perf/instsimplify-constant-args-first
Open

rnk wants to merge 5 commits into
llvm:mainfrom
rnk:perf/instsimplify-constant-args-first

Conversation

@rnk

@rnk rnk commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

tryConstantFoldCall runs two checks before continuing to fold:

  1. It calls canConstantFoldCallTo, which checks if the call is marked nobuiltin and that it is a known library call (cached)
  2. It scans the arguments and bails out if any of them are non-Constant

It turns out that querying attributes and TLI is more expensive than scanning for Constant arguments, so do that first. This patch swaps the order so the common case exits early.

Compile-time tracker: https://llvm-compile-time-tracker.com/compare.php?from=6e58f6f5536016c55365ac55be11e6ac783d64cf&to=ebc6a8a748f0d3338efc8c2633f057418ebca43a&stat=instructions:u

This is worth ~0.15% on my workload, but only ~0.04% on the CTT Clang build. Again, I think this might show that my workload is intrinsic-call heavy, but I haven't confirmed that yet.

LLM-aided

rnk added 2 commits November 10, 2026 03:25
Checking for constant call arguments before calling canConstantFoldCallTo
reduces opt's user instruction count by about 0.15% on a call-heavy
InstCombine microbenchmark, with no change in output.

tryConstantFoldCall asked canConstantFoldCallTo whether the callee is
foldable before checking whether all arguments are constant. The former
queries call-site and callee attributes (nobuiltin, strictfp) and may
consult TargetLibraryInfo, while the latter is a cheap scan that fails for
most calls. Swap the order so the common case exits early.

LLM-aided
@rnk
rnk marked this pull request as ready for review November 10, 2026 17:54
@rnk
rnk requested a review from nikic as a code owner November 10, 2026 17:54
@llvmorg-github-actions llvmorg-github-actions Bot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:analysis Includes value tracking, cost tables and constant folding labels Nov 10, 2026
@llvmorg-github-actions

Copy link
Copy Markdown

@llvm/pr-subscribers-llvm-analysis

Author: Reid Kleckner (rnk)

Changes

tryConstantFoldCall runs two checks before continuing to fold:

  1. It calls canConstantFoldCallTo, which checks if the call is marked nobuiltin and that it is a known library call (cached)
  2. It scans the arguments and bails out if any of them are non-Constant

It turns out that querying attributes and TLI is more expensive than scanning for Constant arguments, so do that first. This patch swaps the order so the common case exits early.

Compile-time tracker: https://llvm-compile-time-tracker.com/compare.php?from=6e58f6f5536016c55365ac55be11e6ac783d64cf&to=ebc6a8a748f0d3338efc8c2633f057418ebca43a&stat=instructions:u

This is worth ~0.15% on my workload, but only ~0.04% on the CTT Clang build. Again, I think this might show that my workload is intrinsic-call heavy, but I haven't confirmed that yet.

LLM-aided


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

1 Files Affected:

  • (modified) llvm/lib/Analysis/InstructionSimplify.cpp (+7-1)
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index c85cfac11110c..cfadaa15cb80b 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -7751,9 +7751,11 @@ static Value *simplifyIntrinsic(CallBase *Call, ArrayRef<Value *> Args,
 static Value *tryConstantFoldCall(CallBase *Call, ArrayRef<Value *> Args,
                                   const SimplifyQuery &Q) {
   auto *F = Call->getCalledFunction();
-  if (!F || !canConstantFoldCallTo(Call, F, Q.TLI))
+  if (!F)
     return nullptr;
 
+  // Check for constant arguments before querying canConstantFoldCallTo.
+  // Checking arguments is cheaper than TLI classification and attribute checks.
   SmallVector<Constant *, 4> ConstantArgs;
   ConstantArgs.reserve(Args.size());
   for (Value *Arg : Args) {
@@ -7766,6 +7768,10 @@ static Value *tryConstantFoldCall(CallBase *Call, ArrayRef<Value *> Args,
     ConstantArgs.push_back(C);
   }
 
+  // Check if this is a known library function that we know how to fold.
+  if (!canConstantFoldCallTo(Call, F, Q.TLI))
+    return nullptr;
+
   return ConstantFoldCall(Call, F, ConstantArgs, Q.TLI);
 }
 

@rnk rnk changed the title [InstSimplify] Check call args are constant first to save ~0.15% instrs [InstSimplify] Check call args are constant first to save ~0.04% instrs Nov 10, 2026

@boomanaiden154 boomanaiden154 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rnk

rnk commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks!

@rnk

rnk commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

Hm, still getting these errors:
requests.exceptions.ReadTimeout: HTTPConnectionPool(host='premerge-advisor.premerge-advisor.svc.cluster.local', port=5000): Read timed out. (read timeout=5)

Should I revisit that workflow PR I sent a while back?

@rnk
rnk enabled auto-merge (squash) November 11, 2026 00:12
@rnk

rnk commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

Merged main to re-run premerge checks and auto-merging, we'll see if it fixes itself

@boomanaiden154

Copy link
Copy Markdown
Contributor

Should I revisit that workflow PR I sent a while back?

LLVM.CodeGen/BPF/adjust-opt-icmp7.ll is the test that is actually failing. This is the same situation as before where premerge advisor infrastructure failures are completely non-blocking and also have no impact on the check status.

@nikic nikic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks fine, but you might want to try reordering checks inside canCosntantFoldCallTo instead?

I'm thinking check the intrinsic case first (as it doesn't need any other the other checks). Then check TLI, which is cached. And then check the attributes.

@rnk

rnk commented Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

Generally looks fine, but you might want to try reordering checks inside canCosntantFoldCallTo instead?

I'm thinking check the intrinsic case first (as it doesn't need any other the other checks). Then check TLI, which is cached. And then check the attributes.

I looked, and canConstantFoldCallTo already checks for intrinsic ids first. Perhaps the expensive part is the FP argument check, which comes first. I will poke at it and open another PR if I find anything worthwhile, but I'll bank these savings for now.

Also, I misspoke, it turns out the workload I was profiling did not have many intrinsic calls, it had many standard direct call sites to opaque functions. I think the original source for this workload is calling shader-library supplied runtime libraries, but as far as LLVM is concerned, they are standard functions.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants