Cranelift: Generate an InstructionData::map method
#11176
Conversation
This allows you to map some functions, described by the given `InstructionMapper`, over each of the entitities in an instruction, producing a new `InstructionData`. I intend to use this as part of an inliner API for Cranelift that I am developing as part of prototyping [Wasmtime's compile-time builtins](bytecodealliance/rfcs#43).
|
I'll take an in-depth look at this after the weekend? |
github-actions
bot
added
cranelift
| @@ -159,7 +159,7 @@ fn gen_arguments_method(formats: &[Rc<InstructionFormat>], fmt: &mut Formatter, | |||
| /// - `pub fn eq(&self, &other: Self, &pool) -> bool` | |||
| /// - `pub fn hash<H: Hasher>(&self, state: &mut H, &pool)` | |||
| fn gen_instruction_data_impl(formats: &[Rc<InstructionFormat>], fmt: &mut Formatter) { | |||
| fmt.add_block("impl InstructionData",|fmt| { | |||
| fmt.add_block("impl InstructionData", |fmt| { | |||
There was a problem hiding this comment.
It's weird that this missing space didn't trigger a formatting error during CI prior to this; is it possible this crate isn't being checked?
There was a problem hiding this comment.
I think the whole crate disabled rustfmt or something, but I haven't looked into it yet.
| @@ -20,6 +20,7 @@ cranelift-srcgen = { workspace = true } | |||
| cranelift-assembler-x64-meta = { path = "../../assembler-x64/meta", version = "0.122.0" } | |||
| cranelift-codegen-shared = { path = "../shared", version = "0.122.0" } | |||
| pulley-interpreter = { workspace = true, optional = true } | |||
| heck = "0.5.0" | |||
There was a problem hiding this comment.
Was it necessary to pull in a new dependency for a single function?
There was a problem hiding this comment.
It's already in the workspace and vetted, so it didn't seem like a big deal.
There was a problem hiding this comment.
It was previously not a Cranelift dependency. Cranelift is slowly getting more and more dependencies.
There was a problem hiding this comment.
Sharing common code via dependencies is not bad in and of itself IMO, and is actually generally good, especially when it is already vetted and relied upon in sibling projects.
Of course, there is the potential downside of from-scratch compile times getting a little worse. I'm happy to review a PR replacing the dependency with the hand-rolled equivalent function if you measure a from-scratch compile time regression and want to send such a PR, but I don't personally have the cycles to investigate all that myself at the moment.
This allows you to map some functions, described by the given
InstructionMapper, over each of the entitities in an instruction, producing a newInstructionData.I intend to use this as part of an inliner API for Cranelift that I am developing as part of prototyping Wasmtime's compile-time builtins.