Support big- and little-endian lane order with bitcast #5196
Conversation
github-actions
bot
added
cranelift
uweigand
force-pushed
the
bitcast-lanes
branch
from
2f530ec
to
086954b
Compare
There was a problem hiding this comment.
Thanks -- this is surprisingly simple; I'm happy how cleanly it came out. The simple definition/derivation of bitcast's semantics via store+load combines really nicely with our endianness support generally.
I think using a restricted subset of MemFlags is fine for that reason. (Instruction format too; at first LoadNoOffset seemed a bit odd but none of the instruction predicates come directly from the format (e.g. can_load) and so I think it's better than introducing another enum variant in the end.)
I do think we should require one of big or little when lane-count differs, as you suggest; otherwise this is a new way that endian-specific behavior is visible in CLIF, which I'd prefer to avoid. Happy to r+ once that is added!
| @@ -3113,11 +3114,15 @@ pub(crate) fn define( | |||
|
|
|||
| The input and output types must be storable to memory and of the same | |||
| size. A bitcast is equivalent to storing one type and loading the other | |||
| type from the same address. | |||
| type from the same address, using the specified MemFlags. | |||
There was a problem hiding this comment.
s/using/both using/, just for clarity (other possibilities here include storing with one MemFlags and loading with another, which isn't what we're emulating but someone might imagine this instead)
Add a MemFlags operand to the bitcast instruction, where only the `big` and `little` flags are accepted. These define the lane order to be used when casting between types of different lane counts. Update all users to pass an appropriate MemFlags argument. Implement lane swaps where necessary in the s390x back-end. This is the final part necessary to fix bytecodealliance#4566.
uweigand
force-pushed
the
bitcast-lanes
branch
from
086954b
to
f5f33e1
Compare
uweigand
changed the title
Ok, implemented. There were four filetests where the error triggered, which I fixed by adding a byte order specifier. Thanks! |
Add a MemFlags operand to the bitcast instruction, where only the
bigandlittleflags are accepted. These define the lane order to be used when casting between types of different lane counts.Update all users to pass an appropriate MemFlags argument.
Implement lane swaps where necessary in the s390x back-end.
This is the final part necessary to fix
#4566.
CC @cfallin
This is still a RFC because of the following questions:
MemFlagsAPI is the right choice. It seems that way to me, since it makes the semantics easy to specific, and is simple to implement based on the already existingMemFlagsinfrastructure. If you prefer some other implementation, please let me know - I'd be happy to change this.