[webkit-reviews] review denied: [Bug 200952] [WASM-References] Do not overwrite argument registers in jsCallEntrypoint : [Attachment 376950] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has denied Justin Michaud
<justin_michaud at apple.com>'s request for review:
Bug 200952: [WASM-References] Do not overwrite argument registers in
jsCallEntrypoint
https://bugs.webkit.org/show_bug.cgi?id=200952

Attachment 376950: Patch

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




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

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

> Source/JavaScriptCore/ChangeLog:8
> +	   We just inline the call to isWebassemblyHostFunction.

can you explain what the issue before was in some detail here?

> Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp:330
> +		   static_assert(std::is_final<WebAssemblyFunction>::value,
"This loop is untested");
> +		  
static_assert(std::is_final<WebAssemblyWrapperFunction>::value, "This loop is
untested");
> +
> +		  
jit.move(CCallHelpers::TrustedImmPtr(WebAssemblyFunction::info()),
scratch2GPR);
> +		   auto isWasmFunction = jit.branch64(CCallHelpers::Equal,
scratchGPR, scratch2GPR);
> +		  
jit.move(CCallHelpers::TrustedImmPtr(WebAssemblyWrapperFunction::info()),
scratch2GPR);
> +		   auto isWasmFunctionWrapper =
jit.branch64(CCallHelpers::Equal, scratchGPR, scratch2GPR);

Please let's not add a loop that's untested. We should just have a single
branch, which should be ensured by the is_final check above.