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.

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

riscv64: Remove support for fixed offset jumps from Jump instructions #6988

Merged
merged 6 commits into from
Sep 11, 2023

Conversation

afonso360
Copy link
Contributor

👋 Hey,

This PR removes fixed offset jumps from some instructions in the RISC-V backend. Fixed offset jumps were replaced with labels in #6955. But we still have support for them.

Currently all branch instructions use the BranchTarget struct, that allows a label based jump, or a fixed offset jump.

This PR does the following changes:

  • Jal now takes a single MachLabel as it's target
  • BrTable similarly now takes a Vec<MachLabel>
  • CondBr still uses BranchTarget, but it can only do a label jump, or fallthrough. It can no longer do a fixed offset jump

Since BranchTarget is now only used in CondBr, I've renamed it CondBrTarget.

This is mostly a personal preference. It emits the exact same code.
Replaces it with `Fallthrough` which works the same way with
a fixed offset of 0.
It is now only used for `CondBr`
@afonso360 afonso360 added the cranelift:area:riscv64 Issues related to the RISC-V 64 backend. label Sep 10, 2023
@afonso360 afonso360 requested a review from a team as a code owner September 10, 2023 10:18
@afonso360 afonso360 requested review from fitzgen and removed request for a team September 10, 2023 10:18
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator isle Related to the ISLE domain-specific language labels Sep 10, 2023
@github-actions
Copy link

Subscribe to Label Action

cc @cfallin, @fitzgen

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

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Nice!

@alexcrichton alexcrichton added this pull request to the merge queue Sep 11, 2023
Merged via the queue into bytecodealliance:main with commit 4dbc1f6 Sep 11, 2023
18 checks passed
alexcrichton pushed a commit that referenced this pull request Sep 12, 2023
…#6988)

* riscv64: Use `MachLabel` for Jal

* riscv64: Use `Inst::gen_jump` for jumps

This is mostly a personal preference. It emits the exact same code.

* riscv64: Use VecMachLabel on BrTable

* riscv64: Remove `BranchTarget::Offset` arm

Replaces it with `Fallthrough` which works the same way with
a fixed offset of 0.

* riscv64: Rename `BranchTarget`

It is now only used for `CondBr`

* riscv64: Panic on fallthrough taken target in condbr
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 13, 2023
…bytecodealliance#6988)

* riscv64: Use `MachLabel` for Jal

* riscv64: Use `Inst::gen_jump` for jumps

This is mostly a personal preference. It emits the exact same code.

* riscv64: Use VecMachLabel on BrTable

* riscv64: Remove `BranchTarget::Offset` arm

Replaces it with `Fallthrough` which works the same way with
a fixed offset of 0.

* riscv64: Rename `BranchTarget`

It is now only used for `CondBr`

* riscv64: Panic on fallthrough taken target in condbr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:riscv64 Issues related to the RISC-V 64 backend. 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