Integrated: 8330539: Use #include instead of -Dalloca'(size)'=__builtin_alloca'(size)' for AIX
On Thu, 2 May 2024 09:54:14 GMT, Joachim Kern wrote: > We need to find a better way to handle alloca on AIX. > > See the discussion in the PR for https://bugs.openjdk.org/browse/JDK-8329257, > e.g. https://github.com/openjdk/jdk/pull/18536#discussion_r1568650313 in > which three alternatives are suggested. Quoting: > > Let me summarize the choices we have and ask for your vote. > Magnus dislikes the -Dalloca'(size)'=__builtin_alloca'(size)' in > flags-cflags.m4 I introduced to get rid of > > #if defined(_AIX) > #include > #endif > > in globalDefinitions_gcc.hpp. > > We have four possible solutions > > 1. Reintroduce > > #if defined(_AIX) > #include > #endif > > in globalDefinitions_gcc.hpp. > > 2. Unconditionally introduce only #include in > globalDefinitions_gcc.hpp. This should work for all platforms using this > header including the unofficial Windows/gcc Port, although only AIX needs it. > > 3. Add > > #if defined(_AIX) > #include > #endif > > to the sources using alloca(). These are > /hotspot/share/runtime/os.cpp > /hotspot/share/runtime/javaThread.cpp > /hotspot/share/utilities/vmError.cpp > Here we need the AIX condition, because otherwise the classic Windows Build > (NTAMD64) fails. > > 4. Replace -Dalloca'(size)'=__builtin_alloca'(size)' in flags-cflags.m4 by > -U__STRICT_ANSI__ at the same place. Explanation can also found in > https://github.com/openjdk/jdk/pull/18536#discussion_r1583360569 and > following. > > I will implement the solution with the most likes and having no dislike. This pull request has now been integrated. Changeset: a10845b5 Author:Joachim Kern Committer: Martin Doerr URL: https://git.openjdk.org/jdk/commit/a10845b553fc6fe7e06a0f37ce73fe5f704dc7c4 Stats: 2 lines in 2 files changed: 1 ins; 0 del; 1 mod 8330539: Use #include instead of -Dalloca'(size)'=__builtin_alloca'(size)' for AIX Reviewed-by: jwaters, mdoerr, kbarrett, ihse - PR: https://git.openjdk.org/jdk/pull/19053
RFR: 8330539: Use #include instead of -Dalloca'(size)'=__builtin_alloca'(size)' for AIX
We need to find a better way to handle alloca on AIX. See the discussion in the PR for https://bugs.openjdk.org/browse/JDK-8329257, e.g. https://github.com/openjdk/jdk/pull/18536#discussion_r1568650313 in which three alternatives are suggested. Quoting: Let me summarize the choices we have and ask for your vote. Magnus dislikes the -Dalloca'(size)'=__builtin_alloca'(size)' in flags-cflags.m4 I introduced to get rid of #if defined(_AIX) #include #endif in globalDefinitions_gcc.hpp. We have four possible solutions 1. Reintroduce #if defined(_AIX) #include #endif in globalDefinitions_gcc.hpp. 2. Unconditionally introduce only #include in globalDefinitions_gcc.hpp. This should work for all platforms using this header including the unofficial Windows/gcc Port, although only AIX needs it. 3. Add #if defined(_AIX) #include #endif to the sources using alloca(). These are /hotspot/share/runtime/os.cpp /hotspot/share/runtime/javaThread.cpp /hotspot/share/utilities/vmError.cpp Here we need the AIX condition, because otherwise the classic Windows Build (NTAMD64) fails. 4. Replace -Dalloca'(size)'=__builtin_alloca'(size)' in flags-cflags.m4 by -U__STRICT_ANSI__ at the same place. Explanation can also found in https://github.com/openjdk/jdk/pull/18536#discussion_r1583360569 and following. I will implement the solution with the most likes and having no dislike. - Commit messages: - JDK-8330539 Changes: https://git.openjdk.org/jdk/pull/19053/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19053&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8330539 Stats: 2 lines in 2 files changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19053.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19053/head:pull/19053 PR: https://git.openjdk.org/jdk/pull/19053
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 30 Apr 2024 14:39:29 GMT, Magnus Ihse Bursie wrote: >> Ok for me. Let's hear what @kimbarrett thinks. > > It might be easier to get input if you create a new PR with the change. This > discussion is hidden deep down in a closed PR. I will do after labor day and create a PR with this suggested solution in your JDK-8330539. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1585020690
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 30 Apr 2024 12:46:31 GMT, Magnus Ihse Bursie wrote: >> I got it. And what about simply disabling the `__STRICT_ANSI__` with >> `CFLAGS_OS_DEF_JVM="-DAIX -D_LARGE_FILES -U__STRICT_ANSI__"` in >> flags-cflags.m4 for AIX. This worked too. The build is fine. > > So what you are saing is basically replacing > > CFLAGS_OS_DEF_JVM="-DAIX -Dalloca'(size)'=__builtin_alloca'(size)' > -D_LARGE_FILES" > ``` > with > > CFLAGS_OS_DEF_JVM="-DAIX -D_LARGE_FILES -U__STRICT_ANSI__" > ``` > ? > > Yeah, that'll work, I guess. "strict ansi" sounds like a problematic thing to > have enabled, and that it is added by `-std=c++14` sounds close to a bug in > my ears. So a "workaround" where this is disabled seem reasonable. Yes this would be the replacement. This is our 4th way to fix the issue. Anyone else who would prefer this too? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1584847372
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 30 Apr 2024 10:19:30 GMT, Magnus Ihse Bursie wrote: >> The compiler flag introducing __STRICT_ANSI__ is -std=c++14. If I omit this >> explicit compiler flag the default is used, which is also c++14. But the >> default does not set __STRICT_ANSI__ but 2 other defines. I will try a build >> without -std=c++14 and if this works, we have a solution. Nevertheless i >> will interrogate IBM what the hell this behavior should be. > > I don't think leaving out `-std=c++14` for AIX is a good solution. I got it. And what about simply disabling the `__STRICT_ANSI__` with `CFLAGS_OS_DEF_JVM="-DAIX -D_LARGE_FILES -U__STRICT_ANSI__"` in flags-cflags.m4 for AIX. This worked too. The build is fine. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1584725053
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Mon, 29 Apr 2024 16:17:13 GMT, Joachim Kern wrote: >> For the impatient, I suggest adopting mechanism 2, i.e. unconditionally >> include in globalDefinitions_gcc.hpp. >> >> We can't include in shared code, and there is a use in shared code >> (in the relatively recently added JavaThread::pretouch_stack). >> >> When I questioned whether we needed to include at all, I referred >> to a Linux man page I'd found on the internet (the same page mdoerr linked >> to), which says (in part) >> >> "By default, modern compilers automatically translate all uses of alloca() >> into the built-in ..." >> >> Apparently I should have kept digging, because it seems that page is >> old/incorrect. A seemingly more recent Linux man page describes a different >> way of handling it that is closer to what we're seeing, but still not quite >> correct. >> >> glibc's includes if __USE_MISC is defined. >> One of the ways __USE_MISC can become defined is if _GNU_SOURCE is defined, >> and we define that for both gcc and clang toolchains. >> >> We include in globalDefinitions_gcc.hpp. So when building with >> gcc, >> globalDefinitions.hpp implicitly includes . >> >> The glibc definition of alloca is >> >> #ifdef __GNUC__ >> # define alloca(size)__builtin_alloca (size) >> #endif /* GCC. */ >> >> So that explains why we don't need any explicit include of when >> building with gcc. I expect there's something similar going on with Visual >> Studio and Xcode/clang. But apparently not with Open XLC clang. > > On AIX `stdlib.h` also would define `alloca`, if `__STRICT_ANSI__` wouldn't > be set. > > > 780 #if !defined(__xlC__) || defined(__ibmxl__) || defined(__cplusplus) > 781 #if defined(__IBMCPP__) && !defined(__ibmxl__) > 782extern "builtin" char *__alloca (size_t); > 783 # define alloca __alloca > 784 #elif defined(__GNUC__) && !defined(__STRICT_ANSI__) > 785#undef alloca > 786#define alloca(size) __builtin_alloca (size) > 787 #endif > > > A small plain Testprogramm not using all of the flags we used in jdk build, > does not set `__STRICT_ANSI__` and then `alloca` is defined correct. The compiler flag introducing __STRICT_ANSI__ is -std=c++14. If I omit this explicit compiler flag the default is used, which is also c++14. But the default does not set __STRICT_ANSI__ but 2 other defines. I will try a build without -std=c++14 and if this works, we have a solution. Nevertheless i will interrogate IBM what the hell this behavior should be. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1584461665
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Thu, 18 Apr 2024 04:26:21 GMT, Kim Barrett wrote: >> I opened https://bugs.openjdk.org/browse/JDK-8330539 so we don't lose track >> of this, but we can keep the discussion/voting here. > > For the impatient, I suggest adopting mechanism 2, i.e. unconditionally > include in globalDefinitions_gcc.hpp. > > We can't include in shared code, and there is a use in shared code > (in the relatively recently added JavaThread::pretouch_stack). > > When I questioned whether we needed to include at all, I referred > to a Linux man page I'd found on the internet (the same page mdoerr linked > to), which says (in part) > > "By default, modern compilers automatically translate all uses of alloca() > into the built-in ..." > > Apparently I should have kept digging, because it seems that page is > old/incorrect. A seemingly more recent Linux man page describes a different > way of handling it that is closer to what we're seeing, but still not quite > correct. > > glibc's includes if __USE_MISC is defined. > One of the ways __USE_MISC can become defined is if _GNU_SOURCE is defined, > and we define that for both gcc and clang toolchains. > > We include in globalDefinitions_gcc.hpp. So when building with gcc, > globalDefinitions.hpp implicitly includes . > > The glibc definition of alloca is > > #ifdef__GNUC__ > # define alloca(size) __builtin_alloca (size) > #endif /* GCC. */ > > So that explains why we don't need any explicit include of when > building with gcc. I expect there's something similar going on with Visual > Studio and Xcode/clang. But apparently not with Open XLC clang. On AIX `stdlib.h` also would define `alloca`, if `__STRICT_ANSI__` wouldn't be set. 780 #if !defined(__xlC__) || defined(__ibmxl__) || defined(__cplusplus) 781 #if defined(__IBMCPP__) && !defined(__ibmxl__) 782extern "builtin" char *__alloca (size_t); 783 # define alloca __alloca 784 #elif defined(__GNUC__) && !defined(__STRICT_ANSI__) 785#undef alloca 786#define alloca(size) __builtin_alloca (size) 787 #endif A small plain Testprogramm not using all of the flags we used in jdk build, does not set `__STRICT_ANSI__` and then `alloca` is defined correct. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1583360569
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 16 Apr 2024 09:15:19 GMT, Magnus Ihse Bursie wrote: >> That was kind of where the discussion started, and which Kim did not like. >> If I read him correctly, his suggestion was instead to place: >> >> #if defined(_AIX) >> #include >> #endif >> >> in the files where `alloca` is needed on AIX. > > (If some of these files happen to be files which are not compiled on Windows, > I assume it will not hurt to drop the ifdef guard, but then again, it can > certainly be kept as well for consistency.) @magicus @TheShermanTanker @TheRealMDoerr @kimbarrett Let me summarize the choices we have and ask for your vote. Julian dislikes the `-Dalloca'(size)'=__builtin_alloca'(size)'` in `flags-cflags.m4` I introduced to get rid of #if defined(_AIX) #include #endif in `globalDefinitions_gcc.hpp`. We have three possible solutions 1. Reintroduce #if defined(_AIX) #include #endif in `globalDefinitions_gcc.hpp`. 2. Unconditionally introduce only `#include ` in `globalDefinitions_gcc.hpp`. This should work for all platforms using this header including the unofficial Windows/gcc Port, although only AIX needs it. 3. Add #if defined(_AIX) #include #endif to the sources using alloca(). These are /hotspot/share/runtime/os.cpp /hotspot/share/runtime/javaThread.cpp /hotspot/share/utilities/vmError.cpp Here we need the AIX condition, because otherwise the classic Windows Build (NTAMD64) fails. I will implement the solution with the most likes and having no dislike. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1568650313
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 10 Apr 2024 22:14:33 GMT, Kim Barrett wrote: >> That build failure in shared code does not happen with Xcode clang, gcc, or >> Visual Studio, even though none of them appear to have a relevant define or >> include. So the clang variant being used for AIX is different from the Xcode >> clang variant (and maybe others) in its treatment of alloca. Weird! >> >> I can also live with either the macro or the includes where needed. I >> dislike >> conditionally adding the include in globalDefinitions_gcc.hpp. > > Should also remove the `#pragma alloca` in os_aix.cpp. We can not use `#include ` in all files which use `alloca`, because windows does not know this header. Maybe we can use `#include ` unconditionally in globalDefinitions_gcc.hpp, if windows will never use this file. @kimbarrett What do you say? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1566988309
Integrated: 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_gcc.hpp the compiler is much more nagging about > ill formatted printf This pull request has now been integrated. Changeset: 3f1d9c44 Author:Joachim Kern Committer: Martin Doerr URL: https://git.openjdk.org/jdk/commit/3f1d9c441ea98910d9483e133bccfac784db393d Stats: 256 lines in 15 files changed: 8 ins; 212 del; 36 mod 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc Reviewed-by: jwaters, stuefe, kbarrett, mdoerr - PR: https://git.openjdk.org/jdk/pull/18536
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v8]
> 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: my_disclaim64 already removed by other PR - Changes: - all: https://git.openjdk.org/jdk/pull/18536/files - new: https://git.openjdk.org/jdk/pull/18536/files/a8d85924..030de164 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18536&range=07 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18536&range=06-07 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 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 [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]
> 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&pr=18536&range=06 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18536&range=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&pr=18536&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18536&range=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&pr=18536&range=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 [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&pr=18536&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18536&range=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]
> 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&pr=18536&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18536&range=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]
> 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&pr=18536&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18536&range=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 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
RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
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 - Commit messages: - JDK-8329257 Changes: https://git.openjdk.org/jdk/pull/18536/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18536&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8329257 Stats: 261 lines in 13 files changed: 21 ins; 208 del; 32 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: 8327701: Remove the xlc toolchain [v4]
On Tue, 12 Mar 2024 15:27:29 GMT, Magnus Ihse Bursie 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 is no longer supported, and should be removed. > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Replace CC_VERSION_OUTPUT with CC_VERSION_STRING > - We need CC_VERSION_OUTPUT before we can check it e.g. We should change the HOTSPOT_TOOLCHAIN_TYPE=xlc to aix, because it is not toolchain, but OS related. As a consequence the globalDefinitions_xlc.hpp will become globalDefinitions_aix.hpp - PR Comment: https://git.openjdk.org/jdk/pull/18172#issuecomment-1993979991
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:48:08 GMT, Magnus Ihse Bursie 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 is no longer supported, and should be removed. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Revert SEARCH_PATH changes Changes requested by jkern (Author). - PR Review: https://git.openjdk.org/jdk/pull/18172#pullrequestreview-1927875301
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Mon, 11 Mar 2024 08:59:03 GMT, Joachim Kern wrote: >> make/autoconf/flags-cflags.m4 line 687: >> >>> 685: PICFLAG="-fPIC" >>> 686: PIEFLAG="-fPIE" >>> 687: elif test "x$TOOLCHAIN_TYPE" = xclang && test "x$OPENJDK_TARGET_OS" >>> = xaix; then >> >> Just a remark: This code has never been executed, since running with clang >> on any OS would hit the branch above. Also, the code is syntactically >> incorrect, missing a trailing `"`. > > OK this was a flaw in my introduction of clang toolchain for AIX. The > intention was to keep the xlc options in form of their clang counterparts. I > will try with a corrected version for clang on AIX and will come back to you. OK, the `-Wl,-bbigtoc` is not a compiler option but a linker option and it is already set in the linker options. But the `-fpic -mcmodel=large` should be set to avoid creating a jump to out-of-order code. So we can replace the JVM_PICFLAG="$PICFLAG" JDK_PICFLAG="$PICFLAG" code some lines below by if test "x$TOOLCHAIN_TYPE" = xclang && test "x$OPENJDK_TARGET_OS" = xaix; then JVM_PICFLAG="-fpic -mcmodel=large" else JVM_PICFLAG="$PICFLAG" fi JDK_PICFLAG="$PICFLAG" - PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1519747481
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:48:08 GMT, Magnus Ihse Bursie 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 is no longer supported, and should be removed.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Revert SEARCH_PATH changes
make/autoconf/toolchain.m4 line 940:
> 938: if test "x$OPENJDK_TARGET_OS" = xaix; then
> 939: # Make sure we have the Open XL version of clang on AIX
> 940: $ECHO "$CC_VERSION_OUTPUT" | $GREP "IBM Open XL C/C++ for AIX" >
> /dev/null
This does not work since $CC_VERSION_OUTPUT is unset. We need
CC_VERSION_OUTPUT=`${XLC_TEST_PATH}ibm-clang++_r --version 2>&1 | $HEAD
-n 1`
before, as in the previous code some lines above which you removed.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1519634810
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:31:18 GMT, Magnus Ihse Bursie wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Revert SEARCH_PATH changes > > make/autoconf/flags-cflags.m4 line 687: > >> 685: PICFLAG="-fPIC" >> 686: PIEFLAG="-fPIE" >> 687: elif test "x$TOOLCHAIN_TYPE" = xclang && test "x$OPENJDK_TARGET_OS" = >> xaix; then > > Just a remark: This code has never been executed, since running with clang on > any OS would hit the branch above. Also, the code is syntactically incorrect, > missing a trailing `"`. OK this was a flaw in my introduction of clang toolchain for AIX. The intention was to keep the xlc options in form of their clang counterparts. I will try with a corrected version for clang on AIX and will come back to you. - PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1519347031
Re: RFR: 8327701: Remove the xlc toolchain [v2]
On Fri, 8 Mar 2024 15:48:08 GMT, Magnus Ihse Bursie 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 is no longer supported, and should be removed.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Revert SEARCH_PATH changes
Changes requested by jkern (Author).
doc/building.html line 679:
> 677: IBM Open XL C/C++
> 678: The minimum accepted version of Open XL is 17.1.1.4. This is in
> 679: essence clang 13, and will be treated as such by the OpenJDK build
It is clang 15 not 13. Clang 13 was in 17.1.0
doc/building.md line 493:
> 491:
> 492: The minimum accepted version of Open XL is 17.1.1.4. This is in essence
> clang
> 493: 13, and will be treated as such by the OpenJDK build system.
clang 15 not 13
make/autoconf/toolchain.m4 line 285:
> 283: XLC_TEST_PATH=${TOOLCHAIN_PATH}/
> 284: fi
> 285: if test "x$TOOLCHAIN_TYPE" = xclang; then
Why did you also remove the test for the clang compiler? Ah I see, you moved
the clang compiler check down below
make/autoconf/toolchain.m4 line 409:
> 407: #Target: powerpc-ibm-aix7.2.0.0
> 408: #Thread model: posix
> 409: #InstalledDir: /opt/IBM/openxlC/17.1.0/bin
Please correct:
IBM Open XL C/C++ for AIX 17.1.**1** (xxx), clang version 1**5**.0.0
#Target: **powerpc-ibm-aix7.2.**5**.7**
#Thread model: posix
#InstalledDir: /opt/IBM/openxlC/17.1.**1**/bin
-
PR Review: https://git.openjdk.org/jdk/pull/18172#pullrequestreview-1927198641
PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1519321337
PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1519321980
PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1519358938
PR Review Comment: https://git.openjdk.org/jdk/pull/18172#discussion_r1519365934
Re: RFR: 8325880: Require minimum Open XL C/C++ version 17.1.1
On Tue, 5 Mar 2024 08:41:00 GMT, Kim Barrett wrote: > > > What does this mean? That you are not using xlc at all? Or is it clang > > > but still with an xlc frontend, so all xlc flags etc need to stay? > > > > > > The `xlc` toolchain is for the compiler versions up to 16 (xlclang++); the > > `clang` toolchain is for the compiler versions 17 (ibm-clang++_r) and > > higher. For the 17 Compiler the frontend is `clang`-ish and we are using > > the `clang` flags instead of the `xlc` flags. `toolchain.m4` decides on the > > basis of the found compiler which toolchain to use as default. > > ``` > > # On AIX the default toolchain depends on the installed (found) compiler > > # xlclang++ -> xlc toolchain > > # ibm-clang++_r -> clang toolchain > > # The compiler is searched on the PATH and TOOLCHAIN_PATH > > # xlclang++ has precedence over ibm-clang++_r if both are installed > > ``` > > > > > > > > > > > > > > > > > > > > > > > > So, if we set the minimum compiler level for AIX to 17, we can remove the > > xlc toolchain at all. We cannot remove every reference to xlc, because at > > least some headers we still use the xlc version (globalDefinitions_xlc.hpp) > > This suggests there might be more that needs to be done here than simply > updating TOOLCHAIN_MINIMUM_VERSION_xlc. I spent some time looking at the > relevant code, but keep getting lost in the distinction between xlc and > clang. Does updating that variable as proposed even work at all? > > I'm going to need some help from you aix-ppc maintainer folks. As I already mentioned, This PR is just the start shot to remove the support for the old xlc compilers below version 17. This means removing the xlc toolchain support in a follow up PR by our team. This is feasible, because the open xl compilers starting with version 17 are using the clang toolchain. So, if this PR is through we feel empowered to remove the xlc toolchain. - PR Comment: https://git.openjdk.org/jdk/pull/17857#issuecomment-1978279887
Re: RFR: 8325880: Require minimum Open XL C/C++ version 17.1.1
On Thu, 15 Feb 2024 12:49:26 GMT, Julian Waters wrote: > > > I see. I believe I wrote that piece of code, but I'd clearly forgotten > > > that. 😕 Thanks! :) > > > > > > No, this was added by me, because this was the root point to still resolve > > to globalDefinitions_xlc.hpp even with toolchain clang > > Shame that we can't fully swap to clang in some areas for AIX, but oh well Although the compiler is now clang, the headers are still the old IBM ones and globalDefinitions_xlc.hpp is not consumable by other clang implementations. So if we change this we still have to differentiate between AIX clang and other clangs. - PR Comment: https://git.openjdk.org/jdk/pull/17857#issuecomment-1946086792
Re: RFR: 8325880: Require minimum Open XL C/C++ version 17.1.1
On Thu, 15 Feb 2024 12:29:50 GMT, Magnus Ihse Bursie wrote: > I see. I believe I wrote that piece of code, but I'd clearly forgotten that. > 😕 Thanks! :) No, this was added by me, because this was the root point to still resolve to globalDefinitions_xlc.hpp even with toolchain clang - PR Comment: https://git.openjdk.org/jdk/pull/17857#issuecomment-1946015507
Re: RFR: 8325880: Require minimum Open XL C/C++ version 17.1.1
On Thu, 15 Feb 2024 10:40:53 GMT, Magnus Ihse Bursie wrote: > What does this mean? That you are not using xlc at all? Or is it clang but > still with an xlc frontend, so all xlc flags etc need to stay? The `xlc` toolchain is for the compiler versions up to 16 (xlclang++); the `clang` toolchain is for the compiler versions 17 (ibm-clang++_r) and higher. For the 17 Compiler the frontend is `clang`-ish and we are using the `clang` flags instead of the `xlc` flags. `toolchain.m4` decides on the basis of the found compiler which toolchain to use as default. # On AIX the default toolchain depends on the installed (found) compiler # xlclang++ -> xlc toolchain # ibm-clang++_r -> clang toolchain # The compiler is searched on the PATH and TOOLCHAIN_PATH # xlclang++ has precedence over ibm-clang++_r if both are installed So, if we set the minimum compiler level for AIX to 17, we can remove the xlc toolchain at all. We cannot remove every reference to xlc, because at least some headers we still use the xlc version (globalDefinitions_xlc.hpp) - PR Comment: https://git.openjdk.org/jdk/pull/17857#issuecomment-1945873440
Re: RFR: 8325880: Require minimum Open XL C/C++ version 17.1.1
On Wed, 14 Feb 2024 22:43:37 GMT, Kim Barrett wrote: > Please review this change that updates the minimum supported version of IBM > Open XL C/C++. SAP dropped support for older versions in JDK 22, only > supporting the version specified in this change. > > I need someone from the aix-ppc porters to test and review the change. make/autoconf/toolchain.m4 line 56: > 54: TOOLCHAIN_MINIMUM_VERSION_gcc="6.0" > 55: TOOLCHAIN_MINIMUM_VERSION_microsoft="19.28.0.0" # VS2019 16.8, aka MSVC > 14.28 > 56: TOOLCHAIN_MINIMUM_VERSION_xlc="17.1.1.4" We do not build AIX with the xlc toolchain any more but the clang one. So this line only stops a build if someone is trying to build with xlc 16 against toolchain xlc. I have to agree to @TheRealMDoerr, that the correct change would be to remove the xlc toolchain in jdk23 at all. - PR Review Comment: https://git.openjdk.org/jdk/pull/17857#discussion_r1490650675
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v10]
On Thu, 8 Feb 2024 14:47:26 GMT, Joachim Kern wrote: >> And also `#define statvfs statvfs64` is not necessary with the same >> explanation as for the `opendir` defines above -- sorry again. >> The very only difference between statvfs and statvfs64 is that the >> filesystem ID field in statvfs has a width of 8 Bytes, while in statvfs64 it >> has a width of 16 Bytes. > >> @JoKern65 So what about `#define fstatvfs fstatvfs64`? Is that still needed? >> If so, I could not be bothered to make another change. Otherwise, we can get >> rid of _all_ AIX 64-bit redefines, and then I'll (probably) do it. > > Same as `statvfs`. Only the file system ID field is smaller. > @JoKern65 @MBaesken I did not receive any reply about what to do with > `fstatvfs`, so I decided to keep the last verified change for AIX. If you > want to clean this up, then please do so, but at that time it will be an > AIX-only patch. @magicus I have to reach out to IBM asking if the different size of the 'filesystem ID' field in statvfs makes an important difference. If not, I will remove the defines in an AIX-only patch. Thanks a lot for your effort. - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1938300228
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v10]
On Thu, 8 Feb 2024 09:03:10 GMT, Joachim Kern wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Once more, remove AIX dirent64 et al defines > > And also `#define statvfs statvfs64` is not necessary with the same > explanation as for the `opendir` defines above -- sorry again. > The very only difference between statvfs and statvfs64 is that the filesystem > ID field in statvfs has a width of 8 Bytes, while in statvfs64 it has a width > of 16 Bytes. > @JoKern65 So what about `#define fstatvfs fstatvfs64`? Is that still needed? > If so, I could not be bothered to make another change. Otherwise, we can get > rid of _all_ AIX 64-bit redefines, and then I'll (probably) do it. Same as `statvfs`. Only the file system ID field is smaller. - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1934275624
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v10]
On Thu, 8 Feb 2024 07:44:18 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Once more, remove AIX dirent64 et al defines And also `#define statvfs statvfs64` is not necessary with the same explanation as for the `opendir` defines above -- sorry again. - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1933630674
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v9]
On Tue, 6 Feb 2024 08:18:14 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Also fix fstatvfs on AIX My apologies the additional defines `#define DIR DIR64` `#define dirent dirent64` `#define opendir opendir64` `#define readdir readdir64` `#define closedir closedir64` are not necessary. Indeed they do not react on _LARGE_FILES, but the DIR struct and the functions are automatically 64 when compiling in 64bit mode (-m64) as we do. - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1932343048
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Mon, 5 Feb 2024 12:07:45 GMT, Matthias Baesken wrote: > Current commit compiles nicely on AIX. One issue we might still have > statvfs/statvfs64 is not mentioned here in the table of functions/structs > redefined on AIX > https://www.ibm.com/docs/en/aix/7.1?topic=volumes-writing-programs-that-access-large-files > so would we fall back to statvfs from the *64 - variant ? The define > _LARGE_FILES might not help in this case on AIX . Yes, if statvfs64() is replaced by statvfs() in the code, we will fallback on AIX to 32-Bit. _LARGE_FILES really does not help in this case! - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1926865295
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs
On Tue, 23 Jan 2024 15:42:55 GMT, Magnus Ihse Bursie wrote: > Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we > should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK > native libraries. src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c line 83: > 81: #define readdir readdir64 > 82: #define closedir closedir64 > 83: #endif same as above - PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1469676165
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs
On Tue, 23 Jan 2024 15:42:55 GMT, Magnus Ihse Bursie wrote: > Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we > should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK > native libraries. src/java.prefs/unix/native/libprefs/FileSystemPreferences.c line 71: > 69: jintArray javaResult = NULL; > 70: int old_umask; > 71: struct flock fl; On AIX flock does not react on _LARGE_FILES - PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1469659732
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs
On Tue, 23 Jan 2024 15:42:55 GMT, Magnus Ihse Bursie wrote: > Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we > should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK > native libraries. src/java.base/share/native/libjli/wildcard.c line 109: > 107: #define readdir readdir64 > 108: #define closedir closedir64 > 109: #endif You have to keep these defines for AIX, because _LARGE_FILES does not act on them, but instead of `#if defined(_AIX)` you could use `#if defined(_AIX) && defined(_LARGE_FILES)` src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c line 52: > 50: #if defined(_AIX) > 51: #include > 52: #endif same as above. src/java.base/unix/native/libjava/UnixFileSystem_md.c line 64: > 62: #define closedir closedir64 > 63: #define stat stat64 > 64: #endif same as above src/java.base/unix/native/libjava/childproc.c line 66: > 64: #define opendir opendir64 > 65: #define readdir readdir64 > 66: #define closedir closedir64 Same as above src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c line 179: > 177: #define readdir readdir64 > 178: #define closedir closedir64 > 179: #endif same as above - PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1469639725 PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1469642941 PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1469644433 PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1469645618 PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1469649038
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs
On Tue, 23 Jan 2024 15:42:55 GMT, Magnus Ihse Bursie wrote: > Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we > should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK > native libraries. make/autoconf/flags-cflags.m4 line 488: > 486: CFLAGS_OS_DEF_JDK="-D_ALLBSD_SOURCE -D_DARWIN_UNLIMITED_SELECT" > 487: elif test "x$OPENJDK_TARGET_OS" = xaix; then > 488: CFLAGS_OS_DEF_JVM="-DAIX" Why not CFLAGS_OS_DEF_JVM="-DAIX -D_LARGE_FILES" as the equivalent on Linux - PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1469627132
Re: RFR: 8324834: Use _LARGE_FILES on AIX
On Mon, 29 Jan 2024 11:44:34 GMT, Magnus Ihse Bursie wrote: > In the same spirit as > [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we should adapt > the AIX-specific code in hotspot so it uses the well-defined posix `` > functions, instead of `64`. By setting the define _LARGE_FILES, this > will make `` behave as `64`, just as _FILE_OFFSET_BITS=64 does on > gcc. (Reference: > https://www.ibm.com/docs/en/aix/7.1?topic=volumes-writing-programs-that-access-large-files) > > In theory, it should not even be necessary to set this, since we only compile > for 64-bit AIX platforms, and this is only relevant on 32-bit platforms. But > let's add the define anyway, for good measure. It shows at least that we have > thought about the matter. :-) > > I have not been able to test this on AIX. I hope someone with AIX access can > take this for a spin. > > The reason I'm doing this is for > [JDK-8324539](https://bugs.openjdk.org/browse/JDK-8324539). After both these > bugs are fixed, there will be no more `64` function calls in the code > base. I have to emphasize, that even when exclusively compiling for 64-bit AIX platforms, the `` functions do not support large files. Only the `64` functions do support large files. For large file support you can either use the `64` functions and the `64` structures explicitly or by compiling all sources with the -D_LARGE_FILES flag which just leads to a redefining of all `` functions and `` structures as `64` functions and `64` structures. - PR Comment: https://git.openjdk.org/jdk/pull/17611#issuecomment-1914737783
