[webkit-reviews] review granted: [Bug 169773] WebAssembly: add fallback to use pinned register to load/store state : [Attachment 305616] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 169773: WebAssembly: add fallback to use pinned register to load/store
state
https://bugs.webkit.org/show_bug.cgi?id=169773

Attachment 305616: Patch

https://bugs.webkit.org/attachment.cgi?id=305616&action=review




--- Comment #40 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 305616
  --> https://bugs.webkit.org/attachment.cgi?id=305616
Patch

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

r=me

> Source/JavaScriptCore/jit/AssemblyHelpers.cpp:812
> +#if ENABLE(WEBASSEMBLY)
> +void AssemblyHelpers::loadWasmContext(GPRReg dst)
> +{
> +#if ENABLE(FAST_TLS_JIT)
> +    if (Options::useWebAssemblyFastTLS()) {
> +	   loadFromTLSPtr(fastTLSOffsetForKey(WTF_WASM_CONTEXT_KEY), dst);
> +	   return;
> +    }
> +#endif
> +    move(Wasm::PinnedRegisterInfo::get().wasmContextPointer, dst);
> +}
> +
> +void AssemblyHelpers::storeWasmContext(GPRReg src)
> +{
> +#if ENABLE(FAST_TLS_JIT)
> +    if (Options::useWebAssemblyFastTLS()) {
> +	   storeToTLSPtr(src, fastTLSOffsetForKey(WTF_WASM_CONTEXT_KEY));
> +	   return;
> +    }
> +#endif
> +    move(src, Wasm::PinnedRegisterInfo::get().wasmContextPointer);
> +}
> +
> +bool AssemblyHelpers::loadWasmContextNeedsMacroScratchRegister()
> +{
> +#if ENABLE(FAST_TLS_JIT)
> +    if (Options::useWebAssemblyFastTLS())
> +	   return loadFromTLSPtrNeedsMacroScratchRegister();
> +#endif
> +    return false;
> +}
> +
> +bool AssemblyHelpers::storeWasmContextNeedsMacroScratchRegister()
> +{
> +#if ENABLE(FAST_TLS_JIT)
> +    if (Options::useWebAssemblyFastTLS())
> +	   return storeToTLSPtrNeedsMacroScratchRegister();
> +#endif
> +    return false;
> +}

These all should all interface with "useFastTLSForContext" instead of how
they're written.

> Source/JavaScriptCore/wasm/WasmMemoryInformation.h:98
> +inline bool useFastTLSForContext()

Style nit: Can we call this "useFastTLSForWasmContext"?