RFR: 8252584: HotSpot Style Guide should permit alignas

[Julian Waters]

On Sun, 27 Nov 2022 11:42:00 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> > > No. Any proposal to permit `alignas` needs to address questions around "extended alignment", and especially "over-aligned types". [...]
> > 
> > 
> > I'm assuming you're referring to this when talking about extended alignment? I'm not sure if you mean this warning has to be included together in the Style Guide with alignas
> > ```
> > An extended alignment is represented by an alignment greater than alignof(std::max_align_t). It is
> > implementation-defined whether any extended alignments are supported and the contexts in which they
> > are supported (7.6.2). A type having an extended alignment requirement is an over-aligned type. [ Note:
> > every over-aligned type is or contains a class type to which extended alignment applies (possibly through a
> > non-static data member). — end note ]
> > ```
> 
> That's the paragraph I referenced. A proposal to permit the use of `alignas` in HotSpot code needs to indicate where (if anywhere) the use of extended alignment is permitted. And "never" doesn't work, since (for example) there are a fair number of constants used by various x86 stub generators that are given extended alignment.
> 
> Like I said, it probably works to use extended alignment for variables with static storage duration (C++14 3.7.1) with existing platforms and build configurations.
> 
> Variables with automatic storage duration might also work (I haven't done much testing of that).
> 
> Even for those cases there may be limitations on what values are supported.
> 
> Heap allocation of over-aligned types does not work until C++17 (in the sense that the requested alignment is not assured).
> 
> Types or data members of types that are never heap allocated (either directly or by inclusion in some other type) would (probably, since we mostly don't allow the use of C++ thread-local variables) end up falling under one of the above. Conceptually, a type derived from StackObj or only ever included in such would fall under the automatic variable case, but I'm pretty sure there are places where StackObj is abused, such that it doesn't have any useful guarantees.
> 
> So I suggest over-aligned types should be forbidden (at least until we move to C++17).
> 
> The Style Guide should also discuss where the `alignas` should be placed. Some non-local variables have it on the declaration, while others on the definition. Perhaps it should it always be on the declaration? Or is there a reason for that inconsistency?
> 
> > I don't think there's anything we can actually do about ZCACHE_ALIGNED, unfortunately, and the dynamic memory alignment mentioned in 8252584 is still going to be an issue regardless of whichever syntax we use:
> 
> That dynamic memory alignment is the thing that is fixed by C++17.
> 
> I think none of the uses of ZCACHE_ALIGN need the requested alignment. They are instead attempting to ensure data members are on different cache lines. We have memory/padded.hpp for that kind of thing. I've already begun a discussion about this with ZGC developers.

I've modified the Style Guide to address some of the issues brought up in the review for the time being

> The Style Guide should also discuss where the `alignas` should be placed. Some non-local variables have it on the declaration, while others on the definition. Perhaps it should it always be on the declaration? Or is there a reason for that inconsistency?

In my opinion it would be helpful to place it at both declaration and definition, just before the type specifier, so it's easier to determine that a variable has a particular alignment without having to check both. Maybe only specifying it at the declaration if that proves to be too troublesome will be enough however

Small side note: Should it also mention that ATTRIBUTE_ALIGNED will be/is deprecated after this style change, and/or that using it is equivalent to `alignas`? After all, as discussed above there isn't actually a difference of using either to specify alignment on whatever object or type it's desired to be used on

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

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