Wasmtime GitHub notifications bot (Apr 22 2020 at 00:15): Wasmtime GitHub notifications bot (Apr 22 2020 at 00:15):github-actions[bot] commented on Issue #1573:
Subscribe to Label Action
cc @bnjbvr
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64"Thus the following users have been cc'd because of the following labels:
- bnjbvr: cranelift
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Wasmtime GitHub notifications bot (Apr 22 2020 at 21:19):alexcrichton commented on Issue #1573:
Thanks for taking a look @cfallin! I definitely have some documentation to shore up no matter what :).
In any case I wanted to ask some questions about your comments. It sounds like you're thinking we should use vregs (which I presume is a virtual not-yet-allocated register, right?) for this stuff. I think that might be difficult though because prologue generation happens after register allocation. The saving grace, though, is that the registers used here in the prologue are only used during the prologue and never afterwards. Additionally I think that all caller-saved registers are available, not just the spilltmp one (that one just had a somewhat convenient name to pull from).
Does that make sense? I could also very well be missing something about how to plumb vregs in here.
I also don't mind writing more documentation about the stack limit support in cranelift. I didn't add documentation in my previous PR (boo) and would be happy to add some here which has notes about how this is implemented for each architecture's backend and how it's running at a unique time where register allocation/legalization aren't available.
Wasmtime GitHub notifications bot (Apr 22 2020 at 22:58):cfallin commented on Issue #1573:
So, one clarification: in the new backend, the prologue generation runs before register allocation. So we are free to allocate temp virtual registers (vregs) and use them if we need to. (I guess the old backend design didn't do it this way; I didn't study the reasoning for that in detail, but the current approach made things a lot simpler for us.) Given that, I think it would be best if we added just a little plumbing: alloc a vreg when calling
gen_prologue()and pass it in (as aWritable<Reg>). Sorry for the confusion on this point!
Wasmtime GitHub notifications bot (Apr 22 2020 at 23:00):cfallin commented on Issue #1573:
(And, as a followup clarification to that, we made sure that the register allocator understands live ranges of both real regs and virtual regs, so it's totally OK to use a vreg and then later in the prologue, refer to a realreg like an argument register; the regalloc considers the real-reg live range when choosing where to allocate the vreg.)
Wasmtime GitHub notifications bot (Apr 23 2020 at 14:29):alexcrichton commented on Issue #1573:
Oh that's news to me! I'm a bit confused though because where
gen_prologueis called takes aRegAllocResultwhich sounds like register allocation has already happened?
Wasmtime GitHub notifications bot (Apr 24 2020 at 12:58):bjorn3 commented on Issue #1573:
I believe
gen_prologueis indeed run after regalloc.
Wasmtime GitHub notifications bot (Apr 24 2020 at 15:19):alexcrichton commented on Issue #1573:
@cfallin hm so given that register allocation runs before prologue/epilogue generation, do you think it'd be ok to shore up the documentation around these and say how they can use any caller-saved register and how we're doing manual register allocation because we generate the prologue so late?
Wasmtime GitHub notifications bot (Apr 24 2020 at 17:24):cfallin commented on Issue #1573:
Argh, OK, I was deeply confused yesterday; my apologies. (Many balls in the air now that we're tying-up-loose-ends...) You're correct, prologue generation happens after regalloc. I think that I was thinking about the other bits of ABI-generated instruction sequences (argument/retval copies) which use vregs freely.
Indeed, the prologue can use only real regs. So I think the best way forward is to use the spilltmp reg as you have, and then carefully document why it's safe -- between the def (computing the GV) and use (the check itself) no spills could have happened.
Wasmtime GitHub notifications bot (Apr 24 2020 at 19:35):alexcrichton commented on Issue #1573:
Ok I've tried to add some comments and such, as well as rebasing and uncommenting a few tests which now pass. Mind taking another looking @cfallin?
Wasmtime GitHub notifications bot (Apr 24 2020 at 20:02):alexcrichton commented on Issue #1573:
No worries at all! Thanks again for the review :)
Last updated: Nov 22 2024 at 16:03 UTC