[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8295146: Clean up native code with newer C/C++ language features [v3]
From:       Julian Waters <jwaters () openjdk ! org>
Date:       2022-11-30 7:55:24
Message-ID: X08uwA9tf91BTq70u3o3koABY-5ibEiJjUGuXECqoJ8=.496742e5-9430-4ff1-9944-9922e4070035 () github ! com
[Download RAW message or body]

On Wed, 23 Nov 2022 04:58:38 GMT, Kim Barrett <kbarrett@openjdk.org> wrote:

> > 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)
> 
> > Out of curiosity, is there a way to get the discussion on approving the use of \
> > alignas back up? [...]
> 
> A PR to address JDK-8252584 would be welcomed by me. Just do the process for
> Style Guide changes (see the Style Guide or previous PRs for such). I don't
> expect it would be very controversial. I think the only reason it hasn't
> already happened is because nobody has gotten around to it, or felt the need
> for it.
> 
> JDK-8250269 touches a bit more code (mostly in stubGenerator_x86_64 and
> macroAssembler_x86_32), but also seems like it should be straightforward.
> 
> > > 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)
> 
> You are right that the Windows-conditionalized uses are in non-HotSpot code.
> I missed that context when skimming through the changes.  Since Visual Studio
> is always C++ (even though the shared files are written as C), using alignas
> with appropriate conditionalization in those files should be fine.

Resolved, will address re-implementing ATTRIBUTE_ALIGNED with alignas in another Pull \
Request

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

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


[prev in list] [next in list] [prev in thread] [next in thread]