wasmtime / PR #14015 x64: lower `isub` by a constant to `... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #14015 x64: lower `isub` by a constant to `...

Wasmtime GitHub notifications bot (Jul 29 2026 at 09:19):

darmie opened PR #14015 from darmie:x64-isub-const-lea to bytecodealliance:main:

iadd of a value and a 32/64-bit constant already lowers to lea, folding
the constant into the address displacement and computing the result in a
different register than the input without a separate mov. isub by a
constant did not have a matching rule: it fell through to the two-operand
sub, which forces a mov+sub pair whenever the destination register
differs from the source.

This adds a rule that lowers x - C (for a 32- or 64-bit integer constant
C) to lea -C(x), mirroring the existing iadd path. The rule fires only
when the negated constant fits in an Offset32 (i.e. C is not i32::MIN);
register/register isub is unchanged.

Example
function %isub_c2(i64) -> i64 {
block0(v0: i64):
    v1 = iconst.i64 2
    v2 = isub v0, v1
    return v2
}
Before:
movq %rdi, %rax
subq $2, %rax
After:
leaq -2(%rdi), %rax
One instruction instead of two, and no flags clobbered.

Testing

New precise-output filetest (isub-const-lea.clif) asserting the lea
lowering for i64 and i32, and that register/register isub still lowers
to sub.

New runtest exercising x - C (including a negative constant) on the
interpreter and every native target.

The existing isa/x64 filetests pass unchanged.

I found this while profiling a call/arithmetic-heavy workload where every
argument setup was an x - C; folding the mov away was a measurable win.

Wasmtime GitHub notifications bot (Jul 29 2026 at 09:19):

darmie requested cfallin for a review on PR #14015.

Wasmtime GitHub notifications bot (Jul 29 2026 at 09:19):

darmie requested wasmtime-compiler-reviewers for a review on PR #14015.

Wasmtime GitHub notifications bot (Jul 29 2026 at 09:30):

darmie requested wasmtime-core-reviewers for a review on PR #14015.

Wasmtime GitHub notifications bot (Jul 29 2026 at 09:30):

darmie updated PR #14015.

Wasmtime GitHub notifications bot (Jul 29 2026 at 09:30):

darmie commented on PR #14015:

Pushed a fixup: refreshed the 12 load-store/x64 disas goldens that the CLI disas test flagged. Those are dynamic-memory bounds checks that compute bound - access_size in place; with this rule that sub $C now goes through lea and degenerates to add $-C — the same instruction count, and identical to how iadd by a constant already lowers today (verified isub v, C and iadd v, -C produce byte-identical output). The win over the old mov+sub shows up when the result lands in a different register than the input.

Wasmtime GitHub notifications bot (Jul 29 2026 at 10:34):

github-actions[bot] added the label cranelift on PR #14015.

Wasmtime GitHub notifications bot (Jul 29 2026 at 10:34):

github-actions[bot] added the label isle on PR #14015.

Wasmtime GitHub notifications bot (Jul 29 2026 at 10:34):

github-actions[bot] added the label cranelift:area:x64 on PR #14015.

Wasmtime GitHub notifications bot (Jul 29 2026 at 10:35):

github-actions[bot] commented on PR #14015:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "isle"

Thus the following users have been cc'd because of the following labels:

cfallin: isle

fitzgen: isle

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Jul 29 2026 at 14:39):

cfallin commented on PR #14015:

Hi @darmie -- are you aware of the add-vs-lea performance discussion thread we had recently (#13325)? I ask because at the very least, we should benchmark this change with the full Sightglass suite. I am also pretty leery in general of introducing more uses of LEA given the performance variability that we've seen on different systems. I'm not surprised that you saw a speedup on one particular benchmark since LEA is "non-destructive" (doesn't clobber the source) but I'd be curious to know how this looks overall. Thanks!

Wasmtime GitHub notifications bot (Jul 29 2026 at 14:47):

darmie commented on PR #14015:

Hi @cfallin! Actually I am not aware of that thread, I should have searched first. Yes I saw a performance improvement on my alderlake x86_64 box, but it's only ~6.9% so far. I'll read through the thread now.

Wasmtime GitHub notifications bot (Aug 01 2026 at 08:28):

darmie updated PR #14015.

Wasmtime GitHub notifications bot (Aug 04 2026 at 09:42):

darmie commented on PR #14015:

@cfallin I have gone through the thread. From what I gather, lea performance is inconsistent on Xeon processors. Naive question: Is it possible to gate the implementation and skip Xeon machines for now?

Wasmtime GitHub notifications bot (Aug 05 2026 at 19:12):

cfallin commented on PR #14015:

We discussed this in the other thread a bit but we don't have per-microarchitecture machine models, and that is what it'd take (which is a really big project in the compiler). We don't want to have a patchwork of heuristics for things like this.

Can you clarify

Yes I saw a performance improvement on my alderlake x86_64 box, but it's only ~6.9% so far

do you mean on all of Sightglass, or on one particular benchmark?

Wasmtime GitHub notifications bot (Aug 05 2026 at 19:19):

darmie commented on PR #14015:

do you mean on all of Sightglass, or on one particular benchmark?

It's a microbench I did for my project. I simply did multiple A/B runs – that was how I arrived at that score.

Wasmtime GitHub notifications bot (Aug 05 2026 at 19:36):

cfallin commented on PR #14015:

OK, I think we'd want Sightglass runs across multiple microarchitectures that show clear benefit here before deciding to take it for sure.

Wasmtime GitHub notifications bot (Aug 05 2026 at 20:41):

darmie commented on PR #14015:

OK, I think we'd want Sightglass runs across multiple microarchitectures that show clear benefit here before deciding to take it for sure.

Yes I can do fhat. Is the Sightglass tool setup in the CI?

Wasmtime GitHub notifications bot (Aug 05 2026 at 20:49):

cfallin commented on PR #14015:

It's not in CI -- see the README at https://github.com/bytecodealliance/sightglass/ for more.

Wasmtime GitHub notifications bot (Aug 05 2026 at 20:53):

darmie commented on PR #14015:

It's not in CI -- see the README at https://github.com/bytecodealliance/sightglass/ for more.

Oh.. thanks!

Wasmtime GitHub notifications bot (Aug 09 2026 at 13:01):

darmie commented on PR #14015:

Sightglass results. The short version: this rule is a wash and I think it should be closed, but the run turned up something more interesting than the PR, so I want to put both in front of you.

Setup: wasmtime 3ebfbe5, sightglass 08180eb, all.suite (130 benchmarks), engines built from one tree differing only in the three codegen files. Ubuntu 26.04, i5-1250P (Alder Lake, family 6 model 154), pinned to the four P-cores. I couldn't disable turbo or set the governor on this box, so everything significant was re-run at 5x the samples with the engine order reversed, so drift would flip the sign instead of surviving.

This PR: no aggregate movement. Sum total across the suite is "no difference" for execution and compilation. Per benchmark, reproducing across both passes: quicksort 1.02 to 1.13x, spidermonkey-regex 1.03 to 1.05x, tinygo-json 1.02 to 1.07x and shootout-ed25519 ~1.01x faster with the rule; sqlite3, libsodium-pwhash_scrypt and shootout-sieve 1.01 to 1.02x slower. (libsodium-randombytes and shootout-memmove were significant in the first pass only, so I read those as noise.)

The rule fires heavily, so this is not a case of the suite failing to exercise it. AOT-compiling for x86_64 and counting mnemonics, spidermonkey-regex moves −13598 subl / +13215 leal / −14042 movq, and total instructions retired drop on every benchmark, including all three that got slower. sqlite3 executes 2530 fewer instructions and loses 1 to 2%.

The more useful result. To find out whether that was LEA being weak here or just this rule being marginal, I ran the counterfactual from #13325 on the same rig: main, with iadd_base_case_32_or_64_lea replaced by x64_add.

execution, 99% conf

main (with iadd→lea) faster 49 benchmarks

no-lea-for-add faster 5 benchmarks

The wins are significant: libsodium-misuse 1.30 to 1.77x, shootout-nestedloop 1.12 to 1.62x, gcc-loops 1.19x, shootout-seqhash 1.14x, meshoptimizer 1.06 to 1.11x, blake3-scalar 1.06 to 1.10x. So on this core LEA-for-add is very strongly load-bearing, and the blunt "use ADD for adds always" option would be expensive here. That's one microarchitecture, and the opposite of what @bongjunj measured on Cascade Lake, so I offer it as a data point for that thread rather than a conclusion.

That also explains why my rule is a wash while the iadd one is not, and it's a difference in kind rather than degree. to_amode_add reaches amode_imm_reg_reg_shift, so the iadd rule collapses base + index*scale + disp, plus chained iadd-by-constant, into a single LEA. My rule only ever produces amode_imm_reg, i.e. lea -C(%base): it folds nothing and buys exactly one saved mov. It captures the weakest form of the LEA benefit while paying the full microarchitectural risk, which is the wrong end of the trade.

Given that, I don't think there's a case for this change and I'm happy to close it. If it's useful I can push the no-lea-for-add data to #13325 as a separate note, and if you'd like any of this re-run on the project's benchmarking hardware, a /bench_x64 here would do it.

	execution, 99% conf
main (with `iadd`→`lea`) faster	49 benchmarks
no-`lea`-for-add faster	5 benchmarks

Wasmtime GitHub notifications bot (Aug 09 2026 at 19:16):

cfallin commented on PR #14015:

Given that, I don't think there's a case for this change and I'm happy to close it. If it's useful I can push the no-lea-for-add data to #13325 as a separate note, and if you'd like any of this re-run on the project's benchmarking hardware, a /bench_x64 here would do it.

Hi @darmie -- this response looks like it is a direct copy/paste out of an AI session, complete with a suggestion to run a slash-command. Aside from not making sense in context (it makes no sense to tell me I can run a slash-command here in a GitHub thread -- it only made sense in your agent chat), this is a direct violation of our AI tool policy, which states that you cannot use an LLM's output directly in correspondence with other humans. You yourself may use such tools, but must review the output yourself, and correspond as a human with other humans. While we appreciate contributions, we want to mentor and collaborate with people, not indirectly with their AI bots through a lossy channel. Please make sure you adhere to this policy in any future contributions.

Given the technical data, as well as this policy violation issue (and the implication that you have not deeply understood the issue yourself but are directly copy/pasting from a bot in the thread), I will go ahead and close this PR. Thanks.

Wasmtime GitHub notifications bot (Aug 09 2026 at 19:16):

:cross_mark: cfallin closed without merge PR #14015.

Wasmtime GitHub notifications bot (Aug 09 2026 at 20:35):

darmie commented on PR #14015:

Given that, I don't think there's a case for this change and I'm happy to close it. If it's useful I can push the no-lea-for-add data to #13325 as a separate note, and if you'd like any of this re-run on the project's benchmarking hardware, a /bench_x64 here would do it.

Hi @darmie -- this response looks like it is a direct copy/paste out of an AI session, complete with a suggestion to run a slash-command. Aside from not making sense in context (it makes no sense to tell me I can run a slash-command _here_ in a GitHub thread -- it only made sense in your agent chat), this is a direct violation of our AI tool policy, which states that you cannot use an LLM's output directly in correspondence with other humans. You yourself may use such tools, but must review the output yourself, and correspond as a human with other humans. While we appreciate contributions, we want to mentor and collaborate with _people_, not indirectly with their AI bots through a lossy channel. Please make sure you adhere to this policy in any future contributions.

Given the technical data, as well as this policy violation issue (and the implication that you have not deeply understood the issue yourself but are directly copy/pasting from a bot in the thread), I will go ahead and close this PR. Thanks.

@cfallin I apologize that it still reads that way even though it has been reviewed and edited by me. Yes I did use AI to gather my thoughts, but between the day you asked me to use Sightglass tool and now, I had run the benchmarks many times partly in hopes to justify this PR, but to give a clear verdict on where it stands. I am fully aware of the policy and I did honestly considered that while I also tried not to remove relevant information that you may find useful.

Last updated: Aug 30 2026 at 10:08 UTC