|Summary:||[GTK][WPE][JSCOnly] compile error when -DWTF_CPU_ARM64_CORTEXA53=ON set for arm64|
|Severity:||Normal||CC:||annulen, berto, bugs-noreply, cgarcia, clopez, ews-watchlist, gyuyoung.kim, keith_miller, mark.lam, mcatanzaro, msaboff, ryuan.choi, sbarati, sergio, ticaiolima, tzagallo, webkit-bug-importer, ysuzuki|
|Version:||WebKit Local Build|
Description Kai 2019-04-23 03:00:49 PDT
Comment 1 Alberto Garcia 2019-10-08 04:20:51 PDT
The Debian arm64 buildd is also having this problem, I confirm that the build works if WTF_CPU_ARM64_CORTEXA53 is OFF. This is the full build log (note that it's a 1.4 MiB text file): https://buildd.debian.org/status/fetch.php?pkg=webkit2gtk&arch=arm64&ver=2.26.1-2&stamp=1570464655&raw=0
Comment 2 Carlos Alberto Lopez Perez 2019-10-08 06:49:33 PDT
'-DWTF_CPU_ARM64_CORTEXA53=ON' Enables a workaround for a CPU Bug (erratum 835769) present in early versions of ARM64 Cortex A53 (in more detail, according to what I find in Internet, to revisions <= r0p4 of the Cortex A53 CPU). This workaround was added 5 years ago in r175514 but it seems it stopped building after r236589 removed removes function JSC::AssemblerBuffer::data() for ARM64. There are two major problems with this workaround: 1) The way it gets enabled its completely broken for cross-building. The CMake logic tries to parse /proc/cpuinfo to see if the build machine is a Cortex A53 (it just assumes build_machine==target_machine). It also doesn't do any extra effort to match the revision of the CPU to see if its newer than r0p4. It simply enables it for any build_machine==cortex_a53. 2) Testing that the workaround continues to be necessary and that it continues to work is tricky because it requires the affected hardware available.
Comment 3 Carlos Alberto Lopez Perez 2019-10-08 07:06:24 PDT
Our current Linux JSCOnly ARM64 test bot happens to be an x86_64 machine that cross-builds and then runs the tests remotely. Because it cross-builds its not enabling the workaround by default The test machine where it currently runs is a Raspberry Pi 3, which happens to be an ARM Cortex A53 with a revision 4 of the core (r0p4) # head /proc/cpuinfo processor : 0 BogoMIPS : 38.40 Features : fp asimd evtstrm crc32 CPU implementer : 0x41 CPU architecture: 8 CPU variant : 0x0 CPU part : 0xd03 CPU revision : 4 [....] So, in theory, it would be affected by this erratum 835769. In practice I'm not 100% sure if its affected. But its the closed hardware I have that might have the bug. And according to the current test results https://build.webkit.org/builders/JSCOnly%20Linux%20AArch64%20Release things seems to be fine there, right? So I guess that either the workaround is not necessary anymore, or that the RPi3 is not affected. But, even if we assume that the RPi3 is not affected, the workaround its not building anymore, and we can't test it works because we don't have hardware. And previously it was not getting enabled by default (only when the build machine was a Cortex A53 ... and native building on ARM is not the usual way of building WebKit). So I think we should just remove this code for the workaround unless there is some bug report about this issue or a way to test it.
Comment 4 Alberto Garcia 2019-10-08 07:08:41 PDT
What happens if you have this enabled but your CPU is not Cortex A53? Does it break?
Comment 5 Carlos Alberto Lopez Perez 2019-10-08 08:53:12 PDT
(In reply to Alberto Garcia from comment #4) > What happens if you have this enabled but your CPU is not Cortex A53? > > Does it break? No, but it can affect performance.
Comment 6 Michael Catanzaro 2020-04-13 07:34:16 PDT
(In reply to Carlos Alberto Lopez Perez from comment #3) > So I think we should just remove this code for the workaround unless there > is some bug report about this issue or a way to test it. Agreed. It clearly doesn't work and breaks the build on this hardware. Let's just remove it for now. The people who are trying to make things work on this hardware can come up with a new workaround if required.
Comment 7 Michael Catanzaro 2020-04-13 07:54:38 PDT
Warning: this might require a clean build to pass EWS. I saw Ruby-level crashes until I deleted my build directory.
Comment 9 Michael Catanzaro 2020-04-24 16:08:52 PDT
Comment 11 EWS 2020-04-24 16:35:07 PDT
Committed r260680: <https://trac.webkit.org/changeset/260680> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396274 [details].