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 [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 [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 [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
