Stream: git-cranelift

Topic: cranelift / Issue #1156 Kill Recipes With Fire 🔥

GitHub (Feb 28 2020 at 23:27):

alexcrichton transferred Issue #1156:

(Really a meta-issue about the meta-crate!)

Nobody likes Recipes. They were supposed to contain factored-out fields of the encodings. But they're mostly one indirection layer that's hard to understand, that needs to be explained to newcomers, and doesn't solve the factoring out problem ideally. For instance, if you want to use one recipe, but just one register allocation constraint differs from the one you're looking at, you'll need to duplicate it and change the one constraint. Other things that are always unclear are whether an instruction/ISA predicate should be attached to an Encoding or to the underlying Recipe.

Let's kill recipes with fire. There's been discussions about this, and a general agreement that it's the right thing to do: it'll make the code easier to understand, more straightforward, and it'll be more pleasant for contributors in general.

The general plan is backed by the following idea:

1. take a subset of related fields in the Recipe
2. put them in the Encoding instead
3. rinse and repeat step 1 until there's nothing anymore in the Recipe
4. ???
5. fun and profit

I've thought about this, and I think the following groups can be done in chunks, so they can serve as divides of the work that's needed here:

• [ ] On x86, have the REX byte emission be a runtime decision in the binemit code, rather than a different encoding. That's #1101.
  • [ ] Once that's done, replace x86's Template by helper functions that take an Encoding and tweak it to return a new encoding.
• [ ] Move ISA and instruction predicates over to Encoding (This is likely to make legalization slightly faster, since the ISel interpreter is able to deduplicate tests for a given pair of (ISA, instruction) predicates attached to several encodings.)
  • [ ] Then we can simplify code in the ISel interpreter to not store and check recipe predicates.
• [ ] Move register allocation constraints over to Encoding; these are operands_in/operands_out right now, giving constraints to Value inputs and outputs of a given Instruction.
• [ ] Move binemit code + size computation information over to Encoding (base_size/compute_size/branch_range). Note (I think) we don't need to move the format field, because an Encoding is attached to an Instruction that has its own format. (Because of recipes, one could attach different formats to the Instruction / Encoding, causing a runtime error in the build script. Good riddance!)
  • [ ] Create an enum for binemit functions, so they can still be shared across Encodings.
  • [ ] Use enums instead of plain function pointers for compute_size, to avoid indirect calls in the codegen crate.
  • [ ] (Bonus) Find a way so binemit function bodies are compiled as part of the meta crate itself (macro?).
• [ ] Move clobbers_flags to the Encoding.

Some of these items might not be doable as I expect; I tried to imagine what the could would ideally look like, but the reality of how things are in Cranelift may have us go down a different road.

Each step will imply some work in the codegen crate too, to make sure it's still working properly, as well as opportunities for simplifications. I haven't put items for each simplification, because it's somewhat unclear in general what these could be, but they may exist. It'd be nice that each step belonged to its own PR (substeps can go in commits), to avoid boiling the ocean at once ;)

Last updated: Oct 23 2024 at 20:03 UTC