Make formats less SIMD-specific#1770
Conversation
|
Waiting on #1762... |
Subscribe to Label Actioncc @bnjbvr DetailsThis issue or pull request has been labeled: "cranelift", "cranelift:meta", "cranelift:wasm"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
|
I am not using the textual IR for anything besides debugging, so no complaints from me :) |
Subscribe to Label Actioncc @fitzgen DetailsThis issue or pull request has been labeled: "cranelift:area:peepmatic"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
|
I am currently using neither |
The InsertLane format has an ordering (`value().imm().value()`) and immediate name (`"lane"`) that make it awkward to use for other instructions. This changes the ordering (`value().value().imm()`) and uses the default name (`"imm"`) throughout the codebase.
Like bytecodealliance#1762, this change the name of the `ExtractLane` format to the more-general `BinaryImm8` and renames its immediate argument from `lane` to `imm`.
There was a problem hiding this comment.
This look good! and learning why it is the way it is has been informative:
(InsertLane) Why change op1, op1-mod, op2-style IR to op1, op2, op1-mod-style?
Because some instructions, like x86's pshufw, would want a three-arg-one-immediate format, but it would be src, imm, dest, where there's no relation between imm and any operand other than the shuffling pshufw actually performs. "philosophically speaking" in how Cranelift IR is defined, existing formats just aren't quite a good fit.
(InsertLane) Why not have both op1, op1-mod, op2 and op1, op2, mod formats?
The instruction format machinery is lax about where immediates show up. This means that Cranelift can't disceren between op, imm, op and op, op, imm formats
Consequently, having formats that are less SIMD-oriented makes sense both for insert/extract and other instructions. And maybe one unlikely day we'll reuse the ternary format for 3-arg imul :)
edit: unwelcome CI failure though, with no logs?
Yeah, I have no idea. I re-ran them and everything was fine 🤷. |
This renames and slightly alters several formats:
InsertLanebecomesTernaryImm8ExtractLanebecomesBinaryImm8BinaryImmbecomesBinaryImm64for consistencyThis has additional comments in #1762 about the ordering of operands; in changing
InsertLanetoTernaryImm8I changed the order of the operands in the CLIF IR (value, imm, valuetovalue, value, imm) so that this format will make more sense for other instructions (x86_pshufd,x86_pblendw). Other than that this should be a straightforward renaming.A heads up at the Cranelift API level: users of Cranelift IR (e.g. @bjorn3, @jyn514?) might notice this naming chaning if they are using
FormatorInstructionDatadirectly.