Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support explicit stack slots #12

Closed
Amanieu opened this issue Sep 5, 2021 · 1 comment · Fixed by #17
Closed

Support explicit stack slots #12

Amanieu opened this issue Sep 5, 2021 · 1 comment · Fixed by #17

Comments

@Amanieu
Copy link
Contributor

Amanieu commented Sep 5, 2021

This is useful for functions which take many arguments and return many values, some of which are in registers while the rest are in ABI-specified stack slots.

I think the best way to add support for fixed stack slots is to add an extra bit to PReg to add 32 integer stack slots and 32 float stack slots.

Implementation-wise, a fixed stack slot is very similar to Operand::Fixed:

  • The same fixups are required in case of multiple Fixed uses of the same vreg.
  • Live ranges assigned to a fixed stack slot need to tracked just like a PReg.
  • Allocation into fixed stack slots must be done in process_bundle: we may need to split a bundle in case of conflicts or evict another conflicting bundle.
  • Requirement computation only needs minor changes to merge Requirement::Stack with Requirement::Fixed(preg) when preg refers to a fixed stack slot.
  • A spill weight of 2000 still makes sense for fixed stack slots: we need expensive moves if we don't get the stack slot that we want.
@cfallin
Copy link
Member

cfallin commented Sep 10, 2021

Sorry for the delay in thinking about this -- it's a worthwhile goal, I think! I'm definitely onboard with some sort of interface that allows the user to name explicit stack slot locations.

It seems to me that this scheme is, more or less, defining virtual PRegs (in the general sense of "virtual", not specifically "VRegs", if that makes sense): we basically have a new class, or register file, and we allocate from its entries/slots like any other. Then these are used in the same way other ABI-specified registers are used for call args. Is that accurate?

I had worried that supporting a more general explicitly-specified-stack-slot need would require a larger index (in general SpillSlot needs to grow up to the number of live values, which can be large) but on more reflection I can see that your scheme seems to cover most cases where the user cares about which spillslot specifically is used.

In Cranelift at least this will a really nice way to avoid the explicit load of stack-passed args in the prologue. Presumably your use-case is similar. (If there are more than 32 stack args of either class, we can still fall back on generally unconstrained vregs and explicit loads in the prologue.)

I'm unsure whether it makes more sense to define actual RegClass values (IntStack and FloatStack) or make this another dimension of the PReg, but I'm sort of leaning toward the latter, as it sounds like you are, as well.

As to where the bit comes from: ideally we buy ourselves more bits by implementing one of the ideas in #5. Possibly for prototyping this one could steal one more bit from the vreg index, though that is getting uncomfortably limited now and 2^19 (512K) seems pretty likely to hit some actual limits. So I think #5 is probably a prerequisite here.

One unaddressed point above is how these slots appear in the Allocation outputs. It seems to me that having a separate index range is needed -- in practice a compiler's frame layout will likely place general spillslots in a different place than stack args (the latter are usually owned by the caller, above the return address). But we have plenty of bits to spare in Allocation so I think that's fine. The alternative is to share the SpillSlot index space and require some indirection on the user side ("spillslot 3 corresponds to arg at [FP+16], ...") which seems somewhat awkward.

I'm excited to see where this goes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants