-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Cranelift: simplify opcode set by removing _imm variants #3250
Description
In various discussions, we have come to the conclusion that "combo ops" generally cost more than they are worth. When one CLIF opcode simply expresses the combination of two other opcodes, it (i) expands the set of opcodes that all consumers of CLIF must handle, but (ii) adds minimal value, because one can pattern-match if one needs to handle the combination specially.
So far we have not really discussed the op_imm opcodes (e.g., iadd_imm and isub_imm) in this context. They are currently converted in the "simple legalization" pass used by new backends into iconst + op, so the backends do not need to actually handle them; but this separate pass is awkward and shouldn't be necessary.
Instead, it might be better to remove the combo opcodes, but provide backward compatibility (and convenience) to producers by adding combination methods to the instruction builder that generate the two opcodes. So InstBuilder::iadd_imm would generate an iconst and an iadd, for example.
This would require some work in the meta crate but is probably feasible. The main downside is that the CLIF becomes slightly more inflated earlier in the pipeline, but we expand it before lowering anyway, so it may actually be better to generate it in the final form and avoid the edit.
cc @abrown @afonso360 @bjorn3 from earlier discussion