Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Magnus Ihse Bursie]

On Fri, 19 Jan 2024 12:08:21 GMT, Julian Waters  wrote:

>> Julian Waters has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Require clang 13 in toolchain.m4
>
> Should I split the compiler upgrades into a different change and integrate 
> that first? Going off the conversation in this thread it would seem like the 
> compiler upgrade would benefit us a lot more than just having C++17 (The 
> noreturn attribute is one big motivating factor for instance) and it might 
> help if the compiler upgrades were not delayed by the discussion of when to 
> jump to C++17

@TheShermanTanker I suggest you close this PR. If we are going to switch to 
C++17, it should start by a discussion in the mailing list, not with a PR (the 
change itself is trivial).

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-2074487573

Re: RFR: 8314488: Compile the JDK as C++17 [v7]

[Matthias Baesken]

On Wed, 20 Mar 2024 05:44:48 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 11 commits:
> 
>  - Merge branch 'master' into patch-7
>  - Require clang 13 in toolchain.m4
>  - Remove unnecessary -std=c++17 option in Lib.gmk
>  - Merge branch 'openjdk:master' into patch-7
>  - Compiler versions in toolchain.m4
>  - Merge branch 'openjdk:master' into patch-7
>  - Merge branch 'openjdk:master' into patch-7
>  - Revert vm_version_linux_riscv.cpp
>  - vm_version_linux_riscv.cpp
>  - allocation.cpp
>  - ... and 1 more: https://git.openjdk.org/jdk/compare/269163d5...9286a964

Seems we use already a little bit of C++17 coding in the Linux codebase .
Just came across this little error when trying to build with clang on Linux

jdk/src/hotspot/os/linux/os_linux.cpp:2975:65: error: 'static_assert' with no 
message is a C++17 extension [-Werror,-Wc++17-extensions]
  static_assert(MADV_POPULATE_WRITE == MADV_POPULATE_WRITE_value);

So switching to C++17 would make our codebase compile :-)  (at least in this 
case)  !
(to be more serious, I guess I better file a JBS bug and post a PR/fix for this 
 static_assert)

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-2061233028

Re: RFR: 8314488: Compile the JDK as C++17 [v7]

[Julian Waters]

> Compile the JDK as C++17, enabling the use of all C++17 language features

Julian Waters has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 11 commits:

 - Merge branch 'master' into patch-7
 - Require clang 13 in toolchain.m4
 - Remove unnecessary -std=c++17 option in Lib.gmk
 - Merge branch 'openjdk:master' into patch-7
 - Compiler versions in toolchain.m4
 - Merge branch 'openjdk:master' into patch-7
 - Merge branch 'openjdk:master' into patch-7
 - Revert vm_version_linux_riscv.cpp
 - vm_version_linux_riscv.cpp
 - allocation.cpp
 - ... and 1 more: https://git.openjdk.org/jdk/compare/269163d5...9286a964

-

Changes: https://git.openjdk.org/jdk/pull/14988/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=14988=06
  Stats: 4 lines in 2 files changed: 0 ins; 1 del; 3 mod
  Patch: https://git.openjdk.org/jdk/pull/14988.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14988/head:pull/14988

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

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Kim Barrett]

On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Require clang 13 in toolchain.m4

As I said earlier, I think the change to build with C++17 needs motivation, 
probably in the form of proposed changes
to the style guide to describe new features whose use is permitted (or 
forbidden!) in HotSpot.  There is also
non-HotSpot C++ code in the JDK, and buy-in from the relevant teams will also 
be needed for that, or decide to
limit the scope of the change.  (I'm not really a fan of that, but mentioning 
it as an option.)

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1997122909

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Magnus Ihse Bursie]

On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Require clang 13 in toolchain.m4

Now the version upgrades are integrated. From my PoV, that was the interesting 
part. However, this PR is still open. Is there an interest in switching from 
C++14 to 17, or should this PR be retracted until a more unison will to upgrade 
C++ standard appears?

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1996733804

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Kim Barrett]

On Wed, 14 Feb 2024 15:59:50 GMT, Kim Barrett  wrote:

> I'd like to separate the version update discussions from C++17 specifics, so 
> we can have focused discussions on the version choices. Of course, that's 
> going to be informed by the possibility of C++17, but that's not the only 
> factor. That way this issue can be just about turning on C++17, when, and why.
> 
> In order to do that, I've filed the following issues about version updates: 
> https://bugs.openjdk.org/browse/JDK-8325881 Require minimum gcc version 10
> 
> https://bugs.openjdk.org/browse/JDK-8325878 Require minimum clang version 13
> 
> https://bugs.openjdk.org/browse/JDK-8325880 Require minimum OpenXL C/C++ 
> version 17.1.1
> 
> PRs will follow shortly.

And here are the promised PRs:

8325881: Require minimum gcc version 10 
https://github.com/openjdk/jdk/pull/17899

8325878: Require minimum Clang version 13
https://github.com/openjdk/jdk/pull/17862

8325880: Require minimum Open XL C/C++ version 17.1.1
https://github.com/openjdk/jdk/pull/17857

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1949900566

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Andrew Haley]

On Fri, 16 Feb 2024 12:43:29 GMT, Magnus Ihse Bursie  wrote:

> I think Kim's approach here -- to separate compiler upgrades from C++17 usage 
> -- is the right way. Then we can discuss each part separately -- what version 
> is suitable for each compiler, and then -- if or when we end up with all 
> compilers supporting C++17 -- we can come back to the discussion of whether 
> that is a good idea.

I agree with all that. I don't even object to upgrading to a recent(ish) GCC, 
but I do think we ought to at least get a mention of the downsides on the 
record, and sometimes that job falls to me. So I think I agree it's the right 
thing to do going forward, even though it might be a bit bumpy for those of us 
trying to build the JDK on a range of elderly long-term-support Linux systems.

> I also apologize for my hash expressions. :( I did not mean to insult anyone.

No problem. We're good.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1948879029

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Magnus Ihse Bursie]

On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Require clang 13 in toolchain.m4

I think Kim's approach here -- to separate compiler upgrades from C++17 usage 
-- is the right way. Then we can discuss each part separately -- what version 
is suitable for each compiler, and then -- if or when we end up with all 
compilers supporting C++17 -- we can come back to the discussion of whether 
that is a good idea.

To be clear: my goal here was not as much the C++17 as getting a compiler bump, 
especially for getting rid of old buggy ones. I kind of piggybacked that 
discussion here, but once again, it is better to take that separately.

I also apologize for my hash expressions. :( I did not mean to insult anyone.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1948323482

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Andrew Haley]

On Fri, 16 Feb 2024 08:35:29 GMT, Andrew Haley  wrote:

> 
> To be clear: I do not object to this PR. I would like to use C++17.

Maybe the advantages of C++17 have been discussed elsewhere, in which case all 
we need is a link to that discussion on the Bug page. Maybe it's just that we 
like to stay current, in which case all we need to do is say so on the Bug page.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1947984619

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Andrew Haley]

On Thu, 15 Feb 2024 17:11:34 GMT, Thomas Stuefe  wrote:

> > Sure, you can always install a newer GCC than the system one, but it's 
> > another thing that makes it harder for people to build OpenJDK. Having said 
> > that, C++17 is nice to have.
> 
> @theRealAph I am still just hearing hand-waving "perhaps someone somewhere 
> will have a somewhat harder time building the JDK". 

I'm going to ignore that rather insulting language.

> Yes, perhaps that is so. But that is very uncertain, and I have still not 
> heard a single concrete example of where this would apply. In contrast, going 
> to gcc 10 will clearly bring a benefit to all other platforms, since it 
> allows us to synchronize the code base at C++17.

Well, hold on. You're implying that going to C++17 allows us to synchronize the 
code base at C++17. Sure, it does, but it's important also to discuss the 
pitfalls. And one of those pitfalls is that the system I'm typing this message 
on — still in support — won't be able to build the JDK without my first finding 
or building gcc 10 for it.

> In light of this, I think we need to hear some really compelling evidence of 
> problems that would ensue if we raise the minimum to gcc 10. If nobody can 
> produce such evidence, then to me it is a sign that this fear is not 
> well-grounded, and we should proceed with this PR.

As the proposer of this change, the onus is on you to show the benefit. 
Certainly there are C++17 advantages, such as hex float constants. better 
templates, and so on. I guess the discussion of such advantages must have taken 
place elsewhere because it's not on the 
https://bugs.openjdk.org/browse/JDK-8314488 either.

To be clear: I do not object to this PR. I would like to use C++17.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1947960141

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Thomas Stuefe]

On Mon, 5 Feb 2024 09:44:02 GMT, Magnus Ihse Bursie  wrote:

> > Sure, you can always install a newer GCC than the system one, but it's 
> > another thing that makes it harder for people to build OpenJDK. Having said 
> > that, C++17 is nice to have.
> 
> @theRealAph I am still just hearing hand-waving "perhaps someone somewhere 
> will have a somewhat harder time building the JDK". Yes, perhaps that is so. 
> But that is very uncertain, and I have still not heard a single concrete 
> example of where this would apply. In contrast, going to gcc 10 will clearly 
> bring a benefit to all other platforms, since it allows us to synchronize the 
> code base at C++17.
> 
> In light of this, I think we need to hear some really compelling evidence of 
> problems that would ensue if we raise the minimum to gcc 10. If nobody can 
> produce such evidence, then to me it is a sign that this fear is not 
> well-grounded, and we should proceed with this PR.

@magicus You put the onus of proving that problems could ensue strictly to the 
objectors. That is a bit one-sided. I do not see much effort - any, really - 
put into detailing the motivation for this move, neither in the PR description 
nor in the JBS issue text. I just read through the whole PR discussion and 
really the only helpful comment I found was from Kim ( 
https://github.com/openjdk/jdk/pull/14988#issuecomment-1858136247 ).

I think important decisions like enforcing C++17 would benefit from a more 
careful preparation.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1946628523

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Thomas Stuefe]

On Thu, 15 Feb 2024 15:54:56 GMT, Magnus Ihse Bursie  wrote:

> > I would like it if toolchain version bumps were discussed somewhere else 
> > first, not in a PR. (And apologies if it was and I missed that discussion).
> 
> Yes, it definitely was. I posted a separate [mail to 
> build-dev](https://mail.openjdk.org/pipermail/build-dev/2024-January/042802.html)
>  with subject "Raising the minimum version of gcc, clang and xlc". I don't 
> think we can make it more clear than that.

Okay, thank you for that clarification. I clearly missed it then.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1946396577

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Magnus Ihse Bursie]

On Thu, 15 Feb 2024 13:00:58 GMT, Thomas Stuefe  wrote:

> I would like it if toolchain version bumps were discussed somewhere else 
> first, not in a PR. (And apologies if it was and I missed that discussion). 

Yes, it definitely was. I posted a separate [mail to 
build-dev](https://mail.openjdk.org/pipermail/build-dev/2024-January/042802.html)
 with subject "Raising the minimum version of gcc, clang and xlc". I don't 
think we can make it more clear than that.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1946388775

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Thomas Stuefe]

On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Require clang 13 in toolchain.m4

Just on a general note:

I would like it if toolchain version bumps were discussed somewhere else first, 
not in a PR. (And apologies if it was and I missed that discussion). Because 
PRs are usually followed only by active developers, but toolchain versions 
affect a broader part of the community. As we have seen in this PR, there are 
conflicting interests.

We have things like CSRs and JEPs. Both are not ideal - a JEP is way too 
massive, and a CSR is about the compatibility of the product, not about 
build-time. Still, maybe something like a CSR would make sense here.

I understand that with toolchain support, there will always be opposing 
parties, and I can see arguments on both sides. We don't want to end up like 
FreeBSD - stuck in time because of opinion stalemates. I just keep thinking 
that a PR is maybe not the perfect forum for this.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1946053820

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Kim Barrett]

On Fri, 19 Jan 2024 14:04:50 GMT, Magnus Ihse Bursie  wrote:

> Well, the only additional thing this PR does except raise the compiler 
> version is to change the `--std` flag. It is a bit unclear what that means. 
> For the JDK libraries, there are already code present that relies on C++17. 
> For hotspot, what C++ constructions to use is strictly limited by the code 
> standard document. As long as it does not mention any C++17 constructs, it 
> does not really matter what the `--std` flag says. But, otoh, to be able to 
> say something about C++17, we need first have proper support from all 
> compilers.
> 
> So I'd say just chill a bit, give folks some time to respond. My 
> understanding of the situation is as follows:
> 
> * Raising clang to 13.0 is uncontroversial
> 
> * Raising xlc to 17.1.1.4 seems acceptable by the folks using it (I hope 
> I got that right)
> 
> * Raising gcc to 10.0 met some resistance. We could stop at gcc 9.0 for 
> this PR (which is enough for C++17), and then continue discussing going to 
> gcc 10.0 in a separate PR, or we can wait a bit more to see if @shipilev 
> feels compelled by the arguments given in the discussion to accept going to 
> 10.

I'd like to separate the version update discussions from C++17 specifics, so
we can have focused discussions on the version choices. Of course, that's
going to be informed by the possibility of C++17, but that's not the only
factor. That way this issue can be just about turning on C++17, when, and why.

In order to do that, I've filed the following issues about version updates:
https://bugs.openjdk.org/browse/JDK-8325881
Require minimum gcc version 10

https://bugs.openjdk.org/browse/JDK-8325878
Require minimum clang version 13

https://bugs.openjdk.org/browse/JDK-8325880
Require minimum OpenXL C/C++ version 17.1.1

PRs will follow shortly.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1944126546

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Magnus Ihse Bursie]

On Mon, 5 Feb 2024 09:17:11 GMT, Andrew Haley  wrote:

> Sure, you can always install a newer GCC than the system one, but it's 
> another thing that makes it harder for people to build OpenJDK. Having said 
> that, C++17 is nice to have.

@theRealAph  I am still just hearing hand-waving "perhaps someone somewhere 
will have a somewhat harder time building the JDK". Yes, perhaps that is so. 
But that is very uncertain, and I have still not heard a single concrete 
example of where this would apply. In contrast, going to gcc 10 will clearly 
bring a benefit to all other platforms, since it allows us to synchronize the 
code base at C++17.

In light of this, I think we need to hear some really compelling evidence of 
problems that would ensue if we raise the minimum to gcc 10. If nobody can 
produce such evidence, then to me it is a sign that this fear is not 
well-grounded, and we should proceed with this PR.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1926580836

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Andrew Haley]

On Wed, 17 Jan 2024 11:19:03 GMT, Magnus Ihse Bursie  wrote:

> We have been stuck on a very old gcc for a long time, due to various reasons. 
> Partly because old gcc versions were not as terrible as old versions of 
> cl.exe, and partly to support odd linux platforms where newer gcc versions 
> were not available.
> 
> It is tempting to raise the bar to get better functionality available on all 
> platforms. In the end, it is a balance between supporting older platforms, 
> and getting a better common language level for the code.
> 
> gcc 10 was released 3+ years ago. I guess that is good enough to consider it 
> a reasonable new minimum.

I guess so, but that sounds rather aggressive to me. Sure, you can always 
install a newer GCC than the system one, but it's another thing that makes it 
harder for people to build OpenJDK. Having said that, C++17 is nice to have.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1926533675

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Martin Doerr]

On Thu, 11 Jan 2024 13:04:35 GMT, Julian Waters  wrote:

> There is a typo in adlc:
> 
> ```
> diff --git a/make/hotspot/gensrc/GensrcAdlc.gmk 
> b/make/hotspot/gensrc/GensrcAdlc.gmk
> index 0898d91e1c2..bb356476847 100644
> --- a/make/hotspot/gensrc/GensrcAdlc.gmk
> +++ b/make/hotspot/gensrc/GensrcAdlc.gmk
> @@ -51,7 +51,7 @@ ifeq ($(call check-jvm-feature, compiler2), true)
>endif
>  
># Set the C++ standard
> -  ADLC_CFLAGS += $(ADLC_LANGSTD_CXXFLAG)
> +  ADLC_CFLAGS += $(ADLC_LANGSTD_CXXFLAGS)
>  
># NOTE: The old build didn't set -DASSERT for windows but it doesn't seem 
> to
># hurt.
> ```

Created a PR for that: https://github.com/openjdk/jdk/pull/17705

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1926450195

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Magnus Ihse Bursie]

On Fri, 19 Jan 2024 12:08:21 GMT, Julian Waters  wrote:

>> Julian Waters has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Require clang 13 in toolchain.m4
>
> Should I split the compiler upgrades into a different change and integrate 
> that first? Going off the conversation in this thread it would seem like the 
> compiler upgrade would benefit us a lot more than just having C++17 (The 
> noreturn attribute is one big motivating factor for instance) and it might 
> help if the compiler upgrades were not delayed by the discussion of when to 
> jump to C++17

Also, @TheShermanTanker, can you please incorporate this patch that updates the 
build documentation to match the changes in configure?


diff --git a/doc/building.html b/doc/building.html
index 7fd530e9dbc..bb5fda24ba7 100644
--- a/doc/building.html
+++ b/doc/building.html
@@ -599,14 +599,14 @@ Native 
Compiler
 All compilers are expected to be able to handle the C11 language
 standard for C, and C++14 for C++.
 gcc
-The minimum accepted version of gcc is 6.0. Older versions will not
+The minimum accepted version of gcc is 10.0. Older versions will not
 be accepted by configure.
 The JDK is currently known to compile successfully with gcc version
 13.2 or newer.
 In general, any version between these two should be usable.
 clang
-The minimum accepted version of clang is 3.5. Older versions will not
-be accepted by configure.
+The minimum accepted version of clang is 13.0. Older versions will
+not be accepted by configure.
 To use clang instead of gcc on Linux, use
 --with-toolchain-type=clang.
 Apple Xcode
@@ -672,7 +672,9 @@ Microsoft Visual 
Studio
 BuildTools, but e.g. Professional, adjust the
 product ID accordingly.
 IBM XL C/C++
-Please consult the AIX section of the The minimum accepted version of xlc is 17.1.1.4. Older versions will
+not be accepted by configure.
+For further information, please consult the AIX section of the https://wiki.openjdk.org/display/Build/Supported+Build+Platforms;>Supported
 Build Platforms OpenJDK Build Wiki page for details about which
 versions of XLC are supported.
diff --git a/doc/building.md b/doc/building.md
index de410439446..9113e0243bd 100644
--- a/doc/building.md
+++ b/doc/building.md
@@ -403,7 +403,7 @@ ## Native Compiler (Toolchain) Requirements
 
 ### gcc
 
-The minimum accepted version of gcc is 6.0. Older versions will not be accepted
+The minimum accepted version of gcc is 10.0. Older versions will not be 
accepted
 by `configure`.
 
 The JDK is currently known to compile successfully with gcc version 13.2 or
@@ -413,7 +413,7 @@ ### gcc
 
 ### clang
 
-The minimum accepted version of clang is 3.5. Older versions will not be
+The minimum accepted version of clang is 13.0. Older versions will not be
 accepted by `configure`.
 
 To use clang instead of gcc on Linux, use `--with-toolchain-type=clang`.
@@ -489,9 +489,12 @@ ### Microsoft Visual Studio
 
 ### IBM XL C/C++
 
-Please consult the AIX section of the [Supported Build Platforms](
-https://wiki.openjdk.org/display/Build/Supported+Build+Platforms) OpenJDK Build
-Wiki page for details about which versions of XLC are supported.
+The minimum accepted version of xlc is 17.1.1.4. Older versions will not be
+accepted by `configure`.
+
+For further information, please consult the AIX section of the [Supported Build
+Platforms](https://wiki.openjdk.org/display/Build/Supported+Build+Platforms)
+OpenJDK Build Wiki page for details about which versions of XLC are supported.
 
 ## Boot JDK Requirements

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1923565064

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Magnus Ihse Bursie]

On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Require clang 13 in toolchain.m4

Nobody complained about raising gcc to 10, see 
https://mail.openjdk.org/pipermail/build-dev/2024-January/042802.html.

@TheShermanTanker Can you update this PR so it sets the minimum gcc to 10.0 
instead of 9.0?

@shipilev Are you okay with us moving forward with this? If not, I'd like to 
see an concrete example of where this will create a problem, so we can discuss 
if the general good for the platform can be dictated by that use case.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1923553875

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Magnus Ihse Bursie]

On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Require clang 13 in toolchain.m4

Well, the only additional thing this PR does except raise the compiler version 
is to change the `--std` flag. It is a bit unclear what that means. For the JDK 
libraries, there are already code present that relies on C++17. For hotspot, 
what C++ constructions to use is strictly limited by the code standard 
document. As long as it does not mention any C++17 constructs, it does not 
really matter what the `--std` flag says. But, otoh, to be able to say 
something about C++17, we need first have proper support from all compilers.

So I'd say just chill a bit, give folks some time to respond. My understanding 
of the situation is as follows:

* Raising clang to 13.0 is uncontroversial
* Raising xlc to 17.1.1.4 seems acceptable by the folks using it (I hope I got 
that right)
* Raising gcc to 10.0 met some resistance. We could stop at gcc 9.0 for this PR 
(which is enough for C++17), and then continue discussing going to gcc 10.0 in 
a separate PR, or we can wait a bit more to see if @shipilev feels compelled by 
the arguments given in the discussion to accept going to 10.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1900481800

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Julian Waters]

On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Require clang 13 in toolchain.m4

Should I split the compiler upgrades into a different change and integrate that 
first? Going off the conversation in this thread it would seem like the 
compiler upgrade would benefit us a lot more than just having C++17 (The 
noreturn attribute is one big motivating factor for instance) and it might help 
if the compiler upgrades were not delayed by the discussion of when to jump to 
C++17

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1900300365

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Erik Joelsson]

On Wed, 17 Jan 2024 12:39:43 GMT, Aleksey Shipilev  wrote:

> For me, there is a huge question on portable JDK builds, which are usually 
> built with the lowest GCC toolchain possible to avoid GLIBC 
> incompatibilities. I am pretty sure currently built portable builds are _not_ 
> riding as high as GCC 10.

I'm not sure if you are referring to something else, but Oracle's builds of the 
JDK are intended to be portable and we are currently on GCC 13.2.0. There is no 
need to keep the GCC version low for portable builds, just the libs in the 
sysroot (which is where the GLIBC dependency is decided). We statically link 
libstdc++ and libgcc to avoid incompatibilities from GCC. This is why we use 
"devkits", to be able to combine a modern GCC with the lowest denominator 
sysroot that we need for our support matrix.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1895844654

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Aleksey Shipilev]

On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Require clang 13 in toolchain.m4

For me, there is a huge question on portable JDK builds, which are usually 
built with the lowest GCC toolchain possible to avoid GLIBC incompatibilities. 
I am pretty sure currently built portable builds are _not_ riding as high as 
GCC 10. 

Looks like the majority of C++17 features were implemented in GCC 6 and GCC 7: 
https://gcc.gnu.org/projects/cxx-status.html#cxx17, and how Kim mentions, C++17 
is no longer experimental for GCC 9. So maybe we should not rush GCC 10, and at 
most have GCC 9 as minimum.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1895729165

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Magnus Ihse Bursie]

On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Require clang 13 in toolchain.m4

We have been stuck on a very old gcc for a long time, due to various reasons. 
Partly because old gcc versions were not as terrible as old versions of cl.exe, 
and partly to support odd linux platforms where newer gcc versions were not 
available.

It is tempting to raise the bar to get better functionality available on all 
platforms. In the end, it is a balance between supporting older platforms, and 
getting a better common language level for the code. 

gcc 10 was released 3+ years ago. I guess that is good enough to consider it a 
reasonable new minimum.

I will make a separate announcement on the build-dev list to draw attention to 
the fact that we want to raise minimum compiler versions, which might not be 
apparent from the title of this PR, to give folks a better chance at voicing 
concerns.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1895603901

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Kim Barrett]

On Fri, 15 Dec 2023 15:52:07 GMT, Kim Barrett  wrote:

> gcc: https://gcc.gnu.org/gcc-9/changes.html
> "The C++17 implementation is no longer experimental."

Bumping to gcc10 rather than gcc9 would have the benefit that we could get a 
work-alike for C++20
`std::is_constant_evaluated` even though we're not otherwise using C++20.  
Sufficiently recent versions
of all of our supported compilers provide `__builtin_is_constant_evaluated`.  
That first shows up in gcc10.
We're already requiring versions of clang and msvc that provide it.  There are 
a bunch of potential improvements
we could make by having such a work-alike.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1895511862

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Magnus Ihse Bursie]

On Thu, 11 Jan 2024 13:04:35 GMT, Julian Waters  wrote:

> I can't believe I missed something so obvious

Don't blame yourself. No-one has noticed for the at least 3 years the code has 
been present. :)

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-174780

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Kim Barrett]

On Fri, 12 Jan 2024 07:49:17 GMT, Matthias Baesken  wrote:

> We at SAP use and document xlC 17.1.1.4 for jdk22 (use the same for jdk23) 
> https://wiki.openjdk.org/display/Build/Supported+Build+Platforms
> 
> version 17.1.1.4 is already clang15 (at least that's what the compiler output 
> is telling me)
> 
> /opt/IBM/openxlC/17.1.1/bin/ibm-clang++_r -v IBM Open XL C/C++ for AIX 17.1.1 
> (5725-C72, 5765-J18), version 17.1.1.4, clang version 15.0.0 (build ca7115e) 
> Target: powerpc-ibm-aix7.2.0.0

My mistake, you are correct.  17.1.0 seems to be clang 13, 17.1.1 seems to be 
clang 15, and 17.1.2 seems to be clang 17.
All of those are based on the documented value of the `__VERSION__` macro's 
string value.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1888731423

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Matthias Baesken]

On Fri, 12 Jan 2024 06:32:34 GMT, Kim Barrett  wrote:

> > Thanks! We may switch to clang 14 on MacOS at some point of time, but it's 
> > better to have that disentangled. Some people build JDK 11 and 23 on the 
> > same machine and that is easier if they don't have to switch Xcode.
> 
> I think the minimum clang version should not be greater than what’s provided 
> by the minimum Open XL C/C++ version.
> 
> If the aix-ppc port only requires Open XL C/C++ 17.1.1 then that’s clang 13. 
> If the aix-ppc port were to instead jump further forward, to 17.1.2, then 
> that’s clang 15.
> 
> I've asked the aix-ppc folks if requiring 17.1.2 would be okay, but haven't 
> heard back yet.

We at SAP use  and document xlC  17.1.1.4   for jdk22 (use the same for jdk23)
https://wiki.openjdk.org/display/Build/Supported+Build+Platforms

version 17.1.1.4  is already clang15 (at least that's what the compiler output 
is telling me) 

/opt/IBM/openxlC/17.1.1/bin/ibm-clang++_r -v
IBM Open XL C/C++ for AIX 17.1.1 (5725-C72, 5765-J18), version 17.1.1.4, clang 
version 15.0.0 (build ca7115e)
Target: powerpc-ibm-aix7.2.0.0

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1888583559

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Kim Barrett]

On Thu, 11 Jan 2024 14:28:20 GMT, Martin Doerr  wrote:

> Thanks! We may switch to clang 14 on MacOS at some point of time, but it's 
> better to have that disentangled. Some people build JDK 11 and 23 on the same 
> machine and that is easier if they don't have to switch Xcode.

I think the minimum clang version should not be greater than what’s provided by 
the minimum Open XL C/C++ version.

If the aix-ppc port only requires Open XL C/C++ 17.1.1 then that’s clang 13.
If the aix-ppc port were to instead jump further forward, to 17.1.2, then 
that’s clang 15.

I've asked the aix-ppc folks if requiring 17.1.2 would be okay, but haven't 
heard back yet.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1888507915

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Martin Doerr]

On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Require clang 13 in toolchain.m4

Thanks! We may switch to clang 14 on MacOS at some point of time, but it's 
better to have that disentangled. Some people build JDK 11 and 23 on the same 
machine and that is easier if they don't have to switch Xcode.

-

Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14988#pullrequestreview-1815729317

Re: RFR: 8314488: Compile the JDK as C++17 [v6]

[Julian Waters]

> Compile the JDK as C++17, enabling the use of all C++17 language features

Julian Waters has updated the pull request incrementally with one additional 
commit since the last revision:

  Require clang 13 in toolchain.m4

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14988/files
  - new: https://git.openjdk.org/jdk/pull/14988/files/4f196292..50ffeeea

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14988=05
 - incr: https://webrevs.openjdk.org/?repo=jdk=14988=04-05

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/14988.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14988/head:pull/14988

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

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Julian Waters]

On Thu, 11 Jan 2024 12:58:43 GMT, Magnus Ihse Bursie  wrote:

> There is a typo in adlc:
> 
> ```
> diff --git a/make/hotspot/gensrc/GensrcAdlc.gmk 
> b/make/hotspot/gensrc/GensrcAdlc.gmk
> index 0898d91e1c2..bb356476847 100644
> --- a/make/hotspot/gensrc/GensrcAdlc.gmk
> +++ b/make/hotspot/gensrc/GensrcAdlc.gmk
> @@ -51,7 +51,7 @@ ifeq ($(call check-jvm-feature, compiler2), true)
>endif
>  
># Set the C++ standard
> -  ADLC_CFLAGS += $(ADLC_LANGSTD_CXXFLAG)
> +  ADLC_CFLAGS += $(ADLC_LANGSTD_CXXFLAGS)
>  
># NOTE: The old build didn't set -DASSERT for windows but it doesn't seem 
> to
># hurt.
> ```

*Facepalm... I can't believe I missed something so obvious

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1887124280

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Magnus Ihse Bursie]

On Wed, 10 Jan 2024 13:11:38 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove unnecessary -std=c++17 option in Lib.gmk

There is a typo in adlc:


diff --git a/make/hotspot/gensrc/GensrcAdlc.gmk 
b/make/hotspot/gensrc/GensrcAdlc.gmk
index 0898d91e1c2..bb356476847 100644
--- a/make/hotspot/gensrc/GensrcAdlc.gmk
+++ b/make/hotspot/gensrc/GensrcAdlc.gmk
@@ -51,7 +51,7 @@ ifeq ($(call check-jvm-feature, compiler2), true)
   endif
 
   # Set the C++ standard
-  ADLC_CFLAGS += $(ADLC_LANGSTD_CXXFLAG)
+  ADLC_CFLAGS += $(ADLC_LANGSTD_CXXFLAGS)
 
   # NOTE: The old build didn't set -DASSERT for windows but it doesn't seem to
   # hurt.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1887114509

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Julian Waters]

On Thu, 11 Jan 2024 12:47:25 GMT, Martin Doerr  wrote:

> @TheRealMDoerr The adlc build is notoriously problematic, since it does not 
> share the common flags set for JVM or JDK native compilation. :( So your 
> suggestion sounds highly likely to me. Running with LOG=cmdlines will confirm 
> this.
> 
> (This can be done on GHA by manually starting a run, and setting the value of 
> "Additional make arguments" to `LOG=cmdlines` or possibly `LOG=info,cmdlines`)

Doesn't ADLC share the same compilation standard options as the rest of the 
codebase though? 
https://github.com/openjdk/jdk/blob/e5aed6be7a184a86a32fa671d48e0781fab54183/make/autoconf/flags-cflags.m4#L587



> > @TheRealMDoerr The adlc build is notoriously problematic, since it does not 
> > share the common flags set for JVM or JDK native compilation. :( So your 
> > suggestion sounds highly likely to me. Running with LOG=cmdlines will 
> > confirm this.
> > (This can be done on GHA by manually starting a run, and setting the value 
> > of "Additional make arguments" to `LOG=cmdlines` or possibly 
> > `LOG=info,cmdlines`)
> 
> Thanks for the hint! The command line is also shown here: 
> make-support/failure-logs/hotspot_variant-server_tools_adlc_objs_adlparse.o.cmdline
>  The -std option is not passed. That seems to be the issue. So, this is not a 
> clang 13 vs 14 thing.

Something is very wrong in that case, they're supposed be be set here: 
https://github.com/openjdk/jdk/blob/e5aed6be7a184a86a32fa671d48e0781fab54183/make/hotspot/gensrc/GensrcAdlc.gmk#L54

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-188710

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Martin Doerr]

On Thu, 11 Jan 2024 12:36:31 GMT, Martin Doerr  wrote:

>> Regarding 
>> https://github.com/TheShermanTanker/jdk/actions/runs/7070564987/job/19247370401,
>>  could it be that the adlc build didn't get the correct C++ version flags? 
>> It doesn't look like a clang 13 specific problem.
>
>> @TheRealMDoerr
>> 
>> > The only issue I see is requiring clang 14.0 on MacOS is not in sync with 
>> > "Other JDK 22 build platforms" 
>> > (https://wiki.openjdk.org/display/Build/Supported+Build+Platforms).
>> 
>> That page is suppose to document what we actually do, not be a binding 
>> contract; so if we change stuff, we update the page to reflect it, rather 
>> than the other way around.
>> 
>> Or maybe I misunderstood your comment?
> 
> Correct, but raising requirements requires extra effort to change the build 
> environments, updating docs, etc. (It may even cause incompatibilities. 
> Probably not in this case.) While it may be better to use a newer Xcode on 
> Mac, I can't see sufficient reason for forcing the whole world to build with 
> clang 14.

> @TheRealMDoerr The adlc build is notoriously problematic, since it does not 
> share the common flags set for JVM or JDK native compilation. :( So your 
> suggestion sounds highly likely to me. Running with LOG=cmdlines will confirm 
> this.
> 
> (This can be done on GHA by manually starting a run, and setting the value of 
> "Additional make arguments" to `LOG=cmdlines` or possibly `LOG=info,cmdlines`)

Thanks for the hint! The command line is also shown here: 
make-support/failure-logs/hotspot_variant-server_tools_adlc_objs_adlparse.o.cmdline
The -std option is not passed. That seems to be the issue. So, this is not a 
clang 13 vs 14 thing.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1887095112

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Martin Doerr]

On Thu, 11 Jan 2024 11:33:07 GMT, Martin Doerr  wrote:

>> Julian Waters has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove unnecessary -std=c++17 option in Lib.gmk
>
> Regarding 
> https://github.com/TheShermanTanker/jdk/actions/runs/7070564987/job/19247370401,
>  could it be that the adlc build didn't get the correct C++ version flags? It 
> doesn't look like a clang 13 specific problem.

> @TheRealMDoerr
> 
> > The only issue I see is requiring clang 14.0 on MacOS is not in sync with 
> > "Other JDK 22 build platforms" 
> > (https://wiki.openjdk.org/display/Build/Supported+Build+Platforms).
> 
> That page is suppose to document what we actually do, not be a binding 
> contract; so if we change stuff, we update the page to reflect it, rather 
> than the other way around.
> 
> Or maybe I misunderstood your comment?

Correct, but raising requirements requires extra effort to change the build 
environments, updating docs, etc. (It may even cause incompatibilities. 
Probably not in this case.) While it may be better to use a newer Xcode on Mac, 
I can't see sufficient reason for forcing the whole world to build with clang 
14.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1887072454

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Magnus Ihse Bursie]

On Wed, 10 Jan 2024 13:11:38 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove unnecessary -std=c++17 option in Lib.gmk

Also please note that if the minimum version of the compilers are bumped in the 
configure script, the documentation in doc/building.md needs to be updated to 
match this as well. (And building.html file needs to be regenerated.)

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1887049046

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Magnus Ihse Bursie]

On Thu, 11 Jan 2024 11:33:07 GMT, Martin Doerr  wrote:

>> Julian Waters has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove unnecessary -std=c++17 option in Lib.gmk
>
> Regarding 
> https://github.com/TheShermanTanker/jdk/actions/runs/7070564987/job/19247370401,
>  could it be that the adlc build didn't get the correct C++ version flags? It 
> doesn't look like a clang 13 specific problem.

@TheRealMDoerr 

> The only issue I see is requiring clang 14.0 on MacOS is not in sync with 
> "Other JDK 22 build platforms" 
> (https://wiki.openjdk.org/display/Build/Supported+Build+Platforms).

That page is suppose to document what we actually do, not be a binding 
contract; so if we change stuff, we update the page to reflect it, rather than 
the other way around. 

Or maybe I misunderstood your comment?

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1887044785

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Magnus Ihse Bursie]

On Thu, 11 Jan 2024 11:33:07 GMT, Martin Doerr  wrote:

>> Julian Waters has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove unnecessary -std=c++17 option in Lib.gmk
>
> Regarding 
> https://github.com/TheShermanTanker/jdk/actions/runs/7070564987/job/19247370401,
>  could it be that the adlc build didn't get the correct C++ version flags? It 
> doesn't look like a clang 13 specific problem.

@TheRealMDoerr The adlc build is notoriously problematic, since it does not 
share the common flags set for JVM or JDK native compilation. :( So your 
suggestion sounds highly likely to me. Running with LOG=cmdlines will confirm 
this.

(This can be done on GHA by manually starting a run, and setting the value of 
"Additional make arguments" to `LOG=cmdlines` or possibly `LOG=info,cmdlines`)

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1887040102

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Martin Doerr]

On Wed, 10 Jan 2024 13:11:38 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove unnecessary -std=c++17 option in Lib.gmk

Regarding 
https://github.com/TheShermanTanker/jdk/actions/runs/7070564987/job/19247370401,
 could it be that the adlc build didn't get the correct C++ version flags? It 
doesn't look like a clang 13 specific problem.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1886947773

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Kim Barrett]

On Thu, 11 Jan 2024 08:04:40 GMT, Julian Waters  wrote:

> > Hi Martin, probably we can update our devkit if really needed. But 
> > https://clang.llvm.org/cxx_status.html states that c++17 is supported for a 
> > very long time, so probably clang 13.1 is sufficient too (or is there a 
> > real showstopper known with this release of clang) .
> 
> I was hoping to avoid 13.x since there seems to be a noexcept bug in that 
> release series, though some other testing seems to suggest this is transient 
> (and also I wanted to align with what Oracle uses, which is 14.x). I guess I 
> can roll back to 13.x if that is really needed

See https://bugs.openjdk.org/browse/JDK-8255082, comments in 12/2023.

> > P0283R2: Ignoring unsupported non-standard attributes
> 
> It's probably important to note that MSVC takes this to mean that unknown 
> attributes don't have an effect, and still warns for them when warning C5030 
> is enabled (which is by default in our make system): 
> https://developercommunity.visualstudio.com/t/c-warning-c5030-generated-for-attribute-within-a-n/138429

So VS doesn't have a mechanism for disabling warnings about just scoped 
attributes it doesn't recognize.  (gcc has
-Wno-attributes=_vendor_:: for this.)  That's unfortunate.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1886727940

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Julian Waters]

On Wed, 10 Jan 2024 13:53:47 GMT, Matthias Baesken  wrote:

> Hi Martin, probably we can update our devkit if really needed. But 
> https://clang.llvm.org/cxx_status.html states that c++17 is supported for a 
> very long time, so probably clang 13.1 is sufficient too (or is there a real 
> showstopper known with this release of clang) .

I was hoping to avoid 13.x since there seems to be a noexcept bug in that 
release series, though some other testing seems to suggest this is transient 
(and also I wanted to align with what Oracle uses, which is 14.x). I guess I 
can roll back to 13.x if that is really needed



> P0283R2: Ignoring unsupported non-standard attributes

It's probably important to note that MSVC takes this to mean that unknown 
attributes don't have an effect, and still warns for them when warning C5030 is 
enabled (which is by default in our make system): 
https://developercommunity.visualstudio.com/t/c-warning-c5030-generated-for-attribute-within-a-n/138429

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1886575743

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Matthias Baesken]

On Wed, 10 Jan 2024 13:11:38 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove unnecessary -std=c++17 option in Lib.gmk

Hi Martin, probably we can update our devkit if really needed.
But 
https://clang.llvm.org/cxx_status.html
states that c++17 is supported for a very long time, so probably  clang 13.1 is 
sufficient too (or is there a real showstopper known with this release) .

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1884892708

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Martin Doerr]

On Wed, 10 Jan 2024 13:11:38 GMT, Julian Waters  wrote:

>> Compile the JDK as C++17, enabling the use of all C++17 language features
>
> Julian Waters has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove unnecessary -std=c++17 option in Lib.gmk

Looks basically still good. The only issue I see is requiring clang 14.0 on 
MacOS is not in sync with "Other JDK 22 build platforms" 
(https://wiki.openjdk.org/display/Build/Supported+Build+Platforms). @MBaesken: 
Do you know if we can use a newer clang?

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1884882004

Re: RFR: 8314488: Compile the JDK as C++17 [v5]

[Julian Waters]

> Compile the JDK as C++17, enabling the use of all C++17 language features

Julian Waters has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove unnecessary -std=c++17 option in Lib.gmk

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14988/files
  - new: https://git.openjdk.org/jdk/pull/14988/files/f1a644e3..4f196292

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14988=04
 - incr: https://webrevs.openjdk.org/?repo=jdk=14988=03-04

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/14988.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14988/head:pull/14988

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

Re: RFR: 8314488: Compile the JDK as C++17 [v4]

[Julian Waters]

> Compile the JDK as C++17, enabling the use of all C++17 language features

Julian Waters has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains eight additional 
commits since the last revision:

 - Merge branch 'openjdk:master' into patch-7
 - Compiler versions in toolchain.m4
 - Merge branch 'openjdk:master' into patch-7
 - Merge branch 'openjdk:master' into patch-7
 - Revert vm_version_linux_riscv.cpp
 - vm_version_linux_riscv.cpp
 - allocation.cpp
 - 8310260

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14988/files
  - new: https://git.openjdk.org/jdk/pull/14988/files/477f6b94..f1a644e3

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14988=03
 - incr: https://webrevs.openjdk.org/?repo=jdk=14988=02-03

  Stats: 20857 lines in 728 files changed: 13941 ins; 3659 del; 3257 mod
  Patch: https://git.openjdk.org/jdk/pull/14988.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14988/head:pull/14988

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

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Kim Barrett]

On Fri, 15 Dec 2023 16:20:02 GMT, Kim Barrett  wrote:

>> Julian Waters has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains five additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into patch-7
>>  - Revert vm_version_linux_riscv.cpp
>>  - vm_version_linux_riscv.cpp
>>  - allocation.cpp
>>  - 8310260
>
> I agree that before throwing this switch, we need to look at some specific
> issues that might need to be addressed, discuss the benefits, and also the
> costs.
> 
> As was discussed for the change to C++14, there is *never* a good time to
> start introducing the use of new language features as far as backporting is
> concerned, unless one is going to backport the language change too. We didn't
> do that for C++14, and I don't think we are going to (nor should) do it for
> C++17 either.  But backporting concerns can't be all powerful, as that will
> forever prevent potentially significant improvements.
> 
> I started to make a list of new language features that seem particularly
> beneficial or otherwise important.  I was going to write style guide updates
> for these, but haven't gotten very far with that yet.
> 
> P0035R4: Dynamic memory allocation for over-aligned data
> P0135R1: Guaranteed copy elision
> P0145R3: Refining Expression Evaluation Order for Idiomatic C++
> P0292R2: constexpr if
> P0091R3/P0512R0: Template argument deduction for class templates
> 
> Here are some others that might be of interest to us.
> N4268: Allow constant evaluation for all non-type template arguments
> N3928: Extending static_assert
> P0118R1: [[fallthrough]] attribute
> P0189R1: [[nodiscard]] attribute
> P0212R1: [[maybe_unused]] attribute
> P0170R1: Wording for constexpr lambda
> P0283R2: Ignoring unsupported non-standard attributes 
> P0061R1: __has_include for C++17
> P0386R2: Inline variables

> @kimbarrett
> 
> > P0035R4: Dynamic memory allocation for over-aligned data
> 
> Do we really need this? I ask because, in the end, this will result in 
> something like `posix_memalign` to be called, and I remember it being 
> notorious for causing large footprint overhead depending on how smart the 
> underlying allocator is about using alignment waste.
> 
> It will also be non-trivial to implement in hotspot since NMT uses malloc 
> headers. Barring a rewrite of NMT malloc metadata tracking (e.g. using a hash 
> map, which would be more costly both in terms of performance and, probably, 
> footprint), malloc headers would have to be revised. Probably would need to 
> be dynamic-sized. This is the reason we did not bother wrapping 
> posix_memalign.

We already have code that (incorrectly) expects dynamic allocation to support
overalignment. There are several classes that overalign (often cache align) a
member to avoid false sharing, but are dynamically allocated. See most (all?)
uses of ZCACHE_ALIGNED for some examples.

One way to fix this would be to give those classes their own operator new to
perform aligned allocation somehow. That's what was done for
OopStorage::Block, but it's clumsy and likely wasteful of memory. And it's
easy to forget. A general solution would probably be better. But yes, NMT
malloc headers certainly make the general problem challenging. The approach
currently used for OopStorage::Block can be generalized and hooked into the
standard mechanism.  But maybe there are (possibly non-portable) alternatives
that avoid the memory waste?

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1864375915

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Kim Barrett]

On Fri, 15 Dec 2023 13:52:28 GMT, Martin Doerr  wrote:

> In case you want to update the required compiler versions as part of this PR: 
> We have tested -TOOLCHAIN_MINIMUM_VERSION_xlc="16.1.0.0011" 
> +TOOLCHAIN_MINIMUM_VERSION_xlc="17.1.1.4" (Already discussed with Kim.)

Also discussed with the aix-ppc port maintainers at IBM.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1864380101

Re: RFR: 8314488: Compile the JDK as C++17

[Kim Barrett]

On Sat, 19 Aug 2023 07:45:50 GMT, Andrew Haley  wrote:

> Is it impractical to drop the obsolete features of C++11, working in the 
> common subset of C++11 and C++17?

I'm not sure what is being suggested.  Maybe some examples would help.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1864383047

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Thomas Stuefe]

On Fri, 15 Dec 2023 16:20:02 GMT, Kim Barrett  wrote:

>> Julian Waters has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains five additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into patch-7
>>  - Revert vm_version_linux_riscv.cpp
>>  - vm_version_linux_riscv.cpp
>>  - allocation.cpp
>>  - 8310260
>
> I agree that before throwing this switch, we need to look at some specific
> issues that might need to be addressed, discuss the benefits, and also the
> costs.
> 
> As was discussed for the change to C++14, there is *never* a good time to
> start introducing the use of new language features as far as backporting is
> concerned, unless one is going to backport the language change too. We didn't
> do that for C++14, and I don't think we are going to (nor should) do it for
> C++17 either.  But backporting concerns can't be all powerful, as that will
> forever prevent potentially significant improvements.
> 
> I started to make a list of new language features that seem particularly
> beneficial or otherwise important.  I was going to write style guide updates
> for these, but haven't gotten very far with that yet.
> 
> P0035R4: Dynamic memory allocation for over-aligned data
> P0135R1: Guaranteed copy elision
> P0145R3: Refining Expression Evaluation Order for Idiomatic C++
> P0292R2: constexpr if
> P0091R3/P0512R0: Template argument deduction for class templates
> 
> Here are some others that might be of interest to us.
> N4268: Allow constant evaluation for all non-type template arguments
> N3928: Extending static_assert
> P0118R1: [[fallthrough]] attribute
> P0189R1: [[nodiscard]] attribute
> P0212R1: [[maybe_unused]] attribute
> P0170R1: Wording for constexpr lambda
> P0283R2: Ignoring unsupported non-standard attributes 
> P0061R1: __has_include for C++17
> P0386R2: Inline variables

@kimbarrett 
> P0035R4: Dynamic memory allocation for over-aligned data

Do we really need this? I ask because, in the end, this will result in 
something like `posix_memalign` to be called, and I remember it being notorious 
for causing large footprint overhead depending on how smart the underlying 
allocator is about using alignment waste.

It will also be non-trivial to implement in hotspot since NMT uses malloc 
headers. Barring a rewrite of NMT malloc metadata tracking (e.g. using a hash 
map, which would be more costly both in terms of performance and, probably, 
footprint), malloc headers would have to be revised. Probably would need to be 
dynamic-sized. This is the reason we did not bother wrapping posix_memalign.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1864039399

Re: RFR: 8314488: Compile the JDK as C++17 [v3]

[Julian Waters]

> Compile the JDK as C++17, enabling the use of all C++17 language features

Julian Waters has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains seven additional 
commits since the last revision:

 - Compiler versions in toolchain.m4
 - Merge branch 'openjdk:master' into patch-7
 - Merge branch 'openjdk:master' into patch-7
 - Revert vm_version_linux_riscv.cpp
 - vm_version_linux_riscv.cpp
 - allocation.cpp
 - 8310260

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14988/files
  - new: https://git.openjdk.org/jdk/pull/14988/files/a1f21bbd..477f6b94

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14988=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=14988=01-02

  Stats: 2377 lines in 158 files changed: 1669 ins; 268 del; 440 mod
  Patch: https://git.openjdk.org/jdk/pull/14988.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14988/head:pull/14988

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

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Andrew Haley]

On Fri, 15 Dec 2023 13:05:32 GMT, Julian Waters  wrote:

>> The keyword also happens to go in the same location as well. How 
>> coincidental...
>
> I also realized that this uses a gcc statement expression currently, I wonder 
> if this could use a lambda expression instead in another change?

That's the standard idiom for accessing CSR, as used in libc and elsewhere. I 
think that avoiding divergence with such other sources is probably more 
important than some notion of standard purity.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1428804231

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Kim Barrett]

On Fri, 15 Dec 2023 08:08:10 GMT, Julian Waters  wrote:

>> Implementation of [JEP draft: Compile the JDK as 
>> C++17](https://bugs.openjdk.org/browse/JDK-8310260)
>
> Julian Waters has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into patch-7
>  - Revert vm_version_linux_riscv.cpp
>  - vm_version_linux_riscv.cpp
>  - allocation.cpp
>  - 8310260

I agree that before throwing this switch, we need to look at some specific
issues that might need to be addressed, discuss the benefits, and also the
costs.

As was discussed for the change to C++14, there is *never* a good time to
start introducing the use of new language features as far as backporting is
concerned, unless one is going to backport the language change too. We didn't
do that for C++14, and I don't think we are going to (nor should) do it for
C++17 either.  But backporting concerns can't be all powerful, as that will
forever prevent potentially significant improvements.

I started to make a list of new language features that seem particularly
beneficial or otherwise important.  I was going to write style guide updates
for these, but haven't gotten very far with that yet.

P0035R4: Dynamic memory allocation for over-aligned data
P0135R1: Guaranteed copy elision
P0145R3: Refining Expression Evaluation Order for Idiomatic C++
P0292R2: constexpr if
P0091R3/P0512R0: Template argument deduction for class templates

Here are some others that might be of interest to us.
N4268: Allow constant evaluation for all non-type template arguments
N3928: Extending static_assert
P0118R1: [[fallthrough]] attribute
P0189R1: [[nodiscard]] attribute
P0212R1: [[maybe_unused]] attribute
P0170R1: Wording for constexpr lambda
P0283R2: Ignoring unsupported non-standard attributes 
P0061R1: __has_include for C++17
P0386R2: Inline variables

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1858136247

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Kim Barrett]

On Fri, 15 Dec 2023 08:08:10 GMT, Julian Waters  wrote:

>> Implementation of [JEP draft: Compile the JDK as 
>> C++17](https://bugs.openjdk.org/browse/JDK-8310260)
>
> Julian Waters has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into patch-7
>  - Revert vm_version_linux_riscv.cpp
>  - vm_version_linux_riscv.cpp
>  - allocation.cpp
>  - 8310260

In conjunction with changing to C++17, I suggest the following changes to the
minimum compiler versions, as indicated in make/autoconf/toolchain.m4

old:
make/autoconf/toolchain.m4
TOOLCHAIN_MINIMUM_VERSION_clang="3.5"
TOOLCHAIN_MINIMUM_VERSION_gcc="6.0"
TOOLCHAIN_MINIMUM_VERSION_microsoft="19.28.0.0" # VS2019 16.8, aka MSVC 14.28
TOOLCHAIN_MINIMUM_VERSION_xlc="16.1.0.0011"

proposed new:
TOOLCHAIN_MINIMUM_VERSION_clang="13.0"
TOOLCHAIN_MINIMUM_VERSION_gcc="9.0"
TOOLCHAIN_MINIMUM_VERSION_microsoft="19.28.0.0" # VS2019 16.8, aka MSVC 14.28
TOOLCHAIN_MINIMUM_VERSION_xlc="17.1.1.4"

Here's the rationale for each of these:

- 
gcc:
https://gcc.gnu.org/gcc-9/changes.html
"The C++17 implementation is no longer experimental."

- 
open xl c++ for aix
https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.1?topic=features-supported-language-levels
supports C17 and C++17, with experimental support for C++20
17.1.0 docs explicitly says __clang_version__ is 13.0.0, with the other
version macros set accordingly.  17.1.1 just describes the version macros, but
doesn't say what their values are.  But the __VERSION__ macro description
includes "Clang 15.0.0" in the string.
Note that there is now a 17.1.2 version, but the aix-ppc porters haven't 
proposed
going that far.

- 
Visual Studio
https://learn.microsoft.com/en-gb/cpp/overview/visual-cpp-language-conformance?view=msvc-170
We already require VS2019 16.8, which covers all of C++17 features listed on
that page.

- 
clang
https://clang.llvm.org/cxx_status.html
c++17 - Clang 5

However, there is a critical bug for which we really want a fix. Using
[[noreturn]] seems to be buggy and leads to crashes. This has been seen with
clang 12. It appears to be fixed with clang 13.0.0 (Xcode 13.0).

There may also be a bug somewhere in the 13.x release series with the handling
of noexcept.  See discussion in https://bugs.openjdk.org/browse/JDK-8255082.

Oracle is currently using Xcode 14.3.1 (clang 14.0.3), so I think we wouldn't
object to something between 13.0.0 and 14.0.3 as the minimum version.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1858097523

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Martin Doerr]

On Fri, 15 Dec 2023 08:08:10 GMT, Julian Waters  wrote:

>> Implementation of [JEP draft: Compile the JDK as 
>> C++17](https://bugs.openjdk.org/browse/JDK-8310260)
>
> Julian Waters has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into patch-7
>  - Revert vm_version_linux_riscv.cpp
>  - vm_version_linux_riscv.cpp
>  - allocation.cpp
>  - 8310260

In case you want to update the required compiler versions as part of this PR: 
We have tested
-TOOLCHAIN_MINIMUM_VERSION_xlc="16.1.0.0011"
+TOOLCHAIN_MINIMUM_VERSION_xlc="17.1.1.4"
(Already discussed with Kim.)

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1857916995

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Julian Waters]

On Fri, 15 Dec 2023 12:56:07 GMT, Julian Waters  wrote:

>> It's not that it overrides the warning.  They are different syntactic 
>> constructs that just happen to have
>> a word in common.  The joys of parsing C++ ...
>
> The keyword also happens to go in the same location as well. How 
> coincidental...

I also realized that this uses a gcc statement expression currently, I wonder 
if this could use a lambda expression instead in another change?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1427955879

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Julian Waters]

On Fri, 15 Dec 2023 12:31:51 GMT, Kim Barrett  wrote:

>> Ah, I was not aware that the asm specifications for explicit registers 
>> overrides the warning about the register keyword, thanks!
>
> It's not that it overrides the warning.  They are different syntactic 
> constructs that just happen to have
> a word in common.  The joys of parsing C++ ...

The keyword also happens to go in the same location as well. How coincidental...

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1427947238

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Kim Barrett]

On Fri, 15 Dec 2023 11:25:24 GMT, Julian Waters  wrote:

>> Looks like this change has also already been made, by JDK-8319440.
>> 
>> All of the other non-comment uses of "register" I found in HotSpot are gcc 
>> local variable register specifications:
>> https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Local-Register-Variables.html
>> So are a different thing and not affected by the deprecation/removal of the 
>> C++ "register" keyword.
>
> Ah, I was not aware that the asm specifications for explicit registers 
> overrides the warning about the register keyword, thanks!

It's not that it overrides the warning.  They are different syntactic 
constructs that just happen to have
a word in common.  The joys of parsing C++ ...

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1427925300

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Julian Waters]

On Fri, 15 Dec 2023 09:05:37 GMT, Kim Barrett  wrote:

>> There are, strangely, many more register keywords in the JDK codebase than 
>> just this one, but none of them throw the same errors, only this one does
>
> Looks like this change has also already been made, by JDK-8319440.
> 
> All of the other non-comment uses of "register" I found in HotSpot are gcc 
> local variable register specifications:
> https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Local-Register-Variables.html
> So are a different thing and not affected by the deprecation/removal of the 
> C++ "register" keyword.

Ah, I was not aware that the asm specifications for explicit registers 
overrides the warning about the register keyword, thanks!

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1427866744

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Kim Barrett]

On Fri, 15 Dec 2023 08:12:47 GMT, Julian Waters  wrote:

>> No problem!
>
> There are, strangely, many more register keywords in the JDK codebase than 
> just this one, but none of them throw the same errors, only this one does

Looks like this change has also already been made, by JDK-8319440.

All of the other non-comment uses of "register" I found in HotSpot are gcc 
local variable register specifications:
https://gcc.gnu.org/onlinedocs/gcc-13.2.0/gcc/Local-Register-Variables.html
So are a different thing and not affected by the deprecation/removal of the C++ 
"register" keyword.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1427728697

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Julian Waters]

On Fri, 15 Dec 2023 08:03:45 GMT, Julian Waters  wrote:

>> src/hotspot/os_cpu/linux_riscv/vm_version_linux_riscv.cpp line 77:
>> 
>>> 75: #define read_csr(csr)   \
>>> 76: ({  \
>>> 77: unsigned long __v;  \
>> 
>> Can this change be made separately?  I'd like to have the C++17 switch be as 
>> clean as possible.
>
> No problem!

There are, strangely, many more register keywords in the JDK codebase than just 
this one, but none of them throw the same errors, only this one does

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1427676677

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Julian Waters]

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

Julian Waters has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains five additional 
commits since the last revision:

 - Merge branch 'openjdk:master' into patch-7
 - Revert vm_version_linux_riscv.cpp
 - vm_version_linux_riscv.cpp
 - allocation.cpp
 - 8310260

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/14988/files
  - new: https://git.openjdk.org/jdk/pull/14988/files/09cecd7e..a1f21bbd

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=14988=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=14988=00-01

  Stats: 1027205 lines in 9267 files changed: 298949 ins; 587677 del; 140579 mod
  Patch: https://git.openjdk.org/jdk/pull/14988.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14988/head:pull/14988

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

Re: RFR: 8314488: Compile the JDK as C++17 [v2]

[Julian Waters]

On Fri, 15 Dec 2023 07:48:46 GMT, Kim Barrett  wrote:

>> Julian Waters has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains five additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'openjdk:master' into patch-7
>>  - Revert vm_version_linux_riscv.cpp
>>  - vm_version_linux_riscv.cpp
>>  - allocation.cpp
>>  - 8310260
>
> src/hotspot/os_cpu/linux_riscv/vm_version_linux_riscv.cpp line 77:
> 
>> 75: #define read_csr(csr)   \
>> 76: ({  \
>> 77: unsigned long __v;  \
> 
> Can this change be made separately?  I'd like to have the C++17 switch be as 
> clean as possible.

No problem!

-

PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1427669495

Re: RFR: 8314488: Compile the JDK as C++17

[Kim Barrett]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

Nearly ready.  Still need to figure out the minimum compiler versions to 
require.

src/hotspot/os_cpu/linux_riscv/vm_version_linux_riscv.cpp line 77:

> 75: #define read_csr(csr)   \
> 76: ({  \
> 77: unsigned long __v;  \

Can this change be made separately?  I'd like to have the C++17 switch be as 
clean as possible.

src/hotspot/share/memory/allocation.cpp line 114:

> 112: //
> 113: 
> 114: void* AnyObj::operator new(size_t size, Arena *arena) {

This change was recently made as part of JDK-8317132.

-

Changes requested by kbarrett (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14988#pullrequestreview-1783283088
PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1427655730
PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1427654886

Re: RFR: 8314488: Compile the JDK as C++17

[Julian Waters]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

Keeping alive

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1785145814

Re: RFR: 8314488: Compile the JDK as C++17

[Martin Doerr]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

We're not changing it for existing releases. I don't think non-LTS releases 
play a significant role regarding such compatibility. Next LTS is supposed to 
be JDK 25 (2025-09-16, https://www.java.com/releases/).

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1742099242

Re: RFR: 8314488: Compile the JDK as C++17

[Michael Felt]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

Requiring xlc17 aka openxl means any AIX rte requirements change as well. 
Binary compatibility, rte , etc.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1741985638

Re: RFR: 8314488: Compile the JDK as C++17

[Martin Doerr]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

Successfully tested on AIX with xlc 17.1.1. Works for us (SAP). Please check 
with others who might still use an older compiler (IBM).

-

Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14988#pullrequestreview-1586571119

Re: RFR: 8314488: Compile the JDK as C++17

[Andrew Haley]

On Sat, 19 Aug 2023 07:20:15 GMT, Kim Barrett  wrote:

> This change needs to be motivated. One way to do that would be to file CRs to 
> change the style guide to permit various features. Label them with `cpp17`. 
> Discuss them prior to making the language change, describing why they are 
> important. A sufficient set of such provides an argument for making the 
> language change.

Is it impractical to drop the obsolete features of C++11, working in the common 
subset of C++11 and C++17?

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1684883361

Re: RFR: 8314488: Compile the JDK as C++17

[Kim Barrett]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

This change needs to be motivated.  One way to do that would be to file CRs to
change the style guide to permit various features.  Label them with `cpp17`.
Discuss them prior to making the language change, describing why they are
important.  A sufficient set of such provides an argument for making the
language change.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1684876746

Re: RFR: 8314488: Compile the JDK as C++17

[Kim Barrett]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

Changes requested by kbarrett (Reviewer).

There may be other changes needed either in preparation or as part of making
the change to the language version.

- Dynamic allocation of over-aligned types is supported in C++17.  We might
need to update our allocation base classes (like CHeapObj<>) to account for
this as part of the update.  I'm not sure it can be deferred to a separate
followup.

- C++17 exception specifications are part of the type system.  "Valid C++14
code may fail to compile or produce different results ..."  This needs to be
looked at before changing the language selection switch.

- Dynamic exception specifications were previously deprecated, and are removed
by C++17.  It looks like some compilers are still permitting the no-throw
case, but we might need to convert to using noexcept before changing the
language selection switch.

There may be others that I've forgotten or haven't noticed.

make/autoconf/flags-cflags.m4 line 577:

> 575:   # CXXFLAGS C++ language level for all of JDK, including Hotspot.
> 576:   if test "x$TOOLCHAIN_TYPE" = xgcc || test "x$TOOLCHAIN_TYPE" = xclang 
> || test "x$TOOLCHAIN_TYPE" = xxlc; then
> 577: LANGSTD_CXXFLAGS="-std=c++17"

No, this change cannot be made yet.  As noted in the prior JEP discussion, we 
need to wait for the aix-ppc
port maintainers to upgrade the compiler they are using.  I happened to check 
last with them about their
progress.  While progress has been made, they are not yet ready to throw that 
switch.  Hoping to finish that
work sometime this fall.

-

PR Review: https://git.openjdk.org/jdk/pull/14988#pullrequestreview-1585595153
PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1684875974
PR Review Comment: https://git.openjdk.org/jdk/pull/14988#discussion_r1299140565

Re: RFR: 8314488: Compile the JDK as C++17

[David Holmes]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

Hi Julian,

Given the JEP was closed/rejected as being unnecessary it didn't really make 
sense to present this as an Implementation of the JEP. This is simply the last 
couple of fixes to make code C++17 clean and to switch to requiring C++17. The 
former seems quite reasonable to me. The latter does require further discussion 
and buy-in.

Thanks

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1683450635

Re: RFR: 8314488: Compile the JDK as C++17

[Julian Waters]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

Hi Andrew, majority of the work to remove C++17 breaking code from the JDK is 
actually already complete, this PR cleans out the last 2 places that cannot 
pass C++17 (a mismatched throw specifier and a register storage class 
qualifier), after which the JDK is fully compilable as C++17. I had taken 
Mark's rejection to mean that going to C++17 is not worth a JEP, unlike 
JEP-347. Is this still too soon?

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1683426702

Re: RFR: 8314488: Compile the JDK as C++17

[Andrew Haley]

On 8/17/23 10:27, Mario Torre wrote:

The right place for this sort of changes is a JEP


Well, maybe. It depends on how much work is involved.

But, to begin with, it'd be interesting to know if the JDK could be compiled
with a C++17 compiler. Then -- if it can't -- submit a PR to make it
C++17-clean.

--
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR: 8314488: Compile the JDK as C++17

[Mario Torre]

I agree with Thomas and Mark, I too don't see this change favourably.

This change would affect current build environments and setups,
introduce unforeseen bugs and as Thomas mentioned the backporting work
itself. Even changes due to JEP 347 did potentially make backporting
more difficult, however it standardised on a fixed and broadly
available version of gcc, and in this case C++17 would mean being
forced to update.

The right place for this sort of changes is a JEP and it needs to be
widely discussed with porters and the updates maintainers before even
going into mainline. In your case the JEP was rejected, I would accept
that because this is an invasive change that may not bring relevant
benefits, but at the very least the right thing to do would be to
write a better proposal and make a [very] compelling case in the JEP
on why we may want to update now, with the prerequisite of a
discussion on this proposal and its benefits on the main development
list to make sure many more eyes did see this.

Eventually there will be a time where we can and maybe should consider
c++17 (or later), I don't think this is now however.

Cheers,
Mario


On Thu, Aug 17, 2023 at 8:04 AM Thomas Stuefe  wrote:
>
> On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:
>
> > Implementation of [JEP draft: Compile the JDK as 
> > C++17](https://bugs.openjdk.org/browse/JDK-8310260)
>
> I disagree with this change. This should be discussed more broadly before 
> trying to get a PR in. The associated JEP has been closed by Mark. Just a "oh 
> well, then I'm doing it without JEP" is not the right way.
>
> Before agreeing to this, I would like to know what actual changes have been 
> planned, and see them weighted against the cost. The cost, as I have stated 
> before, are reviewer churn, implementation risk, and increased cost of 
> downporting patches to older JDK versions.
>
> -
>
> PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1681660826
>


--
Mario Torre
Manager, Software Engineering, Red Hat OpenJDK, Java Champion
https://keybase.io/neugens
9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898
Mastodon: https://mastodon.social/@MarioTorre

Red Hat GmbH, Registered seat: Werner von Siemens Ring 12, D-85630
Grasbrunn, Germany

Commercial register: Amtsgericht Muenchen/Munich, HRB 153243,
Managing Directors: Ryan Barnhart, Charles Cachera, Michael O'Neill, Amy Ross

Re: RFR: 8314488: Compile the JDK as C++17

[Thomas Stuefe]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

I disagree with this change. This should be discussed more broadly before 
trying to get a PR in. The associated JEP has been closed by Mark. Just a "oh 
well, then I'm doing it without JEP" is not the right way.

Before agreeing to this, I would like to know what actual changes have been 
planned, and see them weighted against the cost. The cost, as I have stated 
before, are reviewer churn, implementation risk, and increased cost of 
downporting patches to older JDK versions.

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1681660826

Re: RFR: 8314488: Compile the JDK as C++17

[Julian Waters]

On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters  wrote:

> Implementation of [JEP draft: Compile the JDK as 
> C++17](https://bugs.openjdk.org/browse/JDK-8310260)

The minimum compiler requirements have not been implemented yet, as I wanted to 
leave their discretion to any reviewers before setting a fixed value in the 
build system

-

PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1681605796