Skip to content

x64: Finish migrating brz and brnz to ISLE#4614

Merged
elliottt merged 3 commits intobytecodealliance:mainfrom
elliottt:trevor/x64-isle-brz-brnz
Aug 4, 2022
Merged

x64: Finish migrating brz and brnz to ISLE#4614
elliottt merged 3 commits intobytecodealliance:mainfrom
elliottt:trevor/x64-isle-brz-brnz

Conversation

@elliottt
Copy link
Copy Markdown
Member

@elliottt elliottt commented Aug 4, 2022

Finish migrating the last case for brz and brnz lowering to ISLE in the x64 backend.

@elliottt elliottt changed the title Finish migrating brz and brnz to ISLE x64: Finish migrating brz and brnz to ISLE Aug 4, 2022
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:x64 Issues related to x64 codegen isle Related to the ISLE domain-specific language labels Aug 4, 2022
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 4, 2022

Subscribe to Label Action

cc @cfallin, @fitzgen

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

@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.

r=me with nitpick below resolved

Comment on lines +3279 to +3282
;; A type guard for matching 64-bit sized ints, bools, or references but
;; explicitly ruling out 32-bit references.
(decl ty_int_bool_or_ref () Type)
(extern extractor ty_int_bool_or_ref ty_int_bool_or_ref)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nitpick: I feel like the name could have 64 in it somewhere since this explicitly doesn't handle 32-bit ints/bools/refs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wait looking at the implementation, this accepts 32-bit ints/bools, jsut not 32-bit refs. Which I think is fine, but I think means the comment here is misleading.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The function I ported over seemed to accept a pretty wide range of types, definitely open to better naming suggestions :)

@elliottt elliottt marked this pull request as ready for review August 4, 2022 18:00
@elliottt elliottt merged commit dc8362c into bytecodealliance:main Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift:area:x64 Issues related to x64 codegen 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