Summary: | [JSC] Work around apparent miscompilation on ARM/GCC >=8.4 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Angelos Oikonomopoulos <angelos> | ||||||||
Component: | New Bugs | Assignee: | Angelos Oikonomopoulos <angelos> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | darin, ews-watchlist, fpizlo, ggaren, guijemont, Hironori.Fujii, joepeck, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Angelos Oikonomopoulos
2021-06-17 08:24:07 PDT
Created attachment 431667 [details]
Patch
We get upwards of 1300 failing tests on ARMv7 after https://trac.webkit.org/changeset/278942/webkit. AFAICT the change itself is innocuous, but triggers a bug in GCC, so a revert seems inappropriate. Created attachment 431669 [details]
Patch
Created attachment 431670 [details]
Patch
Comment on attachment 431670 [details]
Patch
Yikes!
Committed r278991 (238917@main): <https://commits.webkit.org/238917@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 431670 [details]. Comment on attachment 431670 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431670&action=review > Source/JavaScriptCore/jit/RegisterSet.h:41 > +#if CPU(ARM) && COMPILER(GCC) > + > +#if GCC_VERSION_AT_LEAST(8, 4, 0) && !GCC_VERSION_AT_LEAST(9, 0, 0) No reason for the nested #if here. Could just be a single string of &&, I think. > Source/JavaScriptCore/jit/RegisterSet.h:46 > +#define REGISTERSET_BITMAP_SLACK 1 This can be a constexpr int; does not need to be a #define. (In reply to Darin Adler from comment #8) > Comment on attachment 431670 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=431670&action=review > > > Source/JavaScriptCore/jit/RegisterSet.h:41 > > +#if CPU(ARM) && COMPILER(GCC) > > + > > +#if GCC_VERSION_AT_LEAST(8, 4, 0) && !GCC_VERSION_AT_LEAST(9, 0, 0) > > No reason for the nested #if here. Could just be a single string of &&, I > think. The previous version of the patch without nested #ifs did not compile on clang EWS bots, as GCC_VERSION_AT_LEAST is not defined. Note: apparently this wasn't a miscompilation; it only looked like one because the bug depended on the compiler configuration and we tested with GCC versions that were not configured identically. Submitted what should be a proper fix in bug 227212. |