Add fixed_nonallocatable constraints when appropriate #5253
Conversation
github-actions
bot
added
cranelift
Subscribe to Label Action
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:machinst", "cranelift:area:x64", "isle"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
elliottt
force-pushed
the
trevor/add-fixed-nonallocatable-operands
branch
from
629819a to
77e36b4
Compare
elliottt
force-pushed
the
trevor/add-fixed-nonallocatable-operands
branch
from
3a71d79 to
f0e84a9
Compare
| pub struct Lower<'func, I: VCodeInst> { | ||
| pub struct Lower<'func, 'env, I: VCodeInst> { | ||
| /// The function to lower. | ||
| f: &'func Function, | ||
|
|
||
| /// Machine-independent flags. | ||
| flags: crate::settings::Flags, | ||
|
|
||
| /// The MachineEnv used when allocating registers. | ||
| machine_env: &'env MachineEnv, | ||
|
|
There was a problem hiding this comment.
Did you try reusing 'func here by chance and find that a new lifetime was actually necessary to get the code to compile? This is pretty invasive and adds a third(!!) lifetime to the IsleContext so if it isn't actually necessary it would be nice to skip.
There was a problem hiding this comment.
I didn't try that, as I was assuming that the two structures would have different lifetimes. I'll give that a try, and see if anything breaks.
There was a problem hiding this comment.
That worked, thanks for pointing this out!
There was a problem hiding this comment.
Now that I'm thinking about this, we don't need the actual MachineEnv plumbed through, just the PRegSet that's used in the OperandCollector. Moving the creation of the PRegSet to Lower::new removes the MachineEnv reference, and only iterates it once instead of for each instruction.
elliottt
force-pushed
the
trevor/add-fixed-nonallocatable-operands
branch
from
f0e84a9 to
0280ba7
Compare
Consumption of non-allocatable operands was added in bytecodealliance#5253 and bytecodealliance#5132, and removed in bytecodealliance#8524 and following PRs. Now they are not only ignored by regalloc2, but the placeholders that it leaves in the allocation results are also ignored by Cranelift. So let's stop adding them to the operands list entirely.
Consumption of non-allocatable operands was added in #5253 and #5132, and removed in #8524 and following PRs. Now they are not only ignored by regalloc2, but the placeholders that it leaves in the allocation results are also ignored by Cranelift. So let's stop adding them to the operands list entirely.
Plumb the set of allocatable registers through the
OperandCollectorand use it validate uses of fixed-nonallocatable registers, like%rspon x86_64.