fitzgen commented on issue #7114:
This PR also includes a test at the Wasmtime level. Note that it takes ~22s to run on my (relatively fast) laptop, because it has to run until it runs out of VRegs in a debug build of the compiler. We could remove the test if we feel we're otherwise confident.
I think it is fine to keep for now, and if it becomes a pain then we can figure out what to do.
cfallin commented on issue #7114:
Updated, thanks!
Would it perhaps be worth handling other callers of
vregs.alloc
as well? There's a number of othervregs.alloc(...).unwrap()
through the ABI code. Additionally there's preexisting error handling forvregs.alloc(...)?
and could that use this same pattern as well? It might be relatively simplifying to assume that vreg allocation always succeeds and only at the end is the global error checked to see if allocation actually failed.Ah, yes, I had missed the ABI code that also unwrapped -- the
VRegAllocator
now has analloc_with_deferred_error()
. I left the originalResult
-returning method because there are a few uses in the pre-lowering-loop setup and IMHO it was a bit more awkward to remember to check in all the right toplevel places than to have a special method used during the main lowering loop.
cfallin commented on issue #7114:
Yep, the ABI code in question is invoked during lowering (at callsites, or at a return instruction) and the last basic block processed will do its check after all other lowering is complete, so everything should be caught, I think.
Last updated: Dec 23 2024 at 12:05 UTC