Skip to content

egraph: peer through ireduce in brif (ctz/clz) skeleton fold#13458

Merged
cfallin merged 3 commits into
bytecodealliance:mainfrom
ggreif:gabor/ireduce-ctz-clz-peephole
May 23, 2026
Merged

egraph: peer through ireduce in brif (ctz/clz) skeleton fold#13458
cfallin merged 3 commits into
bytecodealliance:mainfrom
ggreif:gabor/ireduce-ctz-clz-peephole

Conversation

@ggreif
Copy link
Copy Markdown
Contributor

@ggreif ggreif commented May 22, 2026

Extends #13343's simplify_skeleton fold so the existing
brif (ctz X) / brif (clz X) rules also fire when an ireduce
(i.e. i32.wrap_i64 at the wasm level) sits between the count and
the branch.

Why this matters: non-Rust frontends targeting wasm64 often emit
i64.ctz; i32.wrap_i64; br_if for boolean-context LSB tests. The
wrap is harmless (the count is in [0, 64]) but it currently blocks
the fold, so cranelift falls back to materializing the full count
before the branch. With this PR, the i64-wrap form lowers to the
same testq $1, %rdx; je shape as the i32 case.

Test coverage mirrors the existing patterns:

  • cranelift/filetests/.../brif-cnt-cond.clif gets two new
    brif (ireduce (ctz/clz X)) functions.
  • tests/disas/ctz-clz-bool-condition.wat gets a new
    $if_clz_bare_i64 and the existing $if_ctz_bare_i64
    is reblessed - both now lower to the optimal shape.

Extends the simplify_skeleton fold from bytecodealliance#13343 to peer through an
intervening `ireduce`. The wasm pattern `i64.ctz; i32.wrap_i64; br_if`
(and the `clz` analog) lowers to clif as `brif (ireduce (ctz X))`,
which the existing 2-op rules didn't catch.

`ireduce` is harmless on a ctz/clz result because the count is in
[0, bitwidth] - always fits in the narrower type without loss. The
new rules rewrite to the same bit-extract condition as the bare-form
rules, using the wider source type for the band/sge.

Source pattern: motoko/moc's EOP peephole emits exactly this shape
for `(value & 1) == 0` as a branch condition.

Test coverage:
- cranelift filetest: brif_ireduce_ctz_i64 / brif_ireduce_clz_i64
- tests/disas: \$if_ctz_bare_i64 reblessed (testq \$1,%rdx; je) +
  new \$if_clz_bare_i64 (testq %rdx,%rdx; jge)
@ggreif ggreif requested review from a team as code owners May 22, 2026 20:00
@ggreif ggreif requested review from cfallin and removed request for a team May 22, 2026 20:00
@ggreif ggreif changed the title egraph: peer through ireduce in brif (ctz/clz) skeleton fold egraph: peer through ireduce in brif (ctz/clz) skeleton fold May 22, 2026
Copy link
Copy Markdown
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Thanks! New rule looks good; just some style comments.

Comment thread cranelift/codegen/src/opts/icmp.isle Outdated
Comment thread cranelift/codegen/src/opts/icmp.isle Outdated
Comment thread cranelift/filetests/filetests/egraph/brif-cnt-cond.clif Outdated
ggreif added 2 commits May 23, 2026 00:22
- Shorten the inline comment on the ireduce-peer rules.
- Rename the bound value `X` -> `x` to match the project's lowercase
  binding style.
- Drop the verbose justification comment on the new filetest functions;
  match the one-liner style used by the existing tests in the file.

No behavioural change; filetest still passes.
Function name is self-explanatory; rest of the file leaves the
analogous eq0/ne0/select variants uncommented.
@cfallin cfallin added this pull request to the merge queue May 22, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 22, 2026
@cfallin cfallin added this pull request to the merge queue May 22, 2026
Merged via the queue into bytecodealliance:main with commit fa50c6a May 23, 2026
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants