Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cranelift/egraphs: more select/bitselect optimizations #6214

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

cranelift/egraphs: more select/bitselect optimizations #6214

wants to merge 1 commit into from

Conversation

jameysharp
Copy link
Contributor

I've been thinking about the algebraic relationships between bmask and other instructions due to recent PRs, and wrote down a few of them here.

These patterns don't seem to appear in any of the Sightglass bz2, pulldown-cmark, and spidermonkey benchmarks, as this PR has no effect on execution-time instructions retired for those benchmarks.

@jameysharp
cranelift/egraphs: more select/bitselect optimizations
8d28cc1 
I've been thinking about the algebraic relationships between `bmask` and
other instructions due to recent PRs, and wrote down a few of them here.

These patterns don't seem to appear in any of the Sightglass `bz2`,
`pulldown-cmark`, and `spidermonkey` benchmarks, as this PR has no
effect on execution-time instructions retired for those benchmarks.
@jameysharp jameysharp requested a review from a team as a code owner April 14, 2023 21:40
@jameysharp jameysharp requested review from abrown and removed request for a team April 14, 2023 21:40
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Apr 14, 2023
fitzgen
fitzgen reviewed May 9, 2023
View reviewed changes
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

I think we want to be really careful about including select_spectre_guard optimizations in here. Might want to pull them out to their own file where we can argue why they are speculation safe.

Also these new optimization cases need tests.

@github-actions github-actions bot added the isle Related to the ISLE domain-specific language label May 10, 2023
@github-actions
Copy link

Subscribe to Label Action

cc @cfallin, @fitzgen

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.

@jameysharp jameysharp mentioned this pull request Aug 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fitzgen fitzgen fitzgen left review comments

@abrown abrown Awaiting requested review from abrown abrown is a code owner automatically assigned from bytecodealliance/wasmtime-compiler-reviewers

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jameysharp @fitzgen