Skip to content

Conversation

@jameysharp
Copy link
Contributor

Since #6850, we've been able to rely on iconst instructions having their immediate operands' high bits zeroed before lowering.

So a couple of places in x64/lower.rs can be expressed more simply now as a result.

Out of an abundance of caution, I added a debug-assertion when constants are looked up during lowering, to check that earlier phases really did ensure the high bits are zero.

I also got rid of an expect where a simple pattern-match will do.

@jameysharp jameysharp requested a review from a team as a code owner April 4, 2024 19:40
@jameysharp jameysharp requested review from cfallin and removed request for a team April 4, 2024 19:40
@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:area:x64 Issues related to x64 codegen labels Apr 4, 2024
Since bytecodealliance#6850, we've been able to rely on `iconst` instructions having
their immediate operands' high bits zeroed before lowering.

So a couple of places in `x64/lower.rs` can be expressed more simply now
as a result.

Out of an abundance of caution, I added a debug-assertion when constants
are looked up during lowering, to check that earlier phases really did
ensure the high bits are zero.

I also got rid of an `expect` where a simple pattern-match will do.
@jameysharp
Copy link
Contributor Author

Turns out I can't use Option::inspect yet because it was stabilized in 1.76 and our MSRV is currently 1.75, whoops.

Copy link
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.

LGTM, thanks!

@jameysharp jameysharp added this pull request to the merge queue Apr 4, 2024
Merged via the queue into bytecodealliance:main with commit 700276b Apr 4, 2024
@jameysharp jameysharp deleted the x64-lower-consts branch April 4, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants