Skip to content

Cranelift: Break op cost ties with expression depth in egraphs#7456

Merged
fitzgen merged 5 commits into
bytecodealliance:mainfrom
fitzgen:depth-cost
Nov 7, 2023
Merged

Cranelift: Break op cost ties with expression depth in egraphs#7456
fitzgen merged 5 commits into
bytecodealliance:mainfrom
fitzgen:depth-cost

Conversation

@fitzgen
Copy link
Copy Markdown
Member

@fitzgen fitzgen commented Nov 2, 2023

This means that, when the opcode cost is the same, we prefer shallow and wide
expressions to narrow and deep. For example, (a + b) + (c + d) is preferred to
((a + b) + c) + d. This is beneficial because it exposes more
instruction-level parallelism and shortens live ranges.

Follow up PRs will add rules that take advantage of this.

Depends on #7465

@fitzgen fitzgen requested a review from a team as a code owner November 2, 2023 16:44
@fitzgen fitzgen requested review from elliottt and removed request for a team November 2, 2023 16:44
Comment thread cranelift/codegen/src/egraph/cost.rs
@github-actions github-actions Bot added the cranelift Issues related to the Cranelift code generator label Nov 2, 2023
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 to this as well, thanks! Will wait for #7465 to merge and this to rebase on main then we can merge.

Comment thread cranelift/codegen/src/egraph/cost.rs Outdated
Copy link
Copy Markdown
Member

@elliottt elliottt left a comment

Choose a reason for hiding this comment

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

🎉

Comment thread cranelift/codegen/src/egraph/cost.rs Outdated
Comment thread cranelift/codegen/src/egraph/cost.rs
Comment thread cranelift/codegen/src/egraph/cost.rs Outdated
fitzgen and others added 5 commits November 7, 2023 13:27
This means that, when the opcode cost is the same, we prefer shallow and wide
expressions to narrow and deep. For example, `(a + b) + (c + d)` is preferred to
`((a + b) + c) + d`. This is beneficial because it exposes more
instruction-level parallelism and shortens live ranges.

Co-Authored-By: Trevor Elliott <telliott@fastly.com>
Co-Authored-By: Chris Fallin <chris@cfallin.org>
Co-Authored-By: Trevor Elliott <telliott@fastly.com>
@fitzgen fitzgen enabled auto-merge November 7, 2023 22:13
@fitzgen fitzgen added this pull request to the merge queue Nov 7, 2023
Merged via the queue into bytecodealliance:main with commit b9f2a30 Nov 7, 2023
@fitzgen fitzgen deleted the depth-cost branch November 7, 2023 23:16
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants