Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v7]
On Wed, 10 Apr 2024 12:15:34 GMT, Joachim Kern wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about >> ill formatted printf > > Joachim Kern has updated the pull request incrementally with one additional > commit since the last revision: > > saver solution Yes, I like it too Thanks, Thomas, for your helpful feedback! - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18536#pullrequestreview-1991857617
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 10 Apr 2024 13:46:11 GMT, Martin Doerr wrote: >> In my humble opinion the inclusion of alloca.h was slightly cleaner, but I >> guess it doesn't matter. Out of curiosity, why do you guys prefer not >> including it? > > When only looking at AIX code, I think the inclusion of alloca.h was cleaner. > Agreed. The new code makes AIX behave like other platforms and avoids the AIX > specific part in shared code. > I could live with either version. I can live with either, too. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559536724
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v7]
On Wed, 10 Apr 2024 12:15:34 GMT, Joachim Kern wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about >> ill formatted printf > > Joachim Kern has updated the pull request incrementally with one additional > commit since the last revision: > > saver solution This looks good to me now, provided Martin likes it too. Thanks for incorporating my suggestions. - Marked as reviewed by stuefe (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18536#pullrequestreview-1991847641
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 10 Apr 2024 13:35:39 GMT, Julian Waters wrote: >> Yes I believe. I will remove the `#pragma alloca` everywhere, I will remove >> the `#include ` everywhere and I will add >> `-Dalloca=__builtin_alloca` to the compile commands. If it works I will >> update the PR. > > In my humble opinion the inclusion of alloca.h was slightly cleaner, but I > guess it doesn't matter. Out of curiosity, why do you guys prefer not > including it? When only looking at AIX code, I think the inclusion of alloca.h was cleaner. Agreed. The new code makes AIX behave like other platforms and avoids the AIX specific part in shared code. I could live with either version. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559470659
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 10 Apr 2024 10:13:37 GMT, Joachim Kern wrote: >> Can `-Dalloca=__builtin_alloca` replace `#include `? > > Yes I believe. I will remove the `#pragma alloca` everywhere, I will remove > the `#include ` everywhere and I will add > `-Dalloca=__builtin_alloca` to the compile commands. If it works I will > update the PR. In my humble opinion the inclusion of alloca.h was slightly cleaner, but I guess it doesn't matter. Out of curiosity, why do you guys prefer not including it? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559450230
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v7]
On Wed, 10 Apr 2024 13:19:50 GMT, Martin Doerr wrote: >> Currently XLC16 but looking to upgrade to XLC17 on the minimum supported >> level for it (So it wouldn't be SP7 at present). Thanks for the ping - we >> have no current plans to increase to SP7. > > Seems like we need to keep it. This is unfortunate. I wouldn't risk mixing > malloc and vec_malloc. Who knows what kind of problems this could cause? > What happens if we try to build this code on AIX 7.2 TL5 SP7? Will the > compiler complain because `malloc` is no longer defined? Should we check > `defined(malloc)` in addition? We already built this code since months on AIX 7.2 TL5 SP7, because we raised the OS. This code is needed on SP5 and does not hurt SP7. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559441769
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v7]
On Tue, 9 Apr 2024 17:25:04 GMT, Stewart X Addison wrote: >> Pinging @sxa - what build environment does temurin use for AIX? > > Currently XLC16 but looking to upgrade to XLC17 on the minimum supported > level for it (So it wouldn't be SP7 at present). Thanks for the ping - we > have no current plans to increase to SP7. Seems like we need to keep it. This is unfortunate. I wouldn't risk mixing malloc and vec_malloc. Who knows what kind of problems this could cause? What happens if we try to build this code on AIX 7.2 TL5 SP7? Will the compiler complain because `malloc` is no longer defined? Should we check `defined(malloc)` in addition? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559425371
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v7]
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about > ill formatted printf Joachim Kern has updated the pull request incrementally with one additional commit since the last revision: saver solution - Changes: - all: https://git.openjdk.org/jdk/pull/18536/files - new: https://git.openjdk.org/jdk/pull/18536/files/801cfb54..a8d85924 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18536=06 - incr: https://webrevs.openjdk.org/?repo=jdk=18536=05-06 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18536.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18536/head:pull/18536 PR: https://git.openjdk.org/jdk/pull/18536
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v6]
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about > ill formatted printf Joachim Kern has updated the pull request incrementally with one additional commit since the last revision: replaced pragma alloca and include alloca by compiler define - Changes: - all: https://git.openjdk.org/jdk/pull/18536/files - new: https://git.openjdk.org/jdk/pull/18536/files/302ea6a7..801cfb54 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18536=05 - incr: https://webrevs.openjdk.org/?repo=jdk=18536=04-05 Stats: 8 lines in 3 files changed: 0 ins; 7 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18536.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18536/head:pull/18536 PR: https://git.openjdk.org/jdk/pull/18536
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v5]
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about > ill formatted printf Joachim Kern has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits: - Merge master - cosmetic changes - version check not needed anymore - Followed the proposals - JDK-8329257 - Changes: https://git.openjdk.org/jdk/pull/18536/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18536=04 Stats: 257 lines in 14 files changed: 11 ins; 208 del; 38 mod Patch: https://git.openjdk.org/jdk/pull/18536.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18536/head:pull/18536 PR: https://git.openjdk.org/jdk/pull/18536
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 10 Apr 2024 10:07:02 GMT, Martin Doerr wrote: >> Is the comment in front of >> https://github.com/openjdk/jdk/blob/51ed69a586105b707ae616f9eba898449bf9fba7/src/hotspot/os/aix/os_aix.cpp#L28 >> still correct? Seems like it should get replaced. See >> https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.1?topic=pragmas-pragma-alloca-c-only > > Can `-Dalloca=__builtin_alloca` replace `#include `? Yes I believe. I will remove the `#pragma alloca` everywhere, I will remove the `#include ` everywhere and I will add `-Dalloca=__builtin_alloca` to the compile commands. If it works I will update the PR. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559191851
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 10 Apr 2024 10:00:02 GMT, Martin Doerr wrote: >> If I omit this #include >> I get compiler errors of the following kind >> >> .../src/hotspot/share/runtime/javaThread.cpp::24: error: use of >> undeclared identifier 'alloca' >> char* p1 = (char*) alloca(1); >>^ >> >> >> Of course I can do this include in every nagging file, but I thought it is >> simpler to keep it in the central header. > > Is the comment in front of > https://github.com/openjdk/jdk/blob/51ed69a586105b707ae616f9eba898449bf9fba7/src/hotspot/os/aix/os_aix.cpp#L28 > still correct? Seems like it should get replaced. See > https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.1?topic=pragmas-pragma-alloca-c-only Can `-Dalloca=__builtin_alloca` replace `#include `? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559183757
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 10 Apr 2024 09:40:16 GMT, Joachim Kern wrote: >> Do we even need to include ? >> >> From the Linux man page for alloca: >> >> By necessity, alloca() is a compiler built-in, also known as >> __builtin_alloca(). By default, modern compilers automatically >> translate all uses of alloca() into the built-in, but this is >> forbidden if standards conformance is requested (-ansi, -std=c*), >> in which case is required, lest a symbol dependency be >> emitted. >> >> There are uses of it in shared code where there isn't an applicable include, >> other than from globalDefinitions_xlc.hpp. So it appears all other supported >> compilers do treat it as a built-in with the options we are providing, and >> don't need the include. Maybe that's true for the new xlc compiler too? > > If I omit this #include > I get compiler errors of the following kind > > .../src/hotspot/share/runtime/javaThread.cpp::24: error: use of > undeclared identifier 'alloca' > char* p1 = (char*) alloca(1); >^ > > > Of course I can do this include in every nagging file, but I thought it is > simpler to keep it in the central header. Is the comment in front of https://github.com/openjdk/jdk/blob/51ed69a586105b707ae616f9eba898449bf9fba7/src/hotspot/os/aix/os_aix.cpp#L28 still correct? Seems like it isn't followed everywhere. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559175426
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v4]
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about > ill formatted printf Joachim Kern has updated the pull request incrementally with one additional commit since the last revision: cosmetic changes - Changes: - all: https://git.openjdk.org/jdk/pull/18536/files - new: https://git.openjdk.org/jdk/pull/18536/files/ac1335e5..815974f5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18536=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18536=02-03 Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/18536.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18536/head:pull/18536 PR: https://git.openjdk.org/jdk/pull/18536
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 10 Apr 2024 00:51:22 GMT, Kim Barrett wrote: >> src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 36: >> >>> 34: #if defined(_AIX) >>> 35: #include >>> 36: #endif >> >> I would much rather see this include added in the few places it was actually >> needed, rather than being >> added here. > > Do we even need to include ? > > From the Linux man page for alloca: > > By necessity, alloca() is a compiler built-in, also known as > __builtin_alloca(). By default, modern compilers automatically > translate all uses of alloca() into the built-in, but this is > forbidden if standards conformance is requested (-ansi, -std=c*), > in which case is required, lest a symbol dependency be > emitted. > > There are uses of it in shared code where there isn't an applicable include, > other than from globalDefinitions_xlc.hpp. So it appears all other supported > compilers do treat it as a built-in with the options we are providing, and > don't need the include. Maybe that's true for the new xlc compiler too? If I omit this #include I get compiler errors of the following kind .../src/hotspot/share/runtime/javaThread.cpp::24: error: use of undeclared identifier 'alloca' char* p1 = (char*) alloca(1); ^ Of course I can do this include in every nagging file, but I thought it is simpler to keep it in the central header. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559150964
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 9 Apr 2024 18:32:04 GMT, Kim Barrett wrote: >> Joachim Kern has updated the pull request incrementally with one additional >> commit since the last revision: >> >> version check not needed anymore > > src/hotspot/share/utilities/byteswap.hpp line 2: > >> 1: /* >> 2: * Copyright (c) 2023, Google and/or its affiliates. All rights reserved. > > Don't drop the creation year. Done - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559142128
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 9 Apr 2024 17:00:56 GMT, Thomas Stuefe wrote:
>> Joachim Kern has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> version check not needed anymore
>
> src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp line 440:
>
>> 438: st->print("pc =" INTPTR_FORMAT " ", (unsigned
>> long)uc->uc_mcontext.jmp_context.iar);
>> 439: st->print("lr =" INTPTR_FORMAT " ", (unsigned
>> long)uc->uc_mcontext.jmp_context.lr);
>> 440: st->print("ctr=" INTPTR_FORMAT " ", (unsigned
>> long)uc->uc_mcontext.jmp_context.ctr);
>
> p2i
I had tried this, but got following error:
.../src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp:438:40: error: no matching
function for call to 'p2i'
st->print("pc =" INTPTR_FORMAT " ", p2i(uc->uc_mcontext.jmp_context.iar));
^~~
.../src/hotspot/share/utilities/globalDefinitions.hpp:179:17: note: candidate
function not viable: no known conversion from 'const unsigned long long' to
'const volatile void *' for 1st argument; take the address of the argument with
&
inline intptr_t p2i(const volatile void* p) {
^
.../src/hotspot/share/oops/oopsHierarchy.hpp:169:17: note: candidate function
not viable: no known conversion from 'const unsigned long long' to 'narrowOop'
for 1st argument
inline intptr_t p2i(narrowOop o) {
^
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559128609
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 9 Apr 2024 16:59:39 GMT, Thomas Stuefe wrote: >> Hi Thomas, `maxDisclaimSize` is of type `unsigned int`; therefore I get the >> following warning: >> >> os/aix/os_aix.cpp:314:42: error: format specifies type 'unsigned long' but >> the argument has type 'unsigned int' [-Werror,-Wformat] >> RANGEFMTARGS(p, maxDisclaimSize), >> ^~~ >> >> Should I keep the casts, or change the type of `maxDisclaimSize, >> numFullDisclaimsNeeded, lastDisclaimSize` to `const unsigned long`? > > I would change them to size_t. Thanks for doing this. Done - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559121239
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 9 Apr 2024 19:20:22 GMT, Kim Barrett wrote: >> Joachim Kern has updated the pull request incrementally with one additional >> commit since the last revision: >> >> version check not needed anymore > > src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 36: > >> 34: #if defined(_AIX) >> 35: #include >> 36: #endif > > I would much rather see this include added in the few places it was actually > needed, rather than being > added here. Do we even need to include ? >From the Linux man page for alloca: By necessity, alloca() is a compiler built-in, also known as __builtin_alloca(). By default, modern compilers automatically translate all uses of alloca() into the built-in, but this is forbidden if standards conformance is requested (-ansi, -std=c*), in which case is required, lest a symbol dependency be emitted. There are uses of it in shared code where there isn't an applicable include, other than from globalDefinitions_xlc.hpp. So it appears all other supported compilers do treat it as a built-in with the options we are providing, and don't need the include. Maybe that's true for the new xlc compiler too? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558565268
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 16:14:12 GMT, Joachim Kern wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about >> ill formatted printf > > Joachim Kern has updated the pull request incrementally with one additional > commit since the last revision: > > version check not needed anymore Changes requested by kbarrett (Reviewer). src/hotspot/share/utilities/byteswap.hpp line 2: > 1: /* > 2: * Copyright (c) 2023, Google and/or its affiliates. All rights reserved. Don't drop the creation year. src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 36: > 34: #if defined(_AIX) > 35: #include > 36: #endif I would much rather see this include added in the few places it was actually needed, rather than being added here. - PR Review: https://git.openjdk.org/jdk/pull/18536#pullrequestreview-1989864573 PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558124034 PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558172309
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 9 Apr 2024 17:01:59 GMT, Thomas Stuefe wrote: >> @suchismith1993 >> Hi Suchi, can you please tell me when you will raise your build environment >> from AIX 7.2 TL5 SP5 to SP7? >> I' am asking you, because I want to get rid of this nasty workaround. > > Pinging @sxa - what build environment does temurin use for AIX? Currently XLC16 but looking to upgrade to XLC17 on the minimum supported level for it (So it wouldn't be SP7 at present). Thanks for the ping - we have no current plans to increase to SP7. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558053537
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 09:19:16 GMT, Joachim Kern wrote: >> Hi Thomas, >> I would like to get totally rid of this, because as I mentioned IBM already >> modified the `stdlib.h` header not using `#define malloc vec_malloc` any >> more (and all the other vec_... defines). We have to ask the adoptium >> colleagues at IBM if they already have raised their build environment by the >> 2 SP levels needed. >> In principle we had to do the same workaround for `calloc, free,...` too, >> but they didn't show up as errors in the logging files. >> These lines where never meant to stay for long. Just to be able to compile >> until IBM fixes the issue, which is done now. > > @suchismith1993 > Hi Suchi, can you please tell me when you will raise your build environment > from AIX 7.2 TL5 SP5 to SP7? > I' am asking you, because I want to get rid of this nasty workaround. Pinging @sxa - what build environment does temurin use for AIX? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558020493
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 16:14:12 GMT, Joachim Kern wrote:
>> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building
>> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect
>> clang by another name, and it uses the clang toolchain in the JDK build.
>> Thus the old xlc toolchain was removed by
>> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701).
>> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the
>> last xlc rudiment.
>> This means merging the AIX specific content of
>> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp
>> into the corresponding gcc files on the on side and removing the
>> defined(TARGET_COMPILER_xlc) blocks in the code, because the
>> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX
>> compiler.
>> The rest of the changes are needed because of using
>> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about
>> ill formatted printf
>
> Joachim Kern has updated the pull request incrementally with one additional
> commit since the last revision:
>
> version check not needed anymore
src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp line 440:
> 438: st->print("pc =" INTPTR_FORMAT " ", (unsigned
> long)uc->uc_mcontext.jmp_context.iar);
> 439: st->print("lr =" INTPTR_FORMAT " ", (unsigned
> long)uc->uc_mcontext.jmp_context.lr);
> 440: st->print("ctr=" INTPTR_FORMAT " ", (unsigned
> long)uc->uc_mcontext.jmp_context.ctr);
p2i
src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp line 443:
> 441: st->cr();
> 442: for (int i = 0; i < 32; i++) {
> 443: st->print("r%-2d=" INTPTR_FORMAT " ", i, (unsigned
> long)uc->uc_mcontext.jmp_context.gpr[i]);
p2i
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558017408
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558017827
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 10:26:42 GMT, Joachim Kern wrote:
>> src/hotspot/os/aix/os_aix.cpp line 314:
>>
>>> 312: ErrnoPreserver ep;
>>> 313: log_trace(os, map)("disclaim failed: " RANGEFMT " errno=(%s)",
>>> 314: RANGEFMTARGS(p, (long)maxDisclaimSize),
>>
>> Wait, why are these casts needed? maxDisclaimSize is size_t, RANGEFMT uses
>> SIZE_FORMAT. That should work without cast.
>
> Hi Thomas, `maxDisclaimSize` is of type `unsigned int`; therefore I get the
> following warning:
>
> os/aix/os_aix.cpp:314:42: error: format specifies type 'unsigned long' but
> the argument has type 'unsigned int' [-Werror,-Wformat]
> RANGEFMTARGS(p, maxDisclaimSize),
> ^~~
>
> Should I keep the casts, or change the type of `maxDisclaimSize,
> numFullDisclaimsNeeded, lastDisclaimSize` to `const unsigned long`?
I would change them to size_t. Thanks for doing this.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1558012122
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 16:14:12 GMT, Joachim Kern wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about >> ill formatted printf > > Joachim Kern has updated the pull request incrementally with one additional > commit since the last revision: > > version check not needed anymore The build change look trivially fine. And allow me to show my appreciation for the hotspot code cleanup! (But note that this is not a review of that part). - PR Review: https://git.openjdk.org/jdk/pull/18536#pullrequestreview-1976222691
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 3 Apr 2024 02:28:08 GMT, Julian Waters wrote: >> https://github.com/openjdk/jdk/pull/18586 > > @kimbarrett I've been doing things to permit gcc/Windows, not clang. clang > has too many different distributions on Windows for me to settle on one, and > generalising all of them to be able to compile with any of the Windows clang > distributions seamlessly and without issues sounds like a nightmare :P gcc on > the other hand has just 2: MSYS2 MINGW64 with ucrt (Which is the one I'm > working on) and standalone gcc Windows builds that link to ucrt > > I haven't sent a cleanup in this area because I thought my changes were too > specific to gcc/Windows, and wouldn't help much with HotSpot in general. I've > learnt from my mistakes in the past where I caused reviewers pain in > reviewing my admittedly selfish changes to HotSpot :( > That said, if it is requested of me, I can commit some cleanups to this file. > What do you think? @TheShermanTanker It depends on the details, of course. I think there are lots of possible cleanups in this vicinity that have little or nothing to do with gcc/Windows specifically, though might help there too. And yeah, I misremembered that it was not clang/Windows but rather gcc/Windows you were working on. But the same points largely apply here. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548868243
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 20:10:12 GMT, Kim Barrett wrote: >> I'm waiting for a bunch of tests to complete, so decided to just take that >> issue. > > https://github.com/openjdk/jdk/pull/18586 @kimbarrett I've been doing things to permit gcc/Windows, not clang. clang has too many different distributions on Windows for me to settle on one, and generalising all of them to be able to compile with any of the Windows clang distributions seamlessly and without issues sounds like a nightmare :P gcc on the other hand has just 2: MSYS2 MINGW64 with ucrt (Which is the one I'm working on) and standalone gcc Windows builds that link to ucrt I haven't sent a cleanup in this area because I thought my changes were too specific to gcc/Windows, and wouldn't help much with HotSpot in general. I've learnt from my mistakes in the past where I caused reviewers pain in reviewing my admittedly selfish changes to HotSpot :( That said, if it is requested of me, I can commit some cleanups to this file. What do you think? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548833671
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 11:35:44 GMT, Joachim Kern wrote: >> linux macos and now Aix use this file. > > Who is able to explain if > `#if defined(LINUX) || defined(_ALLBSD_SOURCE) || defined(_AIX)` > in this file is equivalent to > `#if 1` See my other comments and https://bugs.openjdk.org/browse/JDK-8329546 - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548550923
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 17:01:07 GMT, Kim Barrett wrote: >> https://bugs.openjdk.org/browse/JDK-8329546 - I can take this if nobody else >> grabs it soon. > > I'm waiting for a bunch of tests to complete, so decided to just take that > issue. https://github.com/openjdk/jdk/pull/18586 - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548549705
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 16:52:04 GMT, Kim Barrett wrote: >> There was at one time an attempt at a gcc/Solaris port, but I think it was >> never completed, and most vestiges removed. More recently, @TheShermanTanker >> has been doing stuff to permit clang/Windows, and clang-based builds use >> this file. >> I'm kind of surprised he hasn't encountered problems and done some cleanup >> here. >> >> (and ) and 64bit integer types are standard C++ now, >> so no longer need all this conditionalization. I suggest cleaning that up as >> a >> separate precursor. That would eliminate the two !defined blocks entirely. I >> wish the other conditional includes in this block were "where needed" rather >> than in globalDefinitions_gcc.hpp, but that's a different mess. > > https://bugs.openjdk.org/browse/JDK-8329546 - I can take this if nobody else > grabs it soon. I'm waiting for a bunch of tests to complete, so decided to just take that issue. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548252193
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 16:41:40 GMT, Kim Barrett wrote: >> I cannot answer this question. >> If this line is now obsolete it was also obsolete before including AIX, >> because AIX didn't use this file beforehand. > > There was at one time an attempt at a gcc/Solaris port, but I think it was > never completed, and most vestiges removed. More recently, @TheShermanTanker > has been doing stuff to permit clang/Windows, and clang-based builds use this > file. > I'm kind of surprised he hasn't encountered problems and done some cleanup > here. > > (and ) and 64bit integer types are standard C++ now, > so no longer need all this conditionalization. I suggest cleaning that up as a > separate precursor. That would eliminate the two !defined blocks entirely. I > wish the other conditional includes in this block were "where needed" rather > than in globalDefinitions_gcc.hpp, but that's a different mess. https://bugs.openjdk.org/browse/JDK-8329546 - I can take this if nobody else grabs it soon. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548239737
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 11:20:49 GMT, Joachim Kern wrote: >> src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 62: >> >>> 60: #include >>> 61: >>> 62: #if defined(LINUX) || defined(_ALLBSD_SOURCE) || defined(_AIX) >> >> What else is left? Could we just remove this line altogether now? > > I cannot answer this question. > If this line is now obsolete it was also obsolete before including AIX, > because AIX didn't use this file beforehand. There was at one time an attempt at a gcc/Solaris port, but I think it was never completed, and most vestiges removed. More recently, @TheShermanTanker has been doing stuff to permit clang/Windows, and clang-based builds use this file. I'm kind of surprised he hasn't encountered problems and done some cleanup here. (and ) and 64bit integer types are standard C++ now, so no longer need all this conditionalization. I suggest cleaning that up as a separate precursor. That would eliminate the two !defined blocks entirely. I wish the other conditional includes in this block were "where needed" rather than in globalDefinitions_gcc.hpp, but that's a different mess. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548225495
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about > ill formatted printf Joachim Kern has updated the pull request incrementally with one additional commit since the last revision: version check not needed anymore - Changes: - all: https://git.openjdk.org/jdk/pull/18536/files - new: https://git.openjdk.org/jdk/pull/18536/files/689b353d..ac1335e5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18536=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18536=01-02 Stats: 6 lines in 1 file changed: 0 ins; 6 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18536.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18536/head:pull/18536 PR: https://git.openjdk.org/jdk/pull/18536
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v2]
On Tue, 2 Apr 2024 14:48:49 GMT, Martin Doerr wrote: >> My question is, do we need this block, because now already configure warns >> about an outdated compiler, or is a warning to weak and we want to force >> this error here? > > I think that building with xlc 16 is no longer possible because the old build > pipeline is no longer supported and that is already caught by configure. So, > can we even reach here with older xlc compilers? > If not, this code can get removed. Yes, of course you are right. All the compile statements will fail with xlc 16 or older. I will remove it. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548134431
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v2]
On Tue, 2 Apr 2024 11:22:54 GMT, Joachim Kern wrote: >> I'd prefer having less AIX specific parts in this file. Can this be moved >> somewhere else? Or maybe combine it with the AIX code above? > > My question is, do we need this block, because now already configure warns > about an outdated compiler, or is a warning to weak and we want to force this > error here? I think that building with xlc 16 is no longer possible because the old build pipeline is no longer supported and that is already caught by configure. So, can we even reach here with older xlc compilers? If not, this code can get removed. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548043503
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v2]
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about > ill formatted printf Joachim Kern has updated the pull request incrementally with one additional commit since the last revision: Followed the proposals - Changes: - all: https://git.openjdk.org/jdk/pull/18536/files - new: https://git.openjdk.org/jdk/pull/18536/files/61fd0ff2..689b353d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18536=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18536=00-01 Stats: 35 lines in 9 files changed: 0 ins; 4 del; 31 mod Patch: https://git.openjdk.org/jdk/pull/18536.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18536/head:pull/18536 PR: https://git.openjdk.org/jdk/pull/18536
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Tue, 2 Apr 2024 11:28:30 GMT, Joachim Kern wrote: >> src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 103: >> >>> 101: #endif >>> 102: >>> 103: #if !defined(LINUX) && !defined(_ALLBSD_SOURCE) && !defined(_AIX) >> >> I believe this whole section can be removed now. >> >> At least I have no idea who this is for. What gcc versions does OpenJDK >> still support, then, beside these platforms. Also, any gcc platform not on >> linux or bsd would have hit the #error below at line 132. > > linux macos and now Aix use this file. Who is able to explain if `#if defined(LINUX) || defined(_ALLBSD_SOURCE) || defined(_AIX)` in this file is equivalent to `#if 1` - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547692144
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 08:06:01 GMT, Thomas Stuefe wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about >> ill formatted printf > > src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 83: > >> 81: #error "xlc version not supported, macro __open_xl_version__ not found" >> 82: #endif >> 83: #endif // AIX > > Can probably be shortened like this: > > Suggestion: > > #ifdef _AIX > #if !defined(__open_xl_version__) || (__open_xl_version__ < 17) > #error "this xlc version is not supported" > #endif > #endif // AIX followed your proposal. > src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 103: > >> 101: #endif >> 102: >> 103: #if !defined(LINUX) && !defined(_ALLBSD_SOURCE) && !defined(_AIX) > > I believe this whole section can be removed now. > > At least I have no idea who this is for. What gcc versions does OpenJDK still > support, then, beside these platforms. Also, any gcc platform not on linux or > bsd would have hit the #error below at line 132. linux macos and now Aix use this file. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547677545 PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547681162
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Thu, 28 Mar 2024 17:33:29 GMT, Martin Doerr wrote: >> src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 83: >> >>> 81: #error "xlc version not supported, macro __open_xl_version__ not >>> found" >>> 82: #endif >>> 83: #endif // AIX >> >> This `#ifdef _AIX` might be obsolete, because configure will throw a warning >> if the compiler has a lower version, but it's only a warning. > > I'd prefer having less AIX specific parts in this file. Can this be moved > somewhere else? Or maybe combine it with the AIX code above? My question is, do we need this block, because now already configure warns about an outdated compiler, or is a warning to weak and we want to force this error here? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547672502
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:39:06 GMT, Thomas Stuefe wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about >> ill formatted printf > > src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 62: > >> 60: #include >> 61: >> 62: #if defined(LINUX) || defined(_ALLBSD_SOURCE) || defined(_AIX) > > What else is left? Could we just remove this line altogether now? I cannot answer this question. If this line is now obsolete it was also obsolete before including AIX, because AIX didn't use this file beforehand. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547667349
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:25:30 GMT, Thomas Stuefe wrote:
>> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building
>> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect
>> clang by another name, and it uses the clang toolchain in the JDK build.
>> Thus the old xlc toolchain was removed by
>> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701).
>> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the
>> last xlc rudiment.
>> This means merging the AIX specific content of
>> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp
>> into the corresponding gcc files on the on side and removing the
>> defined(TARGET_COMPILER_xlc) blocks in the code, because the
>> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX
>> compiler.
>> The rest of the changes are needed because of using
>> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about
>> ill formatted printf
>
> src/hotspot/os/aix/os_aix.cpp line 1212:
>
>> 1210: st->print_cr("physical free : " SIZE_FORMAT, (unsigned
>> long)mi.real_free);
>> 1211: st->print_cr("swap total : " SIZE_FORMAT, (unsigned
>> long)mi.pgsp_total);
>> 1212: st->print_cr("swap free : " SIZE_FORMAT, (unsigned
>> long)mi.pgsp_free);
>
> A better way to do this would be to change AIX::meminfo to use size_t. We
> should have done this when introducing that API.
Done. modified `os::Aix::meminfo_t` to use `size_t` instead of `long long`
> src/hotspot/os/aix/os_aix.cpp line 1399:
>
>> 1397: os->print("[" PTR_FORMAT " - " PTR_FORMAT "] (" UINTX_FORMAT
>> 1398: " bytes, %ld %s pages), %s",
>> 1399: (uintptr_t)addr, (uintptr_t)addr + size - 1, size, size /
>> pagesize, describe_pagesize(pagesize),
>
> p2i
Done.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547603744
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547606275
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:19:33 GMT, Thomas Stuefe wrote:
>> src/hotspot/os/aix/loadlib_aix.cpp line 120:
>>
>>> 118: (lm->is_in_vm ? '*' : ' '),
>>> 119: (uintptr_t)lm->text, (uintptr_t)lm->text + lm->text_len,
>>> 120: (uintptr_t)lm->data, (uintptr_t)lm->data + lm->data_len,
>>
>> Please don't cast, use `p2i()`.
>
> Check copyrights in this file and all others. Adapt SAP and Oracle copyrights.
Done + will adopt copyrights
>> src/hotspot/os/aix/os_aix.cpp line 651:
>>
>>> 649: lt.print("Thread is alive (tid: " UINTX_FORMAT ", kernel thread
>>> id: " UINTX_FORMAT
>>> 650: ", stack [" PTR_FORMAT " - " PTR_FORMAT " (" SIZE_FORMAT
>>> "k using %luk pages)).",
>>> 651: os::current_thread_id(), (uintx) kernel_thread_id,
>>> (uintptr_t)low_address, (uintptr_t)high_address,
>>
>> Use p2i, not cast
>
> Here, and in other places too where you cast a pointer to fit into PTR_FORMAT
> or INTPTR_FORMAT
Done.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547607793
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547606610
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:21:43 GMT, Thomas Stuefe wrote:
>> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building
>> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect
>> clang by another name, and it uses the clang toolchain in the JDK build.
>> Thus the old xlc toolchain was removed by
>> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701).
>> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the
>> last xlc rudiment.
>> This means merging the AIX specific content of
>> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp
>> into the corresponding gcc files on the on side and removing the
>> defined(TARGET_COMPILER_xlc) blocks in the code, because the
>> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX
>> compiler.
>> The rest of the changes are needed because of using
>> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about
>> ill formatted printf
>
> src/hotspot/os/aix/os_aix.cpp line 314:
>
>> 312: ErrnoPreserver ep;
>> 313: log_trace(os, map)("disclaim failed: " RANGEFMT " errno=(%s)",
>> 314: RANGEFMTARGS(p, (long)maxDisclaimSize),
>
> Wait, why are these casts needed? maxDisclaimSize is size_t, RANGEFMT uses
> SIZE_FORMAT. That should work without cast.
Hi Thomas, `maxDisclaimSize` is of type `unsigned int`; therefore I get the
following warning:
os/aix/os_aix.cpp:314:42: error: format specifies type 'unsigned long' but the
argument has type 'unsigned int' [-Werror,-Wformat]
RANGEFMTARGS(p, maxDisclaimSize),
^~~
Should I keep the casts, or change the type of `maxDisclaimSize,
numFullDisclaimsNeeded, lastDisclaimSize` to `const unsigned long`?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547578012
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Tue, 2 Apr 2024 09:14:10 GMT, Joachim Kern wrote: >> Other than that, and kind of depending on your answer: How important is it >> that we catch every use of the original malloc? Can be safely mix the >> original malloc with vec_malloc if logging is not involved? >> >> I am asking, because from that it depends whether this hunk needs to appear >> right behind `#include ` or whether we can move it into the middle >> of the file together with the other AIX stuff. >> >> Because, if we move it into the middle of the file, we may miss any uses of >> malloc that may happen in system headers (would be unusual for that to >> happen but with IBM one never knows). > > Hi Thomas, > I would like to get totally rid of this, because as I mentioned IBM already > modified the `stdlib.h` header not using `#define malloc vec_malloc` any more > (and all the other vec_... defines). We have to ask the adoptium colleagues > at IBM if they already have raised their build environment by the 2 SP levels > needed. > In principle we had to do the same workaround for `calloc, free,...` too, but > they didn't show up as errors in the logging files. > These lines where never meant to stay for long. Just to be able to compile > until IBM fixes the issue, which is done now. @suchismith1993 Hi Suchi, can you please tell me when you will raise your build environment from AIX 7.2 TL5 SP5 to SP7? I' am asking you, because I want to get rid of this nasty workaround. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547473723
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:59:05 GMT, Thomas Stuefe wrote: >> While looking at this, I noticed that my question in >> https://github.com/openjdk/jdk/pull/14146#discussion_r1207078176 and >> followups had never been answered. Do you know the answers now? >> >> Quoting myself: >> >>> So, we do this only for malloc? Not for calloc, posix_memalign, realloc >>> etc? What about free? >>> Removing that define and hard-coding it here assumes ... pointers it >>> returns work with the unchanged free() and realloc() the system provides, >>> and will always do so. >>> I am basically worried that undefining malloc, even if it seems harmless >>> now, exposes us to difficult-to-investigate problems down the road, since >>> it depends on how the libc devs will reform those macros in the future. > > Other than that, and kind of depending on your answer: How important is it > that we catch every use of the original malloc? Can be safely mix the > original malloc with vec_malloc if logging is not involved? > > I am asking, because from that it depends whether this hunk needs to appear > right behind `#include ` or whether we can move it into the middle > of the file together with the other AIX stuff. > > Because, if we move it into the middle of the file, we may miss any uses of > malloc that may happen in system headers (would be unusual for that to happen > but with IBM one never knows). Hi Thomas, I would like to get totally rid of this, because as I mentioned IBM already modified the `stdlib.h` header not using `#define malloc vec_malloc` any more (and all the other vec_... defines). We have to ask the adoptium colleagues at IBM if they already have raised their build environment by the 2 SP levels needed. In principle we had to do the same workaround for `calloc, free,...` too, but they didn't show up as errors in the logging files. These lines where never meant to stay for long. Just to be able to compile until IBM fixes the issue, which is done now. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547465986
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 05:23:57 GMT, Julian Waters wrote: > > The rest of the changes are needed because of using > > utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about > > ill formatted printf > > Did you mean compilerWarnings_gcc.hpp? Yes, you're right. I fixed it. - PR Comment: https://git.openjdk.org/jdk/pull/18536#issuecomment-2031447588
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:56:10 GMT, Thomas Stuefe wrote:
>> src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 50:
>>
>>> 48: #undef malloc
>>> 49: extern void *malloc(size_t) asm("vec_malloc");
>>> 50: #endif
>>
>> This `#if` is not needed if we are building on AIX 7.2 TL5 SP7 or higher.
>> This is the case in our build environment, but I think IBM is still building
>> with SP5, which would run into build errors if I remove this `#if` now.
>
> While looking at this, I noticed that my question in
> https://github.com/openjdk/jdk/pull/14146#discussion_r1207078176 and
> followups had never been answered. Do you know the answers now?
>
> Quoting myself:
>
>> So, we do this only for malloc? Not for calloc, posix_memalign, realloc etc?
>> What about free?
>> Removing that define and hard-coding it here assumes ... pointers it returns
>> work with the unchanged free() and realloc() the system provides, and will
>> always do so.
>> I am basically worried that undefining malloc, even if it seems harmless
>> now, exposes us to difficult-to-investigate problems down the road, since it
>> depends on how the libc devs will reform those macros in the future.
Other than that, and kind of depending on your answer: How important is it that
we catch every use of the original malloc? Can be safely mix the original
malloc with vec_malloc if logging is not involved?
I am asking, because from that it depends whether this hunk needs to appear
right behind `#include ` or whether we can move it into the middle of
the file together with the other AIX stuff.
Because, if we move it into the middle of the file, we may miss any uses of
malloc that may happen in system headers (would be unusual for that to happen
but with IBM one never knows).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1544201412
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Thu, 28 Mar 2024 16:57:00 GMT, Joachim Kern wrote:
>> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building
>> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect
>> clang by another name, and it uses the clang toolchain in the JDK build.
>> Thus the old xlc toolchain was removed by
>> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701).
>> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the
>> last xlc rudiment.
>> This means merging the AIX specific content of
>> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp
>> into the corresponding gcc files on the on side and removing the
>> defined(TARGET_COMPILER_xlc) blocks in the code, because the
>> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX
>> compiler.
>> The rest of the changes are needed because of using
>> utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about
>> ill formatted printf
>
> src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 50:
>
>> 48: #undef malloc
>> 49: extern void *malloc(size_t) asm("vec_malloc");
>> 50: #endif
>
> This `#if` is not needed if we are building on AIX 7.2 TL5 SP7 or higher.
> This is the case in our build environment, but I think IBM is still building
> with SP5, which would run into build errors if I remove this `#if` now.
While looking at this, I noticed that my question in
https://github.com/openjdk/jdk/pull/14146#discussion_r1207078176 and followups
had never been answered. Do you know the answers now?
Quoting myself:
> So, we do this only for malloc? Not for calloc, posix_memalign, realloc etc?
> What about free?
> Removing that define and hard-coding it here assumes ... pointers it returns
> work with the unchanged free() and realloc() the system provides, and will
> always do so.
> I am basically worried that undefining malloc, even if it seems harmless now,
> exposes us to difficult-to-investigate problems down the road, since it
> depends on how the libc devs will reform those macros in the future.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1544199335
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:18:47 GMT, Thomas Stuefe wrote:
>> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building
>> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect
>> clang by another name, and it uses the clang toolchain in the JDK build.
>> Thus the old xlc toolchain was removed by
>> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701).
>> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the
>> last xlc rudiment.
>> This means merging the AIX specific content of
>> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp
>> into the corresponding gcc files on the on side and removing the
>> defined(TARGET_COMPILER_xlc) blocks in the code, because the
>> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX
>> compiler.
>> The rest of the changes are needed because of using
>> utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about
>> ill formatted printf
>
> src/hotspot/os/aix/loadlib_aix.cpp line 120:
>
>> 118: (lm->is_in_vm ? '*' : ' '),
>> 119: (uintptr_t)lm->text, (uintptr_t)lm->text + lm->text_len,
>> 120: (uintptr_t)lm->data, (uintptr_t)lm->data + lm->data_len,
>
> Please don't cast, use `p2i()`.
Check copyrights in this file and all others. Adapt SAP and Oracle copyrights.
> src/hotspot/os/aix/os_aix.cpp line 651:
>
>> 649: lt.print("Thread is alive (tid: " UINTX_FORMAT ", kernel thread id:
>> " UINTX_FORMAT
>> 650: ", stack [" PTR_FORMAT " - " PTR_FORMAT " (" SIZE_FORMAT
>> "k using %luk pages)).",
>> 651: os::current_thread_id(), (uintx) kernel_thread_id,
>> (uintptr_t)low_address, (uintptr_t)high_address,
>
> Use p2i, not cast
Here, and in other places too where you cast a pointer to fit into PTR_FORMAT
or INTPTR_FORMAT
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1544172412
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1544181011
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Thu, 28 Mar 2024 16:50:20 GMT, Joachim Kern wrote:
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building
> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect
> clang by another name, and it uses the clang toolchain in the JDK build. Thus
> the old xlc toolchain was removed by
> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701).
> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the
> last xlc rudiment.
> This means merging the AIX specific content of
> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp
> into the corresponding gcc files on the on side and removing the
> defined(TARGET_COMPILER_xlc) blocks in the code, because the
> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX
> compiler.
> The rest of the changes are needed because of using
> utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about
> ill formatted printf
Hi @JoKern65 ,
this is a nice simplification!
There are many casts that should be unnecessary (casting size_t to fit into
SIZE_FORMAT) or that should use `p2i` (all those casts to fit pointers into
PTR_FORMAT or INTPTR_FORMAT). I did not mark all of them.
We also have a new macro for printing memory ranges, RANGEFMT and RANGEFMTARGS.
Maybe some call sites could that instead and make the code shorter and better
to read? But I leave that up to you.
Cheers, and happy Easter!
src/hotspot/os/aix/loadlib_aix.cpp line 120:
> 118: (lm->is_in_vm ? '*' : ' '),
> 119: (uintptr_t)lm->text, (uintptr_t)lm->text + lm->text_len,
> 120: (uintptr_t)lm->data, (uintptr_t)lm->data + lm->data_len,
Please don't cast, use `p2i()`.
src/hotspot/os/aix/os_aix.cpp line 314:
> 312: ErrnoPreserver ep;
> 313: log_trace(os, map)("disclaim failed: " RANGEFMT " errno=(%s)",
> 314: RANGEFMTARGS(p, (long)maxDisclaimSize),
Wait, why are these casts needed? maxDisclaimSize is size_t, RANGEFMT uses
SIZE_FORMAT. That should work without cast.
src/hotspot/os/aix/os_aix.cpp line 651:
> 649: lt.print("Thread is alive (tid: " UINTX_FORMAT ", kernel thread id:
> " UINTX_FORMAT
> 650: ", stack [" PTR_FORMAT " - " PTR_FORMAT " (" SIZE_FORMAT "k
> using %luk pages)).",
> 651: os::current_thread_id(), (uintx) kernel_thread_id,
> (uintptr_t)low_address, (uintptr_t)high_address,
Use p2i, not cast
src/hotspot/os/aix/os_aix.cpp line 1212:
> 1210: st->print_cr("physical free : " SIZE_FORMAT, (unsigned
> long)mi.real_free);
> 1211: st->print_cr("swap total : " SIZE_FORMAT, (unsigned
> long)mi.pgsp_total);
> 1212: st->print_cr("swap free : " SIZE_FORMAT, (unsigned
> long)mi.pgsp_free);
A better way to do this would be to change AIX::meminfo to use size_t. We
should have done this when introducing that API.
src/hotspot/os/aix/os_aix.cpp line 1399:
> 1397: os->print("[" PTR_FORMAT " - " PTR_FORMAT "] (" UINTX_FORMAT
> 1398: " bytes, %ld %s pages), %s",
> 1399: (uintptr_t)addr, (uintptr_t)addr + size - 1, size, size /
> pagesize, describe_pagesize(pagesize),
p2i
src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 62:
> 60: #include
> 61:
> 62: #if defined(LINUX) || defined(_ALLBSD_SOURCE) || defined(_AIX)
What else is left? Could we just remove this line altogether now?
src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 83:
> 81: #error "xlc version not supported, macro __open_xl_version__ not found"
> 82: #endif
> 83: #endif // AIX
Can probably be shortened like this:
Suggestion:
#ifdef _AIX
#if !defined(__open_xl_version__) || (__open_xl_version__ < 17)
#error "this xlc version is not supported"
#endif
#endif // AIX
src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 103:
> 101: #endif
> 102:
> 103: #if !defined(LINUX) && !defined(_ALLBSD_SOURCE) && !defined(_AIX)
I believe this whole section can be removed now.
At least I have no idea who this is for. What gcc versions does OpenJDK still
support, then, beside these platforms. Also, any gcc platform not on linux or
bsd would have hit the #error below at line 132.
src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 128:
> 126: #if defined(__APPLE__)
> 127: inline int g_isnan(double f) { return isnan(f); }
> 128: #elif defined(LINUX) || defined(_ALLBSD_SOURCE) || defined(_AIX)
I think this can be just #else
src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 131:
> 129: inline int g_isnan(float f) { return isnan(f); }
> 130: inline int g_isnan(double f) { return isnan(f); }
> 131: #else
and this can be removed
-
Changes requested by stuefe (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18536#pullrequestreview-1967997963
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1544171741
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1544174303
PR Review
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Thu, 28 Mar 2024 16:50:20 GMT, Joachim Kern wrote: > As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about > ill formatted printf That one singular build change looks good :) > The rest of the changes are needed because of using > utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about > ill formatted printf Did you mean compilerWarnings_gcc.hpp? - Marked as reviewed by jwaters (Committer). PR Review: https://git.openjdk.org/jdk/pull/18536#pullrequestreview-1967860264 PR Comment: https://git.openjdk.org/jdk/pull/18536#issuecomment-2026674128
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Thu, 28 Mar 2024 16:53:39 GMT, Joachim Kern wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about >> ill formatted printf > > src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 83: > >> 81: #error "xlc version not supported, macro __open_xl_version__ not found" >> 82: #endif >> 83: #endif // AIX > > This `#ifdef _AIX` might be obsolete, because configure will throw a warning > if the compiler has a lower version, but it's only a warning. I'd prefer having less AIX specific parts in this file. Can this be moved somewhere else? Or maybe combine it with the AIX code above? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1543371129
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Thu, 28 Mar 2024 16:50:20 GMT, Joachim Kern wrote:
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building
> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect
> clang by another name, and it uses the clang toolchain in the JDK build. Thus
> the old xlc toolchain was removed by
> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701).
> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the
> last xlc rudiment.
> This means merging the AIX specific content of
> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp
> into the corresponding gcc files on the on side and removing the
> defined(TARGET_COMPILER_xlc) blocks in the code, because the
> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX
> compiler.
> The rest of the changes are needed because of using
> utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about
> ill formatted printf
src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 50:
> 48: #undef malloc
> 49: extern void *malloc(size_t) asm("vec_malloc");
> 50: #endif
This `#if` is not needed if we are building on AIX 7.2 TL5 SP7 or higher. This
is the case in our build environment, but I think IBM is still building with
SP5, which would run into build errors if I remove this `#if` now.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1543312492
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Thu, 28 Mar 2024 16:50:20 GMT, Joachim Kern wrote: > As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about > ill formatted printf src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 83: > 81: #error "xlc version not supported, macro __open_xl_version__ not found" > 82: #endif > 83: #endif // AIX This `#ifdef _AIX` might be obsolete, because configure will throw a warning if the compiler has a lower version, but it's only a warning. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1543307859
