[Webkit-unassigned] [Bug 221260] [JSC] Enable WasmLLInt on ARMv7

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=221260

Justin Michaud <justin_michaud at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |justin_michaud at apple.com

--- Comment #14 from Justin Michaud <justin_michaud at apple.com> ---
Comment on attachment 458054
  --> https://bugs.webkit.org/attachment.cgi?id=458054
v8 - rebase/2

View in context: https://bugs.webkit.org/attachment.cgi?id=458054&action=review

This is very exciting! I left a few drive-by questions/comments.

> Source/JavaScriptCore/llint/WebAssembly.asm:34
> +    const NumberOfWasmArgumentJSRs = 6

Maybe we can find a more neutral name for this?

> Source/JavaScriptCore/llint/WebAssembly.asm:716
> +    subi CalleeSaveSpaceAsVirtualRegisters + NumberOfWasmArguments, t2

I guess this is guaranteed by the spec?

> Source/JavaScriptCore/llint/WebAssembly.asm:806
> +        loadd -offset -  8 - CalleeSaveSpaceAsVirtualRegisters * 8[cfr], fpr

nit: spacing

> Source/JavaScriptCore/runtime/JSCJSValue.h:165
> +    static constexpr uint32_t WasmTag =         0xfffffffa;

Hmm. This seems confusing, since it would be a huge mistake for a value of this kind to be exposed to JS. I'm not sure what would look better though

> Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:1804
> +#if ENABLE(WEBASSEMBLY_SIGNALING_MEMORY)

Does this need a compile guard?

> Source/JavaScriptCore/wasm/WasmIndexOrName.cpp:35
> +    ASSERT(!(index & allTags));

Should this be a release assert? I'm not sure why this check is here, does it have something to do with dropping the name section? I couldn't find anything in the blame either. @msaboff might know

> Source/JavaScriptCore/wasm/WasmIndexOrName.cpp:49
> +#endif

Should we have a #else error?

> Source/JavaScriptCore/wasm/WasmInstance.h:-110
> -            m_cachedBoundsCheckingSize = memory()->boundsCheckingSize();

Why does boundsCheckingSize exist then?

> Source/JavaScriptCore/wasm/WasmParser.h:211
> +    memcpy(&result, source() + m_offset, sizeof(uint32_t)); // src can be unaligned

Is this only an issue now because unaligned access doesn't work on ARMv7?

> Source/JavaScriptCore/wasm/js/JSToWasm.cpp:-80
> -                static_assert(std::is_same_v<Wasm::Instance*, typename FunctionTraits<decltype(operationAllocateResultsArray)>::ArgumentType<1>>);

Don't we still need this?

> Source/JavaScriptCore/wasm/js/JSToWasm.cpp:219
> +        if constexpr (maxFrameExtentForSlowPathCall)

How does this work? Is this because we might have leafs that don't emit stack overflow checks?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220426/2b46a47b/attachment-0001.htm>