Sorry if this is the wrong stream but I don't think there is one for regalloc2(?)
I'm currently hacking a bit on regalloc2 with the current main branch. Now to make the new regalloc trait work with wasmtime I simply removed the is_move
function from the trait impl in cranelift.
Now, when running the func::import_works
(link) test I get a segfault.
This is because the emit code for the call generates a move for the last call in the test because the sixth argument is a reftype and thus can't be used with an OperandConstraint::Reg
for the call but due to the calling conv it needs to be in a reg. However, this move never gets emitted because regalloc2 doesn't know that its a move anymore and cranelift assumes regalloc2 emitted the move.
Now I think my question is, is the move skipping in cranelift incorrect? Removing it seems to do the trick but I don't know if that's gonna break something else. Also, if the register allocator chooses to use the same reg that now emits the move anyway, though that's a small concern.
I don't personally understand enough of RA2 to answer your question, but I just want to say that I think this is a fine stream to ask this question in.
Could you add more context on what you're trying to do / what you've done already? You'r switching Cranelift to use a more recent version of RA2?
The line you linked (the conditional to skip instructions) should no longer exist, either, since RA2 no longer elides program moves (by CL's definition)
with that fix, I suspect/hope that your upgrade works
(I don't have much mental bandwidth to put on this right now but if there's a true issue, beyond the above, we'll need to debug it when we next upgrade RA2 in CL. cc @Trevor Elliott as well)
I have a branch where I've updated to a newer version of RA2, but I have only tried running filetests and sightglass benchmarks with it so far.
Chris Fallin said:
Could you add more context on what you're trying to do / what you've done already? You'r switching Cranelift to use a more recent version of RA2?
Yes, I'm trying to prototype a greedy approach to see how much compile time improvements you could (possibly) get (as that is related to my bachelor thesis I finished recently and it's also fun). As such I thought starting with the current main for regalloc2 is better since cranelift will move to it eventually (and simply removing the trait impls for is_move
made it compile at least).
I just ran into this problem trying to see if my implementation of the allocator had any problems and was a bit confused about what exactly was the 'incorrect' code in cranelift. The tests seem to pass now so thanks!
OK, cool. Let us know if removing the line linked above solves the problem. If not, we can help debug further at some point!
Just to follow up here, I finished upgrading cranelift to regalloc2-0.7.0, so you should be able to rebase on main and have it just work :thumbs_up:
Thanks! I'll check it out
Last updated: Dec 23 2024 at 12:05 UTC