| Summary: | [JSC] Fix build after r243232 on unsupported 64bit architectures | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tomas Popela <tpopela> | ||||||
| Component: | JavaScriptCore | Assignee: | Tomas Popela <tpopela> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
Created attachment 365529 [details]
Patch
Comment on attachment 365529 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365529&action=review Also please undef your define at the bottom of the header > Source/JavaScriptCore/bytecode/CodeOrigin.h:34 > +#define USE_COMPRESSED_CODE_ORIGIN CPU(ARM64) || CPU(X86_64) We can only opt in (ARM64 && ADDRESS64) We don’t want compression scheme for ARM64_32 Comment on attachment 365529 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365529&action=review >> Source/JavaScriptCore/bytecode/CodeOrigin.h:34 >> +#define USE_COMPRESSED_CODE_ORIGIN CPU(ARM64) || CPU(X86_64) > > We can only opt in (ARM64 && ADDRESS64) > We don’t want compression scheme for ARM64_32 Hence, this line should say: #define USE_COMPRESSED_CODE_ORIGIN (CPU(ADDRESS64) && (CPU(ARM64) || CPU(X86_64))) That should exclude the CPU(ADDRESS64) architectures that cannot support this yet. I think we already expect 16 free bits at the top of any pointer for JSValue. Thus, I think you could change the code to:
#if CPU(ARM64) && CPU(ADDRESS64)
static constexpr unsigned s_freeBitsAtTop = 28;
static constexpr uintptr_t s_maskCompositeValueForPointer = 0x0000000ffffffff8;
#elif CPU(ADDRESS64)
static constexpr unsigned s_freeBitsAtTop = 16;
static constexpr uintptr_t s_maskCompositeValueForPointer = 0x0000fffffffffff8;
#endif
Thank you Saam, Mark and Keith for the feedback! I will go with the Keith's suggestion (when the patch passes our CI). Created attachment 365594 [details]
Patch
Comment on attachment 365594 [details]
Patch
r=me
Comment on attachment 365594 [details] Patch Clearing flags on attachment: 365594 Committed r243363: <https://trac.webkit.org/changeset/243363> All reviewed patches have been landed. Closing bug. |
The build was broken due to: #if CPU(X86_64) && CPU(ADDRESS64) static constexpr unsigned s_freeBitsAtTop = 16; static constexpr uintptr_t s_maskCompositeValueForPointer = 0x0000fffffffffff8; #elif CPU(ARM64) && CPU(ADDRESS64) static constexpr unsigned s_freeBitsAtTop = 28; static constexpr uintptr_t s_maskCompositeValueForPointer = 0x0000000ffffffff8; #endif missing the values for other 64bit arches (such as ppc64, ppc64le, s390x). Mark suggested that we should put the code introduced in r243232 behing the USE() flag.