Skip to content

Revert "Cranelift: Reassociate long and narrow chains of operations into shallow and wide trees (#7466)"#12116

Merged
fitzgen merged 2 commits into
bytecodealliance:mainfrom
fitzgen:revert-isle-rule
Dec 3, 2025
Merged

Revert "Cranelift: Reassociate long and narrow chains of operations into shallow and wide trees (#7466)"#12116
fitzgen merged 2 commits into
bytecodealliance:mainfrom
fitzgen:revert-isle-rule

Conversation

@fitzgen
Copy link
Copy Markdown
Member

@fitzgen fitzgen commented Dec 3, 2025

This reverts commit 72534b0.

This mid-end rule was shown to be the root cause of a codegen regression in #12106 and we didn't have any real examples that would improve from this rule, just a theoretical idea of its benefit. Better to do actual instruction scheduling while lowering and/or during regalloc than to do hack-y instruction scheduling in the mid-end (where we have no idea how many registers are available, for example).

…nto shallow and wide trees (bytecodealliance#7466)"

This reverts commit 72534b0.

This mid-end rule was shown to be the root cause of a codegen regression in
bytecodealliance#12106 and we didn't have any
real examples that would improve from this rule, just a theoretical idea of its
benefit. Better to do actual instruction scheduling while lowering and/or during
regalloc than to do hack-y instruction scheduling in the mid-end (where we have
no idea how many registers are available, for example).
@fitzgen fitzgen requested a review from a team as a code owner December 3, 2025 16:30
@fitzgen fitzgen requested review from alexcrichton and cfallin and removed request for a team and alexcrichton December 3, 2025 16:30
@github-actions github-actions Bot added cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language labels Dec 3, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 3, 2025

Subscribe to Label Action

cc @cfallin, @fitzgen

Details This issue or pull request has been labeled: "cranelift", "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 .github/subscribe-to-label.json configuration file.

Learn more.

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.

LGTM, thanks!

(Looks like some disas tests will need re-blessing as well to pass CI.)

@fitzgen fitzgen requested a review from a team as a code owner December 3, 2025 21:17
@fitzgen fitzgen requested review from pchickey and removed request for a team December 3, 2025 21:17
@fitzgen fitzgen enabled auto-merge December 3, 2025 21:17
@fitzgen fitzgen added this pull request to the merge queue Dec 3, 2025
Merged via the queue into bytecodealliance:main with commit 9a6aa39 Dec 3, 2025
45 checks passed
@fitzgen fitzgen deleted the revert-isle-rule branch December 3, 2025 21:54
bongjunj pushed a commit to prosyslab/wasmtime that referenced this pull request Dec 4, 2025
…nto shallow and wide trees (bytecodealliance#7466)" (bytecodealliance#12116)

* Revert "Cranelift: Reassociate long and narrow chains of operations into shallow and wide trees (bytecodealliance#7466)"

This reverts commit 72534b0.

This mid-end rule was shown to be the root cause of a codegen regression in
bytecodealliance#12106 and we didn't have any
real examples that would improve from this rule, just a theoretical idea of its
benefit. Better to do actual instruction scheduling while lowering and/or during
regalloc than to do hack-y instruction scheduling in the mid-end (where we have
no idea how many registers are available, for example).

* update disas tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants