Stream: git-wasmtime

Topic: wasmtime / issue #7114 Cranelift: return programmatic err...

Wasmtime GitHub notifications bot (Sep 29 2023 at 18:38):

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.

Wasmtime GitHub notifications bot (Sep 29 2023 at 19:48):

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 other vregs.alloc(...).unwrap() through the ABI code. Additionally there's preexisting error handling for vregs.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 an alloc_with_deferred_error(). I left the original Result-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.

Wasmtime GitHub notifications bot (Sep 29 2023 at 20:05):

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: Nov 22 2024 at 16:03 UTC