On Tue, 15 Nov 2022 06:33:00 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:

>> Julian Waters has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Revert to using simpler solution similar to the original 8274980
>
> src/hotspot/share/utilities/globalDefinitions.hpp line 50:
> 
>> 48: 
>> 49: #ifndef ATTRIBUTE_ALIGNED
>> 50: #define ATTRIBUTE_ALIGNED(x) alignas(x)
> 
> HotSpot Group has not discussed or approved use of `alignas` - see 
> https://bugs.openjdk.org/browse/JDK-8250269.  This is another change that is 
> independent of most of the rest of this PR, and should be dealt with 
> separately.  The various MSVC-conditional direct uses of 
> `_declspec(align(N))` should probably currently be using `ATTRIBUTE_ALIGNED`.

Out of curiosity, is there a way to get the discussion on approving the use of 
alignas back up? I've read through 8250269 briefly and unlike the issues that 
come with C++ attributes, alignas looks relatively straightforward to switch 
to, without much effect on existing code. Seems like a bit of a waste to leave 
the JBS entry sitting on the shelf to me

> The various MSVC-conditional direct uses of __declspec(align(N)) should 
> probably currently be using ATTRIBUTE_ALIGNED.

The instances of `__declspec(align())` changed here are in the native libraries 
written in C, not within HotSpot itself. From what I can see at least HotSpot 
never uses compiler alignment attributes directly and always strictly sticks to 
`ATTRIBUTE_ALIGNED` (which is probably a good thing)

-------------

PR: https://git.openjdk.org/jdk/pull/11081

Reply via email to