| Summary: | [GTK][WPE][JSCOnly] compile error when -DWTF_CPU_ARM64_CORTEXA53=ON set for arm64 | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kai <kai.7.kang> | ||||
| Component: | JavaScriptCore | Assignee: | Michael Catanzaro <mcatanzaro> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | annulen, berto, bugs-noreply, cgarcia, clopez, ews-watchlist, gyuyoung.kim, keith_miller, mark.lam, mcatanzaro, msaboff, ryuan.choi, saam, sergio, ticaiolima, tzagallo, webkit-bug-importer, ysuzuki | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Local Build | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=138315 | ||||||
| Attachments: |
|
||||||
|
Description
Kai
2019-04-23 03:00:49 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 '-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. 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. What happens if you have this enabled but your CPU is not Cortex A53? Does it break? (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. (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. Warning: this might require a clean build to pass EWS. I saw Ruby-level crashes until I deleted my build directory. Created attachment 396274 [details]
Patch
Ping reviewers Comment on attachment 396274 [details]
Patch
r=me
Committed r260680: <https://trac.webkit.org/changeset/260680> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396274 [details]. |