[webkit-reviews] review denied: [Bug 213487] run-javascriptcore-tests: Support Apple Silicon running x86 processes : [Attachment 402828] Patch

[bugzilla-daemon at webkit.org]

Guillaume Emont <guijemont at igalia.com> has denied  review:
Bug 213487: run-javascriptcore-tests: Support Apple Silicon running x86
processes
https://bugs.webkit.org/show_bug.cgi?id=213487

Attachment 402828: Patch

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




--- Comment #15 from Guillaume Emont <guijemont at igalia.com> ---
Comment on attachment 402828
  --> https://bugs.webkit.org/attachment.cgi?id=402828
Patch

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

> Tools/Scripts/run-javascriptcore-tests:494
> +    # Fallback is x86_64, which we replace with the native architecture if
the native architecture is in the provided build
> +    $archs = "x86_64";
> +    $archs = nativeArchitecture() if index($archsInBuild,
nativeArchitecture()) != -1;

This always sets $archs to x86_64 on our cross-compiling bots that run tests on
a remote device of a different architecture.

> Tools/Scripts/run-javascriptcore-tests:733
> +	   "-j", jscPath($productDir), "-o", $jscStressResultsDir, "--arch",
$archs);

On our cross-compiling remote-test-running bots, this passes "--arch x86_64".
This means that tests that have //@ headers checking $architecture get a wrong
value when running on the remote device (x86_64 instead of arm or mips).

> Tools/Scripts/run-javascriptcore-tests:737
> +	   push(@jscStressDriverCmd, "--force-architecture");
> +	   push(@jscStressDriverCmd, $archs);

The behavior of --force-architecture seems to only be implemented for a
specific emulation on macOS. Thus if we fix $archs in this script for
cross-compilation cases, we should either avoid passing that argument, or fix
its implementation in run-jsc-stress-tests.

> Tools/Scripts/webkitdirs.pm:844
> +    # Most ports don't have emulation, assume that the user gave us an
accurate architecture

This comment is inaccurate. Though we don't currently use it on bots, we do use
qemu for development on armv7 and mips on linux. I guess you mean a specific
kind of emulation from macOS?

> Tools/Scripts/webkitdirs.pm:846
> +	   return determineArchitecture();

I think you mean "return architecture()", since determineArchitecture() does
not return anything.