-
Notifications
You must be signed in to change notification settings - Fork 42
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
Extend integer register PReg
space to 128 registers
#131
base: main
Are you sure you want to change the base?
Conversation
Interesting -- thank you for thinking about this problem at least! I do agree that the current approach is a little ugly. This solves the problem for now, but I do wonder if this is the right time to look for a more general fix. Otherwise the limit seems likely to come into play again soon, or at least feels pretty arbitrary. Rather than overload the concept of PReg for particular stack slots, it seems we should look for a way to encode a more general stack constraint. I think you're right that a side-table approach as mentioned in #5 would let us build this: we could take a whole Incidentally, that would also give us a mechanism to solve bytecodealliance/wasmtime#6301, where we currently emit loads from BP+offset to bring stack-carried args into vregs in the function prologue; we could instead define Cranelift's use of this "custom location" index to map to BP offsets, and constrain args directly to the stack. We'd need an Thoughts? |
I spent some time thinking about what the future API of this crate might look like. First of all, regarding #5, it is clear that we need more encoding space for The main change that should happen is renaming
Overall this is mostly an API/naming change. Most of the regalloc internals remain the same. |
That's certainly an option. At least in Cranelift's use-case, though, we wouldn't accept a 3% regression; and also, 10 bits of index-space for custom locations would not be enough (we need to support up to 10K args for Wasm at least, per this implementation-limits discussion). I still think that having an out-of-band data structure (we can still have an |
8376b00
to
6080822
Compare
This PR steals space from the last currently unused register class to allow
PReg
to represent more registers, but only inRegClass::Int
.But why?
I'm using regalloc2's support for fixed stack slots (represented as
PReg
s) to let the register allocator manage VMContext state. This allows this state to be kept in registers as much as possible, even across basic blocks.Since each piece of regalloc-managed VMContext state needs its own
PReg
, the current limit of 64 integerPReg
s is insufficient: the first 32 are for actual physical registers, which leaves only 32 for fixed stack slots, but I need 46 fixed stack slots.Better solutions
The code as shown is quite ugly, so I'm open to hearing about better solutions to this. #5 seems relevant here.