Skip to content

riscv64: Fix select.i64+icmp.i128 lowering#6317

Merged
cfallin merged 1 commit into
bytecodealliance:mainfrom
afonso360:riscv64-fix-select-icmp
May 1, 2023
Merged

riscv64: Fix select.i64+icmp.i128 lowering#6317
cfallin merged 1 commit into
bytecodealliance:mainfrom
afonso360:riscv64-fix-select-icmp

Conversation

@afonso360
Copy link
Copy Markdown
Contributor

👋 Hey,

This fixes #6312. We were accidentally selecting an optimized lowering when the icmp was i128bit. gen_select_reg lowers the comparison, but only looking at the bottom registers since it works with Reg's and not ValueRegs.

I think this bug was also in part introduced by a rule that we have (in the RISC-V backend) that auto converts ValueRegs into Reg silently.

@afonso360 afonso360 added the cranelift:area:riscv64 Issues related to the RISC-V 64 backend. label May 1, 2023
@afonso360 afonso360 requested a review from a team as a code owner May 1, 2023 16:51
@afonso360 afonso360 requested review from cfallin and removed request for a team May 1, 2023 16:51
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.

Thanks!

@cfallin cfallin enabled auto-merge May 1, 2023 17:18
Copy link
Copy Markdown
Contributor

@jameysharp jameysharp left a comment

Choose a reason for hiding this comment

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

This looks exactly right, thank you!

@cfallin cfallin added this pull request to the merge queue May 1, 2023
@github-actions github-actions Bot added the cranelift Issues related to the Cranelift code generator label May 1, 2023
Merged via the queue into bytecodealliance:main with commit 3b7274f May 1, 2023
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cranelift: RISC-V Wrong result for select+icmp.i128

3 participants