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: Add f16const and f128const instructions #8893

Merged
merged 2 commits into from
Jul 17, 2024
Merged

cranelift: Add f16const and f128const instructions #8893

merged 2 commits into from
Jul 17, 2024

Conversation

beetrees
Copy link
Contributor

@beetrees beetrees commented Jul 2, 2024

This PR adds f16const and f128const instructions, as well as basic neg/abs/copysign constant propagation. The f128const instruction uses a Constant (like vconst) instead of directly storing the value in the instruction as InstructionData has a size of only 16 bytes.

Issue: #8312

@beetrees beetrees requested a review from a team as a code owner July 2, 2024 00:12
@beetrees beetrees requested review from elliottt and removed request for a team July 2, 2024 00:12
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:meta Everything related to the meta-language. isle Related to the ISLE domain-specific language labels Jul 2, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 2, 2024

Subscribe to Label Action

cc @cfallin, @fitzgen

This issue or pull request has been labeled: "cranelift", "cranelift:area:machinst", "cranelift:meta", "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.

afonso360
afonso360 approved these changes Jul 4, 2024
View reviewed changes
Copy link
Contributor

@afonso360 afonso360 left a comment

Choose a reason for hiding this comment

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

I don't know if @elliottt wants to take a look as well, but this all looks correct to me!

@beetrees
Copy link
Contributor Author

beetrees commented Jul 4, 2024

I've just made a minor improvement to make the f128 cprop tests more readable.

elliottt
elliottt approved these changes Jul 17, 2024
View reviewed changes
Copy link
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.

Sorry for the delay, this looks good to me!

@elliottt elliottt added this pull request to the merge queue Jul 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 17, 2024
@elliottt elliottt added this pull request to the merge queue Jul 17, 2024
Merged via the queue into bytecodealliance:main with commit 41eca60 Jul 17, 2024
66 checks passed
@beetrees beetrees deleted the f16-f128-const branch July 17, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afonso360 afonso360 afonso360 approved these changes

@elliottt elliottt elliottt approved these changes

Assignees
No one assigned
Labels
cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:meta Everything related to the meta-language. 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.
3 participants
@beetrees @elliottt @afonso360