-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Pull in new regalloc2 with fastalloc fixes for exceptions, and re-enable and add to testing. #11533
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
Pull in new regalloc2 with fastalloc fixes for exceptions, and re-enable and add to testing. #11533
Conversation
… for now. (bytecodealliance#10554)" This reverts commit d52e23b.
alexcrichton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Personally though I'd say we can ignore the new compiler strategy part. Fuzzing will already run all *.wast tests will all various options in addition to all the other fuzz targets we have. Given that I feel that's sufficient coverage worth the tradeoff in CI time from running the whole suite with another compiler. Another example is that Pulley for example in theory would want the single-pass option too but that's not modeled here yet (but I think that's niche enough it's definitely not worth spending lots of CI time making sure that works)
Pulls in bytecodealliance/regalloc2#233 to update fastalloc to support the looser constraints needed by exception-related changes.
56e4978 to
2272703
Compare
|
Fair enough! Removed that commit, so now it's just a clean update of RA2 + revert of #10554. |
|
The |
|
The retry button doesn't show up until everything finishes, and CI is overflowing with jobs right now due to a very large queue due to the network failures. |
Pull Request is not mergeable
73de2ee
…ble and add to testing. (bytecodealliance#11533) * Revert "Cranelift/Wasmtime: disable fastalloc (single-pass) allocator for now. (bytecodealliance#10554)" This reverts commit d52e23b. * Upgrade to regalloc2 0.13.1. Pulls in bytecodealliance/regalloc2#233 to update fastalloc to support the looser constraints needed by exception-related changes. * cargo-vet update.
Thanks to @d-sonuga in bytecodealliance/regalloc2#233, the fastalloc (single-pass) register allocator now supports arbitrary numbers of non-register-constrained defs, which we use on call instructions after #10502. This should be sufficient to resolve the issues that had led us to disable fastalloc temporarily in #10554.
This PR reverts that disabling step; adds Cranelift-with-single-pass as a new compiler strategy (alongside Cranelift, Winch, and Cranelift/Pulley) to all of our wast/spec tests and fuzzing; and pulls in the new RA2.
Adding to our compiler strategy list is perhaps the most controversial thing here, but I believe it's probably appropriate to add the extra dimension here to ensure fastalloc stays working (it's a great feature to have!). Note that this surfaces as a new compiler strategy rather than a separate config dimension (like GC backend is currently) because it applies only to Cranelift, not Winch.