Bug 183508 - Use alignas instead of compiler-specific attributes
Summary: Use alignas instead of compiler-specific attributes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
: 183617 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-03-09 02:43 PST by Yusuke Suzuki
Modified: 2018-04-10 20:26 PDT (History)
12 users (show)

See Also:

Attachments
Patch (13.26 KB, patch)
2018-03-09 02:46 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (13.26 KB, patch)
2018-03-09 02:58 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (12.26 KB, patch)
2018-03-09 03:14 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (12.21 KB, patch)
2018-03-09 04:20 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (2.22 KB, patch)
2018-04-08 22:06 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (19.51 KB, patch)
2018-04-09 12:03 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2018-03-09 02:43:44 PST 
Use alignas instead of compiler-specific attributes
Comment 1 Yusuke Suzuki 2018-03-09 02:46:32 PST 
Created attachment 335408 [details]
Patch
Comment 2 Yusuke Suzuki 2018-03-09 02:58:01 PST 
Created attachment 335411 [details]
Patch
Comment 3 Yusuke Suzuki 2018-03-09 03:14:04 PST 
Created attachment 335413 [details]
Patch
Comment 4 Yusuke Suzuki 2018-03-09 04:20:24 PST 
Created attachment 335419 [details]
Patch
Comment 5 Mark Lam 2018-04-08 13:46:23 PDT 
Comment on attachment 335419 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335419&action=review

r=me

> Source/JavaScriptCore/jit/JIT.h:172
> +    // performance fluctuations. Try forcing alignment in an attempt to stabalize this.

nit: /stabalize/stabilize/.  I know this came from an old comment, but might as well fix it.

> Source/WTF/wtf/Gigacage.cpp:35
> +alignas(void*) char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE];

This disagrees with the bmalloc definition which has it as alignas(GIGACAGE_BASE_PTRS_SIZE).  Unless you have a reason to not do so, let's make it consistent with the bmalloc one.

> Source/WTF/wtf/Gigacage.h:37
> +alignas(void*) extern WTF_EXPORTDATA char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE];

Ditto.
Comment 6 Yusuke Suzuki 2018-04-08 14:18:43 PDT 
Comment on attachment 335419 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335419&action=review

>> Source/JavaScriptCore/jit/JIT.h:172
>> +    // performance fluctuations. Try forcing alignment in an attempt to stabalize this.
> 
> nit: /stabalize/stabilize/.  I know this came from an old comment, but might as well fix it.

Nice, fixed.

>> Source/WTF/wtf/Gigacage.cpp:35
>> +alignas(void*) char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE];
> 
> This disagrees with the bmalloc definition which has it as alignas(GIGACAGE_BASE_PTRS_SIZE).  Unless you have a reason to not do so, let's make it consistent with the bmalloc one.

OK, changed.

>> Source/WTF/wtf/Gigacage.h:37
>> +alignas(void*) extern WTF_EXPORTDATA char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE];
> 
> Ditto.

Fixed.
Comment 7 Yusuke Suzuki 2018-04-08 14:30:08 PDT 
Committed r230380: <https://trac.webkit.org/changeset/230380>
Comment 8 Radar WebKit Bug Importer 2018-04-08 14:31:16 PDT 
<rdar://problem/39267802>
Comment 9 Yusuke Suzuki 2018-04-08 14:36:54 PDT 
Comment on attachment 335419 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335419&action=review

>>> Source/WTF/wtf/Gigacage.cpp:35
>>> +alignas(void*) char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE];
>> 
>> This disagrees with the bmalloc definition which has it as alignas(GIGACAGE_BASE_PTRS_SIZE).  Unless you have a reason to not do so, let's make it consistent with the bmalloc one.
> 
> OK, changed.

I've reverted this part to void* since some of the compilers do not support such a large alignment.
Here, what we want is pointer size alignment.

>>> Source/WTF/wtf/Gigacage.h:37
>>> +alignas(void*) extern WTF_EXPORTDATA char g_gigacageBasePtrs[GIGACAGE_BASE_PTRS_SIZE];
>> 
>> Ditto.
> 
> Fixed.

Ditto.
Comment 10 Yusuke Suzuki 2018-04-08 14:38:29 PDT 
Committed r230381: <https://trac.webkit.org/changeset/230381>
Comment 11 Yusuke Suzuki 2018-04-08 21:55:28 PDT 
Committed r230388: <https://trac.webkit.org/changeset/230388>
Comment 12 Yusuke Suzuki 2018-04-08 22:02:33 PDT 
Hmmm, we enabled warnings on MSVC, which easily leads to build failures.
I'll just disable JIT alignas for MSVC.
Comment 13 Yusuke Suzuki 2018-04-08 22:06:19 PDT 
Reopening to attach new patch.
Comment 14 Yusuke Suzuki 2018-04-08 22:06:21 PDT 
Created attachment 337481 [details]
Patch

Attempt to fix
Comment 15 Yusuke Suzuki 2018-04-09 04:28:06 PDT 
Committed r230403: <https://trac.webkit.org/changeset/230403>
Comment 16 youenn fablet 2018-04-09 12:03:28 PDT 
Reopening to attach new patch.
Comment 17 youenn fablet 2018-04-09 12:03:29 PDT 
Created attachment 337517 [details]
Patch
Comment 18 youenn fablet 2018-04-09 12:04:18 PDT 
Wrong bug id
Comment 19 Mark Lam 2018-04-10 20:26:44 PDT 
*** Bug 183617 has been marked as a duplicate of this bug. ***