Re: [Buildroot] [PATCH] package/wpewebkit: disable JIT for all MIPS CPUs

[Adrian Perez de Castro <aperez@igalia.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3167 bytes --]

Hello Thomas,

On Fri, 23 Aug 2024 16:56:40 +0200 Thomas Petazzoni via buildroot <buildroot@buildroot.org> wrote:

> On Fri, 23 Aug 2024 15:29:16 +0300
> Adrian Perez de Castro <aperez@igalia.com> wrote:
> 
> > -# JIT is not supported for MIPS r6, but the WebKit build system does not
> > -# have a check for these processors. The same goes for ARMv5 and ARMv6.
> > -# Disable JIT forcibly here and use the CLoop interpreter instead.
> > +# JIT is not supported for MIPS, ARMv5, and ARMv6, but the WebKit build
> > +# system does not have a check for these processors.
> >  #
> > +# Disable JIT forcibly here and use the CLoop interpreter instead.
> >  # Also, we have to disable the sampling profiler and WebAssembly, which
> >  # do NOT work with ENABLE_C_LOOP.
> >  #
> > -# Upstream bugs: https://bugs.webkit.org/show_bug.cgi?id=191258
> > -#                https://bugs.webkit.org/show_bug.cgi?id=172765
> > -#                https://bugs.webkit.org/show_bug.cgi?id=265218
> > +# Upstream bug: https://bugs.webkit.org/show_bug.cgi?id=278559
> >  #
> > -ifeq ($(BR2_ARM_CPU_ARMV5)$(BR2_ARM_CPU_ARMV6)$(BR2_MIPS_CPU_MIPS32R6)$(BR2_MIPS_CPU_MIPS64R6),y)
> > +ifeq ($(BR2_ARM_CPU_ARMV5)$(BR2_ARM_CPU_ARMV6)$(BR2_mips)$(BR2_mipsel),y)
> 
> But then do we actually need a change?
> 
> Keep in mind that here we are only disabling JIT explicitly for ARMv5,
> ARMv6 and MIPS32R6/MIPS64R6 because they were misdetected as supported
> by wpewebkit. Indeed wpewekit supported ARM and MIPS, but not some
> variants of it.
> 
> For all other architectures, we do nothing, and wpewebit properly
> decides whether it supports JIT or not. So if JIT support for MIPS has
> been dropped I believe the correct change is:
> 
> -ifeq ($(BR2_ARM_CPU_ARMV5)$(BR2_ARM_CPU_ARMV6)$(BR2_MIPS_CPU_MIPS32R6)$(BR2_MIPS_CPU_MIPS64R6),y)
> +ifeq ($(BR2_ARM_CPU_ARMV5)$(BR2_ARM_CPU_ARMV6),y)
> 
> but only of course once we use a version of wpewebkit that really has
> dropped MIPS support.

I am confident that the 2.44.x does not include MIPS JIT support in
JavaScriptCore, but when working this I could not just remove the
$(BR2_MIPS_*) checks because the build failed. Which is another reason
why I filed https://bugs.webkit.org/show_bug.cgi?id=278559 to update
the checks in WebKit's CMake build system.
 
> As such, your change would potentially break: it re-enables JIT on
> BR2_MIPS_CPU_MIPS64R6=y, because you disable for BR2_mips and
> BR2_mipsel but there are the 32-bit variants of MIPS, and so
> BR2_MIPS_CPU_MIPS64R6=y would again allow JIT.

My intention was to always make the Buildroot packaging disable JIT
support on both 32- and 64-bit MIPS. I didn't notice that BR2_mips[el]
only covered the 32-bit ones.

> Could you clarify if we have already updated to a version of wpewebkit
> that has dropped MIPS support for JIT? If so, we should simply apply
> the simpler change above.

I'll double check again but I am not 100% sure we can use the simpler
check. Ideally, we shouldn't need a check in the Buildroot packaging
and my ultimate goal is to have WebKit's CMake build system configure
itself correctly so we can remove the ad-hoc checks in Buildroot.

Cheers,
—Adrián

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot