Re: RFR: 8339364: AIX build fails: various unused variable and function warnings [v3]
On Tue, 3 Sep 2024 07:26:53 GMT, Matthias Baesken wrote: >> We get a couple of warnings as errors on AIX because of unused variables or >> functions , for example : >> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:665:10: >> error: unused variable 'exePath' [-Werror,-Wunused-variable] >> char exePath[PATH_MAX]; >> ^ >> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:668:9: >> error: unused variable 'ret' [-Werror,-Wunused-variable] >> int ret; >> ^ >> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:664:10: >> error: unused variable 'fn' [-Werror,-Wunused-variable] >> char fn[32]; >> ^ >> >> This seems to be related to the recent make changes >> 8339156: Use more fine-granular clang unused warnings >> https://bugs.openjdk.org/browse/JDK-8339156 >> (we use clang too on AIX) > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > TimeZone_md move platform stuff Thanks for the update! I also think that adding result checks is better in a separate RFE. This build fix is good to go IMHO. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/20812#pullrequestreview-2276906529
Re: RFR: 8339364: AIX build fails: various unused variable and function warnings [v2]
On Mon, 2 Sep 2024 13:25:51 GMT, Matthias Baesken wrote:
>> We get a couple of warnings as errors on AIX because of unused variables or
>> functions , for example :
>> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:665:10:
>> error: unused variable 'exePath' [-Werror,-Wunused-variable]
>> char exePath[PATH_MAX];
>> ^
>> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:668:9:
>> error: unused variable 'ret' [-Werror,-Wunused-variable]
>> int ret;
>> ^
>> /priv/jenkins/client-home/workspace/openjdk-jdk-dev-aix_ppc64-opt/jdk/src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:664:10:
>> error: unused variable 'fn' [-Werror,-Wunused-variable]
>> char fn[32];
>> ^
>>
>> This seems to be related to the recent make changes
>> 8339156: Use more fine-granular clang unused warnings
>> https://bugs.openjdk.org/browse/JDK-8339156
>> (we use clang too on AIX)
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> adjust indentation in X11Color.c
LGTM. Only minor nits.
src/java.base/unix/native/libjava/TimeZone_md.c line 63:
> 61: static const char *ETC_ENVIRONMENT_FILE = "/etc/environment";
> 62: #else
> 63: static char *isFileIdentical(char* buf, size_t size, char *pathname);
I think it should better be moved below `#if defined(__linux__) ||
defined(MACOSX)` instead of creating an else case.
src/java.desktop/unix/native/common/awt/X11Color.c line 1234:
> 1232: awt_allocate_systemrgbcolors (jint *rgbColors, int num_colors,
> 1233: AwtGraphicsConfigDataPtr awtData) {
> 1234: for (int i = 0; i < num_colors; i++)
This requires C99. Is this ok for all platforms with all supported compilers?
-
PR Review: https://git.openjdk.org/jdk/pull/20812#pullrequestreview-2276121217
PR Review Comment: https://git.openjdk.org/jdk/pull/20812#discussion_r1741226250
PR Review Comment: https://git.openjdk.org/jdk/pull/20812#discussion_r1741228253
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 30 Apr 2024 16:36:52 GMT, Kim Barrett wrote: >> I will do after labor day and create a PR with this suggested solution in >> your JDK-8330539. > > I think I still prefer just unconditionally including in > globalDefinitions_gcc.hpp. For gcc/clang we are using `-std=c++14` + > `-D_GNU_SOURCE` instead of `-std=gnu++14`. I forget exactly why. I don't > really want > to be messing with `__STRICT_ANSI__`. https://github.com/openjdk/jdk/pull/19053 - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1587736142
Re: RFR: 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. LGTM. This is my preferred solution. GHA failures are there because this PR is based on the code before https://github.com/openjdk/jdk/commit/286cbf831c2eb76e31bd69b4a93cd5ae9a821493 . - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19053#pullrequestreview-2035796660
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. Ok for me. Let's hear what @kimbarrett thinks. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1584884372
Re: RFR: 8331298: avoid alignment checks in UBSAN enabled build
On Mon, 29 Apr 2024 12:15:55 GMT, Matthias Baesken wrote: > Currently we run into some alignment related issues when building with > '--enable-ubsan' . Those errors already occur in the build. Fixing them might > take some time and maybe also some discussion if it is worth the effort , > So for now the alignment related checks should be disabled to get the UBSAN > build working. > Example : > > /jdk/src/hotspot/cpu/x86/macroAssembler_x86.hpp:128:13: runtime error: store > to misaligned address 0x15099c3cf4ce for type 'int', which requires 4 byte > alignment > 0x15099c3cf4ce: note: pointer points here > 00 80 0f 86 00 00 00 00 3d 06 00 00 80 76 60 3d 07 00 00 80 76 40 3d 08 00 > 00 80 76 20 3d 1e 00 > ^ > #0 0x1509b3b04f10 in MacroAssembler::pd_patch_instruction(unsigned char*, > unsigned char*, char const*, int) > /jdk/src/hotspot/cpu/x86/macroAssembler_x86.hpp:128 > #1 0x1509b3b04f10 in Label::patch_instructions(MacroAssembler*) > /jdk/src/hotspot/share/asm/assembler.cpp:201 > #2 0x1509b940b6d8 in VM_Version_StubGenerator::generate_get_cpu_info() > /jdk/src/hotspot/cpu/x86/vm_version_x86.cpp:381 > #3 0x1509b94059bd in VM_Version::initialize() > /jdk/src/hotspot/cpu/x86/vm_version_x86.cpp:2138 > #4 0x1509b93fb56e in VM_Version_init() > /jdk/src/hotspot/share/runtime/vm_version.cpp:32 > #5 0x1509b61ef947 in init_globals() > /jdk/src/hotspot/share/runtime/init.cpp:126 > #6 0x1509b8fb0e29 in Threads::create_vm(JavaVMInitArgs*, bool*) > /jdk/src/hotspot/share/runtime/threads.cpp:553 > #7 0x1509b67da3d7 in JNI_CreateJavaVM_inner > /jdk/src/hotspot/share/prims/jni.cpp:3581 > #8 0x1509b67da3d7 in JNI_CreateJavaVM > /jdk/src/hotspot/share/prims/jni.cpp:3672 > #9 0x1509c0eed957 in InitializeJVM > /jdk/src/java.base/share/native/libjli/java.c:1550 > #10 0x1509c0eed957 in JavaMain > /jdk/src/java.base/share/native/libjli/java.c:491 >... (rest of output omitted) I wish we could fix all UB, but that seems unrealistic for the time being. LGTM. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18998#pullrequestreview-2028402095
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 17 Apr 2024 12:22:10 GMT, Magnus Ihse Bursie wrote:
>> I don't mind all 3, though I certainly prefer 1 and 3 over 2 (The way I see
>> it, the AIX macro check is more of a message to the programmer than it is
>> important to the compiler, so I prefer the options that have it. However, I
>> also don't mind if we were to go the way of option 2, this is more of a
>> preference thing). The fact that only 3 files need it is also surprising to
>> me, and makes option 3 seem like a good fit (Again, personal preference)
>>
>> Magnus and Kim, what do you guys think?
>
> If there are just 3 files using alloca, I strongly prefer solution 3. I think
> solution 1 has already been rejected by Kim.
>
> (Also, for the record, it was me, not Julian, who expressed dislike about the
> `-Dalloca'(size)'=__builtin_alloca'(size)'` change)
https://man7.org/linux/man-pages/man3/alloca.3.html sounds like solution 2 is
the cleanest one ("standards conformance"). It is also the version with minimal
code and which will even work with future alloca usages :-)
If solution 2 has any disadvantage, I'd prefer solution 3.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1568809674
Re: RFR: 8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Mon, 15 Apr 2024 11:46:25 GMT, Magnus Ihse Bursie wrote: >> Should also remove the `#pragma alloca` in os_aix.cpp. > > It was too bad that I did not see and review this change in the makefiles. :-( > > While you guys could have gone either way, I strongly dislike the choice to > include a redefinition in the makefiles. If this really should be done, we > should introduced a new variable to carry such changes, instead of > piggybacking it with the OS defines. :-( But, I don't think it should be done > at all. > > There are several reasons why this is a inferior solution: > > 1) It does not follow prior examples. We have tried hard before not do things > like this, but rather pass flags as defines (e.g. `-DREDEFINE_ALLOCA` had > been better) > 2) It does not scale. If we start in effect allowing code in the command > line, there is no clear limit anymore what should be placed in the source > code files and what should be placed on the command line. > 3) It messes up command lines. Keeping command lines as short as reasonable > possible is a goal we try to strive for. In this case, there is also the `'` > inside them (which I don't understand why), which is just begging for > quoting/escaping problems, making command lines hard to copy/paste, send to > different systems (like logging) etc. > > I'd really like to see a follow-up PR that moves this away from the command > line define and into a source code file instead. Can we unconditionally `#include ` in all files which use `alloca`? Or does that disturb any platform? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1565770190
Re: RFR: 8330110: AIX build fails after JDK-8329704 - issue with libjli.a
On Fri, 12 Apr 2024 12:26:06 GMT, Magnus Ihse Bursie wrote: > Unfortunately, after > [JDK-8329704](https://bugs.openjdk.org/browse/JDK-8329704) AIX fails to > build. The reason is that libjli is specially treated on AIX, and built like > a static library. I tried to compensate for this (and had tested on an > earlier draft), but some late-minute changes in JDK-8329704 made this check > fail. > > At this point, I will not add any more checks for `$1_$2_STATIC_LIBRARY` in > `ResolveLibPath`. From reading the code, it become apparent that building > with `STATIC_BUILD` (n.b. -- not `STATIC_LIBS`!) will likely fail. This has > on the other hand not been tested for a long time, and is likely bit-rotten > anyway. Also, I will very soon address all of these shortcomings in the > upcoming rewrite for proper static builds. Thanks for fixing it so quickly! - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18759#pullrequestreview-1997252234
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v7]
On Wed, 10 Apr 2024 12:15:34 GMT, Joachim Kern wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about >> ill formatted printf > > Joachim Kern has updated the pull request incrementally with one additional > commit since the last revision: > > saver solution Yes, I like it too 👍 Thanks, Thomas, for your helpful feedback! - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18536#pullrequestreview-1991857617
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 10 Apr 2024 13: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 [v7]
On Tue, 9 Apr 2024 17:25:04 GMT, Stewart X Addison wrote: >> Pinging @sxa - what build environment does temurin use for AIX? > > Currently XLC16 but looking to upgrade to XLC17 on the minimum supported > level for it (So it wouldn't be SP7 at present). Thanks for the ping - we > have no current plans to increase to SP7. Seems like we need to keep it. This is unfortunate. I wouldn't risk mixing malloc and vec_malloc. Who knows what kind of problems this could cause? What happens if we try to build this code on AIX 7.2 TL5 SP7? Will the compiler complain because `malloc` is no longer defined? Should we check `defined(malloc)` in addition? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1559425371
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [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 [v2]
On Tue, 2 Apr 2024 11:22:54 GMT, Joachim Kern wrote: >> I'd prefer having less AIX specific parts in this file. Can this be moved >> somewhere else? Or maybe combine it with the AIX code above? > > My question is, do we need this block, because now already configure warns > about an outdated compiler, or is a warning to weak and we want to force this > error here? I think that building with xlc 16 is no longer possible because the old build pipeline is no longer supported and that is already caught by configure. So, can we even reach here with older xlc compilers? If not, this code can get removed. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548043503
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Thu, 28 Mar 2024 16:53:39 GMT, Joachim Kern wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about >> ill formatted printf > > src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 83: > >> 81: #error "xlc version not supported, macro __open_xl_version__ not found" >> 82: #endif >> 83: #endif // AIX > > This `#ifdef _AIX` might be obsolete, because configure will throw a warning > if the compiler has a lower version, but it's only a warning. I'd prefer having less AIX specific parts in this file. Can this be moved somewhere else? Or maybe combine it with the AIX code above? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1543371129
Re: RFR: 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. Your current version still serves the purpose of disallowing usage of xlc 16 and below. Users of the old compiler will get to know which compiler to use at minimum. Users of any Open XL compilers are not affected because "TOOLCHAIN_MINIMUM_VERSION_xlc" is not used for that AFAICS. But that's ok IMHO because the minimum clang check will be in place and the supported compilers are documented (https://wiki.openjdk.org/display/Build/Supported+Build+Platforms). So, I think this PR could get integrated and the old build pipeline get removed in a separate PR. Do you agree @JoKern65? - PR Comment: https://git.openjdk.org/jdk/pull/17857#issuecomment-1978274678
Re: RFR: 8325881: Require minimum gcc version 10
On Sat, 17 Feb 2024 08:28:56 GMT, Kim Barrett wrote: > Please review this change that updates the minimum supported version of gcc > to be used for building OpenJDK from 6.0 to 10.0. > > This permits enabling C++17 (JDK-8314488), though gcc 9.0 might suffice for > that. A minimum of gcc 10 also obtains the primitives needed to support a > work-alick for std::is_constant_evaluated (added in C++20). There are a bunch > of improvements that would be enabled by that. Having it would also allow the > elimination of a bit of a mess in the HotSpot assert macros that was needed to > work around the lack of that feature (JDK-8303805). Either current or proposed > minimum versions of other supported compilers also provide the needed > primitives. > > Testing: mach5 tier1 (uses gcc13.2 on gcc-based platforms) > Locally (linux-x64) built and ran tier1 with gcc10.3. I have no objections. We already build with gcc 11. Some machines may need to get a newer gcc, but I think that's acceptable. - PR Comment: https://git.openjdk.org/jdk/pull/17899#issuecomment-1968440090
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. LGTM and works for us. @JoKern65: We should probably remove the old build pipeline after this is integrated. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17857#pullrequestreview-1882123803
Re: RFR: 8325213: Flags introduced by configure script are not passed to ADLC build
On Mon, 5 Feb 2024 08:26:55 GMT, Martin Doerr wrote: > A trivial ADLC build fix. The ADLC_LANGSTD_CXXFLAGS should get passed. Thanks for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/17705#issuecomment-1928892061
Integrated: 8325213: Flags introduced by configure script are not passed to ADLC build
On Mon, 5 Feb 2024 08:26:55 GMT, Martin Doerr wrote: > A trivial ADLC build fix. The ADLC_LANGSTD_CXXFLAGS should get passed. This pull request has now been integrated. Changeset: 9ee9f288 Author: Martin Doerr URL: https://git.openjdk.org/jdk/commit/9ee9f288497268d64ddd48783ecb68f7e5426084 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8325213: Flags introduced by configure script are not passed to ADLC build Reviewed-by: jwaters, ihse - PR: https://git.openjdk.org/jdk/pull/17705
RFR: 8325213: Flags introduced by configure script are not passed to ADLC build
A trivial ADLC build fix. The ADLC_LANGSTD_CXXFLAGS should get passed. - Commit messages: - 8325213: Flags introduced by configure script are not passed to ADLC build Changes: https://git.openjdk.org/jdk/pull/17705/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17705&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8325213 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17705.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17705/head:pull/17705 PR: https://git.openjdk.org/jdk/pull/17705
Re: RFR: 8314488: Compile the JDK as C++17 [v5]
On Thu, 11 Jan 2024 13:04:35 GMT, Julian Waters wrote: > There is a typo in adlc: > > ``` > diff --git a/make/hotspot/gensrc/GensrcAdlc.gmk > b/make/hotspot/gensrc/GensrcAdlc.gmk > index 0898d91e1c2..bb356476847 100644 > --- a/make/hotspot/gensrc/GensrcAdlc.gmk > +++ b/make/hotspot/gensrc/GensrcAdlc.gmk > @@ -51,7 +51,7 @@ ifeq ($(call check-jvm-feature, compiler2), true) >endif > ># Set the C++ standard > - ADLC_CFLAGS += $(ADLC_LANGSTD_CXXFLAG) > + ADLC_CFLAGS += $(ADLC_LANGSTD_CXXFLAGS) > ># NOTE: The old build didn't set -DASSERT for windows but it doesn't seem > to ># hurt. > ``` Created a PR for that: https://github.com/openjdk/jdk/pull/17705 - PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1926450195
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs
On Tue, 30 Jan 2024 14:02:41 GMT, Matthias Baesken wrote: >>> Yes there is a nice define in the AIX header >> >> *sigh* On linux, they go to some lengths to avoid this, using a __REDEFINE >> mechanism. Oh well. >> >> Anyway, I think this particular can be resolved by not including the >> standard includes in the header file (which is bad practice anyway!). I'm >> currently testing this build in our CI to see that it does not break >> anything else. I'd appreciate if you could take the latest version for a >> spin, particularly a debug build... > >> I'd appreciate if you could take the latest version for a spin, particularly >> a debug build... > > Yes we pick up the latest version of the PR in a couple of hours for the > build+'lots of tests' (and this includes a fastdebug too). @MBaesken, @JoKern65: This seems to break the debug build (fast and slow) on AIX: jdk/src/java.desktop/share/native/libawt/java2d/pipe/BufferedRenderPipe.c:101:24: error: no member named 'open64' in 'SpanIteratorFuncs'; did you mean 'open'? srData = (*pFuncs->open)(env, si); ^~~~ open /usr/include/fcntl.h:115:14: note: expanded from macro 'open' #define openopen64 ^ jdk/src/java.desktop/share/native/libawt/java2d/pipe/SpanIterator.h:37:17: note: 'open' declared here void *(*open)(JNIEnv *env, jobject iterator); ^ Ah, that has already been reported above. Yeah, interesting that the normal build has passed. - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1918443702
Re: [jdk22] RFR: 8323008: filter out harmful -std* flags added by autoconf from CXX
On Sat, 13 Jan 2024 17:29:58 GMT, Christoph Langer wrote: > Hi all, > > This pull request contains a backport of > [JDK-8323008](https://bugs.openjdk.org/browse/JDK-8323008), commit > [68c42860](https://github.com/openjdk/jdk/commit/68c4286026bc2c0ec0f594e0b96fe03fe5624d6d) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Matthias Baesken on 12 Jan 2024 > and was reviewed by Erik Joelsson, Christoph Langer and Magnus Ihse Bursie. > > Thanks! I think this backport makes sense to have a cleaner build when autoconf 2.72 is used. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk22/pull/70#pullrequestreview-1821870608
Re: RFR: 8314488: Compile the JDK as C++17 [v6]
On Thu, 11 Jan 2024 13:23:45 GMT, Julian Waters wrote: >> Compile the JDK as C++17, enabling the use of all C++17 language features > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Require clang 13 in toolchain.m4 Thanks! We may switch to clang 14 on MacOS at some point of time, but it's better to have that disentangled. Some people build JDK 11 and 23 on the same machine and that is easier if they don't have to switch Xcode. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14988#pullrequestreview-1815729317
Re: RFR: 8314488: Compile the JDK as C++17 [v5]
On Thu, 11 Jan 2024 12:36:31 GMT, Martin Doerr wrote: >> Regarding >> https://github.com/TheShermanTanker/jdk/actions/runs/7070564987/job/19247370401, >> could it be that the adlc build didn't get the correct C++ version flags? >> It doesn't look like a clang 13 specific problem. > >> @TheRealMDoerr >> >> > The only issue I see is requiring clang 14.0 on MacOS is not in sync with >> > "Other JDK 22 build platforms" >> > (https://wiki.openjdk.org/display/Build/Supported+Build+Platforms). >> >> That page is suppose to document what we actually do, not be a binding >> contract; so if we change stuff, we update the page to reflect it, rather >> than the other way around. >> >> Or maybe I misunderstood your comment? > > Correct, but raising requirements requires extra effort to change the build > environments, updating docs, etc. (It may even cause incompatibilities. > Probably not in this case.) While it may be better to use a newer Xcode on > Mac, I can't see sufficient reason for forcing the whole world to build with > clang 14. > @TheRealMDoerr The adlc build is notoriously problematic, since it does not > share the common flags set for JVM or JDK native compilation. :( So your > suggestion sounds highly likely to me. Running with LOG=cmdlines will confirm > this. > > (This can be done on GHA by manually starting a run, and setting the value of > "Additional make arguments" to `LOG=cmdlines` or possibly `LOG=info,cmdlines`) Thanks for the hint! The command line is also shown here: make-support/failure-logs/hotspot_variant-server_tools_adlc_objs_adlparse.o.cmdline The -std option is not passed. That seems to be the issue. So, this is not a clang 13 vs 14 thing. - PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1887095112
Re: RFR: 8314488: Compile the JDK as C++17 [v5]
On Thu, 11 Jan 2024 11:33:07 GMT, Martin Doerr wrote: >> Julian Waters has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove unnecessary -std=c++17 option in Lib.gmk > > Regarding > https://github.com/TheShermanTanker/jdk/actions/runs/7070564987/job/19247370401, > could it be that the adlc build didn't get the correct C++ version flags? It > doesn't look like a clang 13 specific problem. > @TheRealMDoerr > > > The only issue I see is requiring clang 14.0 on MacOS is not in sync with > > "Other JDK 22 build platforms" > > (https://wiki.openjdk.org/display/Build/Supported+Build+Platforms). > > That page is suppose to document what we actually do, not be a binding > contract; so if we change stuff, we update the page to reflect it, rather > than the other way around. > > Or maybe I misunderstood your comment? Correct, but raising requirements requires extra effort to change the build environments, updating docs, etc. (It may even cause incompatibilities. Probably not in this case.) While it may be better to use a newer Xcode on Mac, I can't see sufficient reason for forcing the whole world to build with clang 14. - PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1887072454
Re: RFR: 8314488: Compile the JDK as C++17 [v5]
On Wed, 10 Jan 2024 13:11:38 GMT, Julian Waters wrote: >> Compile the JDK as C++17, enabling the use of all C++17 language features > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Remove unnecessary -std=c++17 option in Lib.gmk Regarding https://github.com/TheShermanTanker/jdk/actions/runs/7070564987/job/19247370401, could it be that the adlc build didn't get the correct C++ version flags? It doesn't look like a clang 13 specific problem. - PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1886947773
Re: RFR: 8314488: Compile the JDK as C++17 [v5]
On Wed, 10 Jan 2024 13:11:38 GMT, Julian Waters wrote: >> Compile the JDK as C++17, enabling the use of all C++17 language features > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Remove unnecessary -std=c++17 option in Lib.gmk Looks basically still good. The only issue I see is requiring clang 14.0 on MacOS is not in sync with "Other JDK 22 build platforms" (https://wiki.openjdk.org/display/Build/Supported+Build+Platforms). @MBaesken: Do you know if we can use a newer clang? - PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1884882004
Re: RFR: 8314488: Compile the JDK as C++17 [v2]
On Fri, 15 Dec 2023 08:08:10 GMT, Julian Waters wrote: >> Implementation of [JEP draft: Compile the JDK as >> C++17](https://bugs.openjdk.org/browse/JDK-8310260) > > Julian Waters has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains five additional > commits since the last revision: > > - Merge branch 'openjdk:master' into patch-7 > - Revert vm_version_linux_riscv.cpp > - vm_version_linux_riscv.cpp > - allocation.cpp > - 8310260 In case you want to update the required compiler versions as part of this PR: We have tested -TOOLCHAIN_MINIMUM_VERSION_xlc="16.1.0.0011" +TOOLCHAIN_MINIMUM_VERSION_xlc="17.1.1.4" (Already discussed with Kim.) - PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1857916995
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v7]
On Tue, 21 Nov 2023 19:30:30 GMT, suchismith1993 wrote: >> The math library in AIX specifically, is a static archive. Doing a -lm wont >> suffice, because when the symbols are looked up using dlsym or accessing >> native code through Java, it will lead to failures. >> Hence we had to come up with a list of symbols to allow math library symbols >> to be accesible. >> Also, there are parts of libc library that are static too, and hence those >> symbols also are present in this list. >> Without this change, the StdLibTest and multiple other tests which make >> native function calls using FFI, fail with NoSuchElementException. >> >> >> 1. Adding required compiler flags. >> 2. Adding required symbols. >> >> >> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799) > > suchismith1993 has updated the pull request incrementally with two additional > commits since the last revision: > > - Fix Typos > - Remove unnecessary includes Thanks for cleaning it up! LGTM. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1742925562
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v6]
On Tue, 21 Nov 2023 17:49:43 GMT, suchismith1993 wrote: >> The math library in AIX specifically, is a static archive. Doing a -lm wont >> suffice, because when the symbols are looked up using dlsym or accessing >> native code through Java, it will lead to failures. >> Hence we had to come up with a list of symbols to allow math library symbols >> to be accesible. >> Also, there are parts of libc library that are static too, and hence those >> symbols also are present in this list. >> Without this change, the StdLibTest and multiple other tests which make >> native function calls using FFI, fail with NoSuchElementException. >> >> >> 1. Adding required compiler flags. >> 2. Adding required symbols. >> >> >> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799) > > suchismith1993 has updated the pull request incrementally with two additional > commits since the last revision: > > - Comments > - Change comments src/java.base/aix/native/libsyslookup/syslookup.c line 30: > 28: #include > 29: #include > 30: #include Are string.h and stdlib.h needed? I can't see them in the comments below. src/java.base/aix/native/libsyslookup/syslookup.c line 33: > 31: #include > 32: > 33: // Addresses of functions to referenced using static linking. What does "functions to referenced" mean? Typo? - PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400959365 PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400961995
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v6]
On Tue, 21 Nov 2023 17:54:08 GMT, suchismith1993 wrote: >> src/java.base/aix/native/libsyslookup/syslookup.c line 30: >> >>> 28: #include >>> 29: #include >>> 30: #include >> >> Are string.h and stdlib.h needed? I can't see them in the comments below. > > string.h is needed for strlen. Let me check for stdlib.h by excluding it. ah, a comment above strlen would help. - PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400966451
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v5]
On Tue, 21 Nov 2023 13:01:50 GMT, suchismith1993 wrote:
>> The math library in AIX specifically, is a static archive. Doing a -lm wont
>> suffice, because when the symbols are looked up using dlsym or accessing
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make
>> native function calls using FFI, fail with NoSuchElementException.
>>
>>
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>>
>>
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Update Copyright
> - Revert lookup file in share directory.
>Add lookup file for AIX specific implementation.
src/java.base/aix/native/libsyslookup/syslookup.c line 209:
> 207: char* syslookup() {
> 208: return "syslookup";
> 209: }
This is probably not needed, either (see Windows version).
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400629119
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v5]
On Tue, 21 Nov 2023 13:01:50 GMT, suchismith1993 wrote: >> The math library in AIX specifically, is a static archive. Doing a -lm wont >> suffice, because when the symbols are looked up using dlsym or accessing >> native code through Java, it will lead to failures. >> Hence we had to come up with a list of symbols to allow math library symbols >> to be accesible. >> Also, there are parts of libc library that are static too, and hence those >> symbols also are present in this list. >> Without this change, the StdLibTest and multiple other tests which make >> native function calls using FFI, fail with NoSuchElementException. >> >> >> 1. Adding required compiler flags. >> 2. Adding required symbols. >> >> >> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799) > > suchismith1993 has updated the pull request incrementally with two additional > commits since the last revision: > > - Update Copyright > - Revert lookup file in share directory. >Add lookup file for AIX specific implementation. Thanks! This looks good, now. One question: Are all the `#include`s needed? - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1741972916
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v3]
On Tue, 21 Nov 2023 11:52:23 GMT, suchismith1993 wrote: >> The math library in AIX specifically, is a static archive. Doing a -lm wont >> suffice, because when the symbols are looked up using dlsym or accessing >> native code through Java, it will lead to failures. >> Hence we had to come up with a list of symbols to allow math library symbols >> to be accesible. >> Also, there are parts of libc library that are static too, and hence those >> symbols also are present in this list. >> Without this change, the StdLibTest and multiple other tests which make >> native function calls using FFI, fail with NoSuchElementException. >> >> >> 1. Adding required compiler flags. >> 2. Adding required symbols. >> >> >> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799) > > suchismith1993 has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains four commits: > > - Remove symbols file after using inline way. > - Provide support for math library in inline way. > - Update symbols-aix-foreign > >Remove comments from export list, causes build failures. > - Symbol Resolution fix for Panama changes. >1.Adding required compiler flags. >2. Adding required symbols. Nice! This solution looks better. But, please don't modify the shared version of syslookup.c. You can create one for AIX here: src/java.base/aix/native/libsyslookup - Changes requested by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1741796785
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v2]
On Tue, 21 Nov 2023 11:21:40 GMT, suchismith1993 wrote: >> The math library in AIX specifically, is a static archive. Doing a -lm wont >> suffice, because when the symbols are looked up using dlsym or accessing >> native code through Java, it will lead to failures. >> Hence we had to come up with a list of symbols to allow math library symbols >> to be accesible. >> Also, there are parts of libc library that are static too, and hence those >> symbols also are present in this list. >> Without this change, the StdLibTest and multiple other tests which make >> native function calls using FFI, fail with NoSuchElementException. >> >> >> 1. Adding required compiler flags. >> 2. Adding required symbols. >> >> >> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799) > > suchismith1993 has updated the pull request incrementally with 172 additional > commits since the last revision: > > - Provide support for math library in inline way. > - Symbol Resolution fix for Panama changes. >1. Adding required compiler flags. >2. Adding required symbols. > - 8320348: test/jdk/java/io/File/GetAbsolutePath.windowsDriveRelative fails > if working directory is not on drive C > >Reviewed-by: alanb, mbaesken > - 8320372: test/jdk/sun/security/x509/DNSName/LeadingPeriod.java validity > check failed > >Reviewed-by: alanb > - 8320234: Merge doclint.Env.AccessKind with tool.AccessKind > >Reviewed-by: jjg > - 8319817: Charset constructor should make defensive copy of aliases > >Reviewed-by: rriggs, alanb, bpb, iris, jpai > - 8320147: Remove DumpSharedSpaces > >Reviewed-by: ccheung, matsaave > - 8319973: AArch64: Save and restore FPCR in the call stub > >Reviewed-by: adinn, stuefe > - 8320410: Reflow markdown in building.md > >Reviewed-by: erikj > - 8319928: Exceptions thrown by cleanup actions should be handled correctly > >Reviewed-by: jvernee > - ... and 162 more: https://git.openjdk.org/jdk/compare/c340abf0...9c77fa98 It's messed up, now. Can you revert the last step? - Changes requested by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1741741189
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols
On Fri, 17 Nov 2023 12:45:32 GMT, suchismith1993 wrote: > And I still don't understand if this is the list of symbols that are required > by our tests, or the complete list of symbols that FFI `defaultLookup` > returns to user applications. If it is the latter, then this is sort-of okay > as a final solution, but it is the former, I'm stll frowning upon this > solution. I have tried creating own .so libraries and the symbol lookup has worked just like on linux. So, it seems like we need workarounds only for special AIX stuff. My understanding is that a lot of symbols can be found out of the box because they are in dynamically linked libraries. Only some libraries are special on AIX which don't support dynamic linking and we need this workaround for them. @suchismith1993: Is this correct? - PR Comment: https://git.openjdk.org/jdk/pull/16414#issuecomment-1816384750
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols
On Mon, 13 Nov 2023 11:47:52 GMT, suchismith1993 wrote: >>> There is not generic way of generating this. It needs a manual intervention >>> and symbols are to be added on a need basis in future. Looks like a list to >>> be maintained. I had tried adding comments to explain the list, but that >>> causes build failures. >> >> Would it be possible to paste the summary of the "instructions" to generate >> this? My initial reaction when seeing this PR is to wonder why it can't be >> generated at build time but from the discussion, it seems like it's a subset >> symbols, just the functions used by libjvm or is it all the native libraries? > >> > There is not generic way of generating this. It needs a manual >> > intervention and symbols are to be added on a need basis in future. Looks >> > like a list to be maintained. I had tried adding comments to explain the >> > list, but that causes build failures. >> >> Would it be possible to paste the summary of the "instructions" to generate >> this? My initial reaction when seeing this PR is to wonder why it can't be >> generated at build time but from the discussion, it seems like it's a subset >> symbols, just the functions used by libjvm or is it all the native libraries? > > I just did a nm command and did a grep of " T " . That way i got all the > symbols for math libraries. There were additional parsing instructions to > remove the first column and remove periods at beginning. > > Then we had to remove certain symbols that are dependent on DFP symbols and > xlc17 doesn't support them yet, So we had to trim down the list further, > which is why it now becomes a list to be maintained. @suchismith1993: Can you add your explanations to the description at the top of this PR, please? I think restricting symbols to the ones from C standard makes sense. @magicus: We need a solution for JDK22 since the FFI including symbol lookup has become a required interface. RDP1 is coming closer and I don't see a better solution coming soon enough. Can we go ahead with a symbol list file for JDK22? Where should it reside if not in make/data/hotspot-symbols? - PR Comment: https://git.openjdk.org/jdk/pull/16414#issuecomment-1816276842
Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols
On Mon, 30 Oct 2023 10:54:48 GMT, suchismith1993 wrote: > 1. Adding required compiler flags. > 2. Adding required symbols. > > JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799) Changes requested by mdoerr (Reviewer). LGTM. You may want to replace the Copyright header of the new file. It was contributed by IBM. Still good. I suggest to remove the empty lines at the beginning. make/modules/java.base/Lib.gmk line 216: > 214: LDFLAGS := $(LDFLAGS_JDKLIB), \ > 215: LDFLAGS_linux := -Wl$(COMMA)--no-as-needed, \ > 216: LDFLAGS_aix := -brtl -bloadmap:/home/hotspot/openjdk/symbol.log > -bE:/home/hotspot/tmp/all-libs/1.exp , \ These files need to get added somewhere. Maybe symbols could get added to make/data/hotspot-symbols/symbols-aix? Or to new files in the same directory (with aix in the file names)? Also, please remove extra whitespaces. make/modules/java.base/Lib.gmk line 256: > 254: endif > 255: > 256: > I guess this was done by mistake. Please revert. src/java.base/share/native/libsyslookup/syslookup.c line 37: > 35: > 36: > 37: Please avoid changing lines which you don't need to modify! Is `#include ` really needed, here? If so, please protect it by `#ifdef _AIX` and add a comment explaining why. - PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1705916269 Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1716162496 PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1720838225 PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1377348834 PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1377352267 PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1377355378
Re: RFR: 8314488: Compile the JDK as C++17
On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters wrote: > Implementation of [JEP draft: Compile the JDK as > C++17](https://bugs.openjdk.org/browse/JDK-8310260) We're not changing it for existing releases. I don't think non-LTS releases play a significant role regarding such compatibility. Next LTS is supposed to be JDK 25 (2025-09-16, https://www.java.com/releases/). - PR Comment: https://git.openjdk.org/jdk/pull/14988#issuecomment-1742099242
Re: RFR: 8312522: Implementation of Foreign Function & Memory API [v11]
On Wed, 6 Sep 2023 10:04:35 GMT, Jorn Vernee wrote:
>> This patch contains the implementation of the foreign linker & memory API
>> JEP for Java 22. The initial patch is composed of commits brought over
>> directly from the [panama-foreign
>> repo](https://github.com/openjdk/panama-foreign). The main changes found in
>> this patch come from the following PRs:
>>
>> 1. https://github.com/openjdk/panama-foreign/pull/836 Where previous
>> iterations supported converting Java strings to and from native strings in
>> the UTF-8 encoding, we've extended the supported encoding to all the
>> encodings found in the `java.nio.charset.StandardCharsets` class.
>> 2. https://github.com/openjdk/panama-foreign/pull/838 We dropped the
>> `MemoryLayout::sequenceLayout` factory method which inferred the size of the
>> sequence to be `Long.MAX_VALUE`, as this led to confusion among clients. A
>> client is now required to explicitly specify the sequence size.
>> 3. https://github.com/openjdk/panama-foreign/pull/839 A new API was added:
>> `Linker::canonicalLayouts`, which exposes a map containing the
>> platform-specific mappings of common C type names to memory layouts.
>> 4. https://github.com/openjdk/panama-foreign/pull/840 Memory access
>> varhandles, as well as byte offset and slice handles derived from memory
>> layouts, now feature an additional 'base offset' coordinate that is added to
>> the offset computed by the handle. This allows composing these handles with
>> other offset computation strategies that may not be based on the same memory
>> layout. This addresses use-cases where clients are working with 'dynamic'
>> layouts, whose size might not be known statically, such as variable length
>> arrays, or variable size matrices.
>> 5. https://github.com/openjdk/panama-foreign/pull/841 Remove this now
>> redundant API. Clients can simply use the difference between the base
>> address of two memory segments.
>> 6. https://github.com/openjdk/panama-foreign/pull/845 Disambiguate uses of
>> `SegmentAllocator::allocateArray`, by renaming methods that both allocate +
>> initialize memory segments to `allocateFrom`. (see the original PR for the
>> problematic case)
>> 7. https://github.com/openjdk/panama-foreign/pull/846 Improve the
>> documentation for variadic functions.
>> 8. https://github.com/openjdk/panama-foreign/pull/849 Several test fixes to
>> make sure the `jdk_foreign` tests can pass on 32-bit machines, taking
>> linux-x86 as a test bed.
>> 9. https://github.com/openjdk/panama-foreign/pull/850 Make the linker API
>> required. The `Linker::nativeLinker` method is not longer allowed to throw
>> an `UnsupportedO...
>
> Jorn Vernee has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix typo in doc
>
> Co-authored-by: Paul Sandoz
Please adapt the new `LinuxPPC64Linker` when merging:
diff --git
a/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/linux/LinuxPPC64Linker.java
b/src/java.base/share/classes/jdk/inter
nal/foreign/abi/ppc64/linux/LinuxPPC64Linker.java
index 150687d4078..7cf2d524bff 100644
---
a/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/linux/LinuxPPC64Linker.java
+++
b/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/linux/LinuxPPC64Linker.java
@@ -27,15 +27,22 @@
import jdk.internal.foreign.abi.AbstractLinker;
import jdk.internal.foreign.abi.LinkerOptions;
+import jdk.internal.foreign.abi.SharedUtils;
import jdk.internal.foreign.abi.ppc64.CallArranger;
import java.lang.foreign.FunctionDescriptor;
+import java.lang.foreign.MemoryLayout;
+import java.lang.foreign.ValueLayout;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodType;
import java.nio.ByteOrder;
+import java.util.Map;
public final class LinuxPPC64Linker extends AbstractLinker {
+static final Map CANONICAL_LAYOUTS =
+SharedUtils.canonicalLayouts(ValueLayout.JAVA_LONG,
ValueLayout.JAVA_LONG, ValueLayout.JAVA_INT);
+
public static LinuxPPC64Linker getInstance() {
final class Holder {
private static final LinuxPPC64Linker INSTANCE = new
LinuxPPC64Linker();
@@ -62,4 +69,9 @@ protected UpcallStubFactory arrangeUpcall(MethodType
targetType, FunctionDescrip
protected ByteOrder linkerByteOrder() {
return ByteOrder.BIG_ENDIAN;
}
+
+@Override
+public Map canonicalLayouts() {
+return CANONICAL_LAYOUTS;
+}
}
The `test/jdk/java/foreign` tests have passed on both, linux PPC64 and PPC64le.
-
PR Comment: https://git.openjdk.org/jdk/pull/15103#issuecomment-1708107829
Re: RFR: JDK-8313764: Offer JVM HS functionality to shared lib load operations done by the JDK codebase
On Mon, 14 Aug 2023 07:48:00 GMT, Matthias Baesken wrote: > Currently there is a number of functionality that would be interesting to > have for shared lib load operations in the JDK C code. > Some examples : > Events::log_dll_message for hs-err files reporting > JFR event NativeLibraryLoad > There is the need to update the shared lib Cache on AIX ( see > LoadedLibraries::reload() , see also > https://bugs.openjdk.org/browse/JDK-8314152 ), > this is currently not fully in sync with libs loaded form jdk c-libs and > sometimes reports outdated information > > Offer an interface (e.g. jvm.cpp) to support this. Please check windows-aarch64 build error: unresolved external symbol dlopen_ext - PR Comment: https://git.openjdk.org/jdk/pull/15264#issuecomment-1689871700
Re: RFR: 8314488: Compile the JDK as C++17
On Mon, 24 Jul 2023 01:41:16 GMT, Julian Waters wrote: > Implementation of [JEP draft: Compile the JDK as > C++17](https://bugs.openjdk.org/browse/JDK-8310260) Successfully tested on AIX with xlc 17.1.1. Works for us (SAP). Please check with others who might still use an older compiler (IBM). - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14988#pullrequestreview-1586571119
Re: RFR: 8309934: Update GitHub Actions to use JDK 17 for building jtreg
On Wed, 14 Jun 2023 10:07:01 GMT, Christian Stein wrote: > Hi all, > > This pull request contains a backport of commit > [8aad881e](https://github.com/openjdk/jdk/commit/8aad881e803fddc26f45270f779ff0c0e5a095d8) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Christian Stein on 13 Jun 2023 > and was reviewed by Erik Joelsson. > > Thanks! Yes. You can post the link to the results, here. - PR Comment: https://git.openjdk.org/jdk21/pull/18#issuecomment-1591273763
Re: RFR: 8309934: Update GitHub Actions to use JDK 17 for building jtreg
On Wed, 14 Jun 2023 10:07:01 GMT, Christian Stein wrote: > Hi all, > > This pull request contains a backport of commit > [8aad881e](https://github.com/openjdk/jdk/commit/8aad881e803fddc26f45270f779ff0c0e5a095d8) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Christian Stein on 13 Jun 2023 > and was reviewed by Erik Joelsson. > > Thanks! Thanks for backporting! Could you enable GitHub Actions and run the tests, please? - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk21/pull/18#pullrequestreview-1479333689
Re: RFR: JDK-8309219: Fix xlc17 clang 15 warnings in java.base [v3]
On Wed, 7 Jun 2023 11:23:26 GMT, JoKern65 wrote:
>> This pr is a split off from JDK-8308288: Fix xlc17 clang warnings in shared
>> code https://github.com/openjdk/jdk/pull/14146
>> It handles the part in java.base.
>>
>> Compiling on AIX with xlc17 which contains the new clang 15 frontend
>> shows the following warnings.
>>
>> built by make/modules/java.base/Lib.gmk
>>
>> src/java.base/unix/native/libnet/DefaultProxySelector.c:378:41:22: error:
>> passing arguments to a function without a prototype is deprecated in all
>> versions of C and is not supported in C2x
>> [-Werror,-Wdeprecated-non-prototype]
>> proxies = (*g_proxy_resolver_lookup)(resolver, uri, NULL, &error);
>> ^
>>
>>
>> src/java.base/unix/native/libnet/NetworkInterface.c:1612:24: error:
>> arithmetic on a pointer to void is a GNU extension
>> [-Werror,-Wgnu-pointer-arith]
>> end = (void *)nddp + size;
>>
>> src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c:1251:7: error:
>> '_ALLBSD_SOURCE' is not defined, evaluates to 0 [-Werror,-Wundef]
>> #elif _ALLBSD_SOURCE
>> The whole file checks whether _ALLBSD_SOURCE is defined, only where #elif is
>> used the call to defined() was forgotten.
>>
>> built by make/modules/java.base/lib/CoreLibraries.gmk
>>
>> src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:638 comparison of
>> integers of different signs: 'int' and 'unsigned long'
>> if (ret < sizeof(psinfo_t)) {
>> ProcessHandleImpl_unix.c: Just using the proper type `size_t` instead of
>> `int`.
>>
>>
>> src/java.base/aix/native/libjli/java_md_aix.c:42:38: error: arithmetic on a
>> pointer to void is a GNU extension [-Werror,-Wgnu-pointer-arith]
>> addr < p->ldinfo_textorg + p->ldinfo_textsize) {
>> we can fix this with temporary casting the `void*` pointer to `char*` or by
>> disabling the warning in CoreLibraries.gmk
>>
>> src/java.base/share/native/libzip/zlib/inffast.c:74:20: error: a function
>> definition without a prototype is deprecated in all versions of C and is not
>> supported in C2x [-Werror,-Wdeprecated-non-prototype]
>> void ZLIB_INTERNAL inflate_fast(strm, start)
>> ^
>> src/java.base/share/native/libjli/java.c:2311:22: error: format string is
>> not a string literal [-Werror,-Wformat-nonliteral]
>> vfprintf(stderr, fmt, vl);
>> ^~~
>>
>> The test library libGetXSpace.c also shows a warning.
>> In addition, the cast in that lib is wrong, we opened
>> "[JDK-8309216](https://bugs.openjdk.org/browse/JDK-8309216) cast from jchar*
>> to char* in test GetXSpace.java" for that.
>
> JoKern65 has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update libGetXSpace.c
>
> cast not necessary
Marked as reviewed by mdoerr (Reviewer).
-
PR Review: https://git.openjdk.org/jdk/pull/14281#pullrequestreview-1467571356
Re: RFR: JDK-8309219: Fix xlc17 clang 15 warnings in java.base [v2]
On Wed, 7 Jun 2023 09:03:27 GMT, JoKern65 wrote:
>> This pr is a split off from JDK-8308288: Fix xlc17 clang warnings in shared
>> code https://github.com/openjdk/jdk/pull/14146
>> It handles the part in java.base.
>>
>> Compiling on AIX with xlc17 which contains the new clang 15 frontend
>> shows the following warnings.
>>
>> built by make/modules/java.base/Lib.gmk
>>
>> src/java.base/unix/native/libnet/DefaultProxySelector.c:378:41:22: error:
>> passing arguments to a function without a prototype is deprecated in all
>> versions of C and is not supported in C2x
>> [-Werror,-Wdeprecated-non-prototype]
>> proxies = (*g_proxy_resolver_lookup)(resolver, uri, NULL, &error);
>> ^
>>
>>
>> src/java.base/unix/native/libnet/NetworkInterface.c:1612:24: error:
>> arithmetic on a pointer to void is a GNU extension
>> [-Werror,-Wgnu-pointer-arith]
>> end = (void *)nddp + size;
>>
>> src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c:1251:7: error:
>> '_ALLBSD_SOURCE' is not defined, evaluates to 0 [-Werror,-Wundef]
>> #elif _ALLBSD_SOURCE
>> The whole file checks whether _ALLBSD_SOURCE is defined, only where #elif is
>> used the call to defined() was forgotten.
>>
>> built by make/modules/java.base/lib/CoreLibraries.gmk
>>
>> src/java.base/unix/native/libjava/ProcessHandleImpl_unix.c:638 comparison of
>> integers of different signs: 'int' and 'unsigned long'
>> if (ret < sizeof(psinfo_t)) {
>> ProcessHandleImpl_unix.c: Just using the proper type `size_t` instead of
>> `int`.
>>
>>
>> src/java.base/aix/native/libjli/java_md_aix.c:42:38: error: arithmetic on a
>> pointer to void is a GNU extension [-Werror,-Wgnu-pointer-arith]
>> addr < p->ldinfo_textorg + p->ldinfo_textsize) {
>> we can fix this with temporary casting the `void*` pointer to `char*` or by
>> disabling the warning in CoreLibraries.gmk
>>
>> src/java.base/share/native/libzip/zlib/inffast.c:74:20: error: a function
>> definition without a prototype is deprecated in all versions of C and is not
>> supported in C2x [-Werror,-Wdeprecated-non-prototype]
>> void ZLIB_INTERNAL inflate_fast(strm, start)
>> ^
>> src/java.base/share/native/libjli/java.c:2311:22: error: format string is
>> not a string literal [-Werror,-Wformat-nonliteral]
>> vfprintf(stderr, fmt, vl);
>> ^~~
>>
>> The test library libGetXSpace.c also shows a warning.
>> In addition, the cast in that lib is wrong, we opened
>> "[JDK-8309216](https://bugs.openjdk.org/browse/JDK-8309216) cast from jchar*
>> to char* in test GetXSpace.java" for that.
>
> JoKern65 has updated the pull request with a new target base due to a merge
> or a rebase. The pull request now contains two commits:
>
> - Merge branch 'master' into JDK-8309219
> - JDK-8309219
LGTM.
-
Marked as reviewed by mdoerr (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14281#pullrequestreview-1467159943
Re: RFR: JDK-8309225: Fix xlc17 clang 15 warnings in security and servicability
On Fri, 2 Jun 2023 10:19:53 GMT, JoKern65 wrote:
> This pr is a split off from JDK-8308288: Fix xlc17 clang warnings in shared
> code https://github.com/openjdk/jdk/pull/14146
> It handles the part in security and servicability.
>
> Compiling on AIX with xlc17 which contains the new clang 15 frontend shows
> the following warnings:
>
> src/java.security.jgss/share/native/libj2gss/NativeUtil.h:30:
> src/java.security.jgss/share/native/libj2gss/gssapi.h:48:5: error:
> 'TARGET_OS_MAC' is not defined, evaluates to 0 [-Werror,-Wundef]
> #if TARGET_OS_MAC && (defined(ppc) || defined(ppc64) || defined(i386) ||
> defined(x86_64))
> ^
> TARGET_OS_MAC is not defined. Instead of disabling the warning, I could
> ` #ifndef TARGET_OS_MAC`
> `#define TARGET_OS_MAC=0`
> `#endif`
> But this is already handled by disabling the warning for gcc.
>
> src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c:718:33: error:
> suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
> struct in6_addr mappedAny = IN6ADDR_ANY_INIT;
> ^~~~
> /usr/include/netinet/in.h:454:32: note: expanded from macro 'IN6ADDR_ANY_INIT'
> #define IN6ADDR_ANY_INIT {0, 0, 0, 0}
LGTM.
-
Marked as reviewed by mdoerr (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14282#pullrequestreview-1465140605
Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v4]
On Fri, 2 Jun 2023 18:24:16 GMT, Y. Srinivas Ramakrishna wrote: >> Kelvin Nilsen has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Force PLAB sizes to align on card-table size > > src/hotspot/cpu/riscv/gc/shenandoah/c1/shenandoahBarrierSetC1_riscv.cpp line > 4: > >> 2: * Copyright (c) 2018, 2019, Red Hat, Inc. All rights reserved. >> 3: * Copyright (c) 2020, 2021, Huawei Technologies Co., Ltd. All rights >> reserved. >> 4: * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. > > This should be backed out, since it seems that there is no (other) change to > this fie. Yes. And also from files which were changed by non-Amazon employees only, please. - PR Review Comment: https://git.openjdk.org/jdk/pull/14185#discussion_r1214693296
Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental) [v4]
On Fri, 2 Jun 2023 02:49:25 GMT, Kelvin Nilsen wrote: >> OpenJDK Colleagues: >> >> Please review this proposed integration of Generational mode for Shenandoah >> GC under https://bugs.openjdk.org/browse/JDK-8307314. >> >> Generational mode of Shenandoah is enabled by adding >> `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a >> command line that already specifies ` -XX:+UseShenandoahGC`. The >> implementation automatically adjusts the sizes of old generation and young >> generation to efficiently utilize the entire heap capacity. Generational >> mode of Shenandoah resembles G1 in the following regards: >> >> 1. Old-generation marking runs concurrently during the time that multiple >> young generation collections run to completion. >> 2. After old-generation marking completes, we perform a sequence of mixed >> collections. Each mixed collection combines collection of young generation >> with evacuation of a portion of the old-generation regions identified for >> collection based on old-generation marking information. >> 3. Unlike G1, young-generation collections and evacuations are entirely >> concurrent, as with single-generation Shenandoah. >> 4. As with single-generation Shenandoah, there is no explicit notion of eden >> and survivor space within the young generation. In practice, regions that >> were most recently allocated tend to have large amounts of garbage and these >> regions tend to be collected with very little effort. Young-generation >> objects that survive garbage collection tend to accumulate in regions that >> hold survivor objects. These regions tend to have smaller amounts of >> garbage, and are less likely to be collected. If they survive a sufficient >> number of young-generation collections, the “survivor” regions are promoted >> into the old generation. >> >> We expect to refine heuristics as we gain experience with more production >> workloads. In the future, we plan to remove the “experimental” qualifier >> from generational mode, at which time we expect that generational mode will >> become the default mode for Shenandoah. >> >> **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, >> gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, >> HyperAlloc, and multiple AWS production workload simulators. We test on >> Linux x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and >> Windows x64. > > Kelvin Nilsen has updated the pull request incrementally with one additional > commit since the last revision: > > Force PLAB sizes to align on card-table size > # Internal Error (shenandoahVerifier.cpp:1244), pid=2951116, tid=2951124 > # Error: Verify init-mark remembered set violation; clean card should be > dirty I've seen the same issue on PPC64: https://bugs.openjdk.org/browse/JDK-8309371 - PR Comment: https://git.openjdk.org/jdk/pull/14185#issuecomment-1574125489
Re: RFR: JDK-8307314: Implementation: Generational Shenandoah (Experimental)
On Fri, 26 May 2023 20:46:29 GMT, Kelvin Nilsen wrote: > OpenJDK Colleagues: > > Please review this proposed integration of Generational mode for Shenandoah > GC under https://bugs.openjdk.org/browse/JDK-8307314. > > Generational mode of Shenandoah is enabled by adding > `-XX:+UnlockExperimentalVMOptions -XX:ShenandoahGCMode=generational` to a > command line that already specifies ` -XX:+UseShenandoahGC`. The > implementation automatically adjusts the sizes of old generation and young > generation to efficiently utilize the entire heap capacity. Generational > mode of Shenandoah resembles G1 in the following regards: > > 1. Old-generation marking runs concurrently during the time that multiple > young generation collections run to completion. > 2. After old-generation marking completes, we perform a sequence of mixed > collections. Each mixed collection combines collection of young generation > with evacuation of a portion of the old-generation regions identified for > collection based on old-generation marking information. > 3. Unlike G1, young-generation collections and evacuations are entirely > concurrent, as with single-generation Shenandoah. > 4. As with single-generation Shenandoah, there is no explicit notion of eden > and survivor space within the young generation. In practice, regions that > were most recently allocated tend to have large amounts of garbage and these > regions tend to be collected with very little effort. Young-generation > objects that survive garbage collection tend to accumulate in regions that > hold survivor objects. These regions tend to have smaller amounts of > garbage, and are less likely to be collected. If they survive a sufficient > number of young-generation collections, the “survivor” regions are promoted > into the old generation. > > We expect to refine heuristics as we gain experience with more production > workloads. In the future, we plan to remove the “experimental” qualifier > from generational mode, at which time we expect that generational mode will > become the default mode for Shenandoah. > > **Testing**: We continuously run jtreg tiers 1-4 + hotspot_gc_shenandoah, > gcstress, jck compiler, jck runtime, Dacapo, SpecJBB, SpecVM, Extremem, > HyperAlloc, and multiple AWS production workload simulators. We test on Linux > x64 and aarch64, Alpine x64 and aarch64, macOS x64 and aarch64, and Windows > x64. Issues already reported to GenShen engineers: gc/shenandoah/TestElasticTLAB.java#generational # Internal Error (src\hotspot\share\gc\shenandoah\shenandoahFreeSet.cpp:695), pid=23288, tid=23784 # assert(size % CardTable::card_size_in_words() == 0) failed: size must be multiple of card table size, was 258 gc/stress/gcold/TestGCOldWithShenandoah.java#generational # Internal Error (src\hotspot\share\gc\shenandoah\heuristics\shenandoahOldHeuristics.cpp:82), pid=20828, tid=5836 # assert(_old_generation->available() > old_evacuation_budget) failed: Cannot budget more than is available gc/shenandoah/oom/TestAllocOutOfMemory.java#large Execution failed: `main' threw exception: java.lang.RuntimeException: 'java.lang.OutOfMemoryError: Java heap space' missing from stdout/stderr (Issue with 64k Pages) gc/shenandoah/TestRetainObjects.java#no-tlab gc/shenandoah/TestSieveObjects.java#no-tlab Timeouts. gc/shenandoah/TestAllocObjects.java#generational gc/shenandoah/TestDynamicSoftMaxHeapSize.java#generational # Internal Error src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp:664), pid=18434, tid=29955 # assert(is_global() || ShenandoahHeap::heap()->is_full_gc_in_progress() || (_used + _humongous_waste <= _affiliated_region_count * ShenandoahHeapRegion::region_size_bytes())) failed: used cannot exceed regions - PR Comment: https://git.openjdk.org/jdk/pull/14185#issuecomment-1571947174
Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code [v2]
On Fri, 26 May 2023 16:58:41 GMT, Thomas Stuefe wrote: >> The crazy thing is that `malloc` is defined! That means all places where we >> use the term malloc are getting replaced without such a workaround. (E.g. >> for log tags.) > > So, we do this only for malloc? Not for calloc, posix_memalign, realloc etc? > What about free? > > As ugly as defining malloc is (and I remember QADRT), I hesitate about > removing that define. > > Removing that define and hard-coding it here assumes 1) our replacement is > equivalent (ok, easy to check) 2) it will always be equivalent in future AIX > versions 3) pointers it returns work with the unchanged free() and realloc() > the system provides, and will always do so. > > I don't know... I would not do this just to get rid of a warning. This one is not just to get rid of a warning. We get real test errors because malloc gets replaced by vec_malloc in log tags. - PR Review Comment: https://git.openjdk.org/jdk/pull/14146#discussion_r1207308708
Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code [v2]
On Fri, 26 May 2023 08:31:46 GMT, JoKern65 wrote: >> When using the new xlc17 compiler (based on a recent clang) to build OpenJDk >> on AIX , we run into various "warnings as errors". >> Some of those are in shared codebase and could be addressed by small >> adjustments. >> A lot of those changes are in hotspot, some might be somewhere else in the >> OpenJDK C/C++ code. > > JoKern65 has updated the pull request incrementally with one additional > commit since the last revision: > > forgotton _ Just a side note: The warning elimination is new for AIX compilers. We had given it up at earlier times: https://bugs.openjdk.org/browse/JDK-8202325 I still appreciate if we can get warning free and it makes sense to review all of them. But, I wouldn't require it for JDK21. - PR Comment: https://git.openjdk.org/jdk/pull/14146#issuecomment-1564406986
Re: RFR: JDK-8308288: Fix xlc17 clang warnings in shared code
On Thu, 25 May 2023 15:04:32 GMT, Kim Barrett wrote:
>> When using the new xlc17 compiler (based on a recent clang) to build OpenJDk
>> on AIX , we run into various "warnings as errors".
>> Some of those are in shared codebase and could be addressed by small
>> adjustments.
>> A lot of those changes are in hotspot, some might be somewhere else in the
>> OpenJDK C/C++ code.
>
> src/hotspot/share/utilities/globalDefinitions_xlc.hpp line 47:
>
>> 45: #undef malloc
>> 46: extern void *malloc(size_t) asm("vec_malloc");
>> 47: #endif
>
> Wow! And I don't mean that in a good way. I'm not questioning whether this
> is correct, just commenting
> on how crazy it seems that such a thing might be needed.
The crazy thing is that `malloc` is defined! That means all places where we use
the term malloc are getting replaced without such a workaround. (E.g. for log
tags.)
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14146#discussion_r1205867287
Re: RFR: JDK-8306304: Fix xlc17 clang warnings in ppc and aix code [v2]
On Fri, 12 May 2023 21:51:59 GMT, Kim Barrett wrote:
>> src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp line 426:
>>
>>> 424: // Missing test if instr is commutative and if we should swap.
>>> 425: if (right.value()->type()->as_LongConstant() &&
>>> 426: (x->op() == Bytecodes::_lsub &&
>>> right.value()->type()->as_LongConstant()->value() == -32768 ) ) {
>>
>> I would prefer a shifted value here as it's usually more readable. If the
>> compiler is being stubborn in its warnings, a comment explaining the magic
>> value would be fine too.
>
> What is the warning here? Note that we've already turned off
> `-Wshift-negative-value` for gcc and xlc
> (but not for clang, for some reason). See `# Disabled warnings` in
> CompileJvm.gmk.
I think disabling the warning is fine. Alternatively, we could `#define
MIN_INT16 -32768` somewhere or introduce `const int16_t min_int16 = (int16_t)1
<< (sizeof(int16_t)*BitsPerByte-1);`. What do you prefer, Kim?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/13953#discussion_r1193762594
Re: RFR: JDK-8306304: Fix xlc17 clang warnings in ppc and aix code [v2]
On Fri, 12 May 2023 16:16:01 GMT, JoKern65 wrote: >> When using the new xlc17 compiler (based on a recent clang) to build OpenJDk >> on AIX , we run into various "warnings as errors". >> Many of those are in the aix or ppc specific codebase and could be addressed >> by small adjustments. >> A lot of those changes are in hotspot, some might be somewhere else in the >> OpenJDK C/C++ code. >> With this PR we address only the platform dependent code changes. > > JoKern65 has updated the pull request incrementally with one additional > commit since the last revision: > > cosmetic changes Thanks for addressing all the warnings! Looks basically good to me. Some details need to get checked. src/hotspot/os/aix/os_aix.cpp line 677: > 675: #ifdef AIX_XLC_GE_17 > 676: #include "alloca.h" > 677: #endif Includes should better be at the beginning of the file. - PR Review: https://git.openjdk.org/jdk/pull/13953#pullrequestreview-1426444262 PR Review Comment: https://git.openjdk.org/jdk/pull/13953#discussion_r1193770194
Re: RFR: JDK-8307604: gcc12 based Alpine build broken build after JDK-8307301
On Mon, 8 May 2023 10:29:56 GMT, Matthias Baesken wrote:
> After the harfbuzz 7.2 update we run into
>
> /linuxmuslx86_64/jdk-dev/src/java.desktop/share/native/libharfbuzz/OT/glyf/Glyph.hh:281:8:
> error: offset '4' outside bounds of constant string [-Werror=array-bounds]
> 281 | bool get_points (hb_font_t *font, const accelerator_t
> &glyf_accelerator,
> | ^~
> /linuxmuslx86_64/jdk-dev/src/java.desktop/share/native/libharfbuzz/hb-static.cc:45:16:
> note: '_hb_NullPool' declared here
>45 | uint64_t const _hb_NullPool[(HB_NULL_POOL_SIZE + sizeof (uint64_t) -
> 1) / sizeof (uint64_t)] = {};
> | ^~~~
> /linuxmuslx86_64/jdk-dev/src/java.desktop/share/native/libharfbuzz/OT/glyf/Glyph.hh:281:8:
> error: offset '4' outside bounds of constant string [-Werror=array-bounds]
> 281 | bool get_points (hb_font_t *font, const accelerator_t
> &glyf_accelerator,
> | ^~
> /linuxmuslx86_64/jdk-dev/src/java.desktop/share/native/libharfbuzz/hb-static.cc:45:16:
> note: '_hb_NullPool' declared here
>45 | uint64_t const _hb_NullPool[(HB_NULL_POOL_SIZE + sizeof (uint64_t) -
> 1) / sizeof (uint64_t)] = {};
>
> We use gcc12 on Alpine.
> Switching off this 'warning as error' fixes the issue.
It's always unfortunate if we have to disable warnings, but I'm not aware of a
better solution. Changing the harfbuzz code should be done upstream (in the
harfbuzz repo).
-
Marked as reviewed by mdoerr (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/13859#pullrequestreview-1416794665
Re: RFR: 8307058: Implementation of Generational ZGC [v3]
On Thu, 4 May 2023 09:50:23 GMT, Axel Boldt-Christmas
wrote:
>>> I'm getting build warnings on all linux platforms with gcc-11.3.0:
>>>
>>> ```
>>> src/hotspot/share/gc/z/zDriver.cpp:84:13: error: In the GNU C Library,
>>> "minor" is defined
>>> by . For historical compatibility, it is
>>> currently defined by as well, but we plan to
>>> remove this soon. To use "minor", include
>>> directly. If you did not intend to use a system-defined macro
>>> "minor", you should undefine it after including . [-Werror]
>>>84 | ZDriverMinor* ZDriver::minor() {
>>> ```
>>
>> That's unfortunate as minor and major are quite central to Generational ZGC
>> and having to rename those functions will make the code look worse. I wonder
>> if we should undef minor and major where needed.
>
>> I'm getting build warnings on all linux platforms with gcc-11.3.0:
>>
>> ```
>> src/hotspot/share/gc/z/zDriver.cpp:84:13: error: In the GNU C Library,
>> "minor" is defined
>> by . For historical compatibility, it is
>> currently defined by as well, but we plan to
>> remove this soon. To use "minor", include
>> directly. If you did not intend to use a system-defined macro
>> "minor", you should undefine it after including . [-Werror]
>>84 | ZDriverMinor* ZDriver::minor() {
>> ```
>
> @TheRealMDoerr I cannot reproduce this with gcc but can see the issue with
> clangd.
> Can you check if this patch solves the issue you are seeing?
>
> diff --git a/src/hotspot/share/gc/z/zDriver.hpp
> b/src/hotspot/share/gc/z/zDriver.hpp
> index 640ea6575ef..7fa650b1fa1 100644
> --- a/src/hotspot/share/gc/z/zDriver.hpp
> +++ b/src/hotspot/share/gc/z/zDriver.hpp
> @@ -29,6 +29,14 @@
> #include "gc/z/zThread.hpp"
> #include "gc/z/zTracer.hpp"
>
> +#ifdef minor
> +#undef minor
> +#endif
> +
> +#ifdef major
> +#undef major
> +#endif
> +
> class VM_ZOperation;
> class ZDriverMinor;
> class ZDriverMajor;
@xmas92: Thanks for your quick solution. Your patch solves the problem. If you
want to integrate it, please also add a comment why this is needed.
-
PR Comment: https://git.openjdk.org/jdk/pull/13771#issuecomment-1534563624
Re: RFR: 8307058: Implementation of Generational ZGC [v3]
On Wed, 3 May 2023 19:36:55 GMT, Stefan Karlsson wrote: >> Hi all, >> >> Please review the implementation of Generational ZGC, which can be turned on >> by adding -XX:+ZGenerational in addition to using -XX:+UseZGC. Generational >> ZGC is a major rewrite of the non-generational ZGC version that exists in >> the openjdk/jdk repository. It splits the heap into two generations; the >> young generation where newly allocated objects are born, and the old >> generation where long-lived objects get promoted to. The motivation for >> introducing generations is to allow ZGC to reclaim memory faster by not >> having to walk the entire object graph every time a garbage collection is >> run. This should make Generational ZGC suitable for more workloads. In >> particular workloads that previously hit allocation stalls because of high >> allocation rates, large live sets, or limited spare machine resources, have >> the potential to work better with Generational ZGC. For an in-depth >> description of Generational ZGC, see https://openjdk.org/jeps/439. >> >> The development of Generational ZGC started around the same time as the >> development of JDK 17. At that point we forked off the Generational ZGC >> development into its own branch and let non-generational live unaffected in >> openjdk/jdk. This safe-guarded non-generational ZGC and allowed Generational >> ZGC to move unhindered, without the shackles of having to fit into another >> GC implementation's design and quirks. Since then, almost all of the ZGC >> files have been changed. Moving forward to today, when it's ready for us to >> upstream Generational ZGC, we now need to deliver Generational ZGC without >> disrupting our current user-base. We have therefore opted to initially >> include both versions of ZGC in the code base, but with the intention to >> deprecate non-generational ZGC in a future release. Existing users running >> with only -XX:+UseZGC will get the non-generational ZGC, and users that want >> the new Generational ZGC need to run with -XX:+ZGenerational in addition to >> -XX:+UseZGC. The intention is to give the users time to validate and deploy their workloads with the new GC implementation. >> >> Including both the new evolution of a GC and its legacy predecessor poses a >> few challenges for us GC developers. The first reaction could be to try to >> mash the two implementations together and sprinkle the GC code with >> conditional statements or dynamic dispatches. We have done similar >> experiments before. When ZGC was first born, we started an experiment where >> we converted G1 into getting the same features as the evolving ZGC. It was >> quite clear to us how time consuming and complex things end up being when we >> tried to keep both the original G1 working, and at the same time implemented >> the ZGC-alike G1. Given this experience, we don't see that as a viable >> solution to deliver a maintainable and evolving Generational ZGC. Our >> pragmatic suggestion to these challenges is to let Generational ZGC live >> under the current gc/z directories and let the legacy, non-generational ZGC >> be completely separated in its own directories. This way we can continue to >> move quickly with the continued develo pment of Generational ZGC and let the non-generational ZGC be mostly untouched until it gets deprecated, and eventually removed. The non-generational ZGC directory will be gc/x and all the classes of non-generational have been prefixed with X instead of Z. An alternative to this rename could be to namespace out non-generational ZGC. We experimented with that, but it was too easy to accidentally cross-compile Generational ZGC code into non-generational ZGC, so we didn't like that approach. >> >> Most of the stand-alone cleanups and enhancements outside of the ZGC code >> have already been upstreamed to openjdk/jdk. There are still a few patches >> that could/should be pushed separately, but they will be easier to >> understand by also looking at the Generational ZGC code, so they will be >> sent out after this PR has been published. The patches that could be >> published separately are: >> >> * 59d1e96af6a UPSTREAM: Introduce check_oop infrastructure to check oops in >> the oop class >> * ca9edf8aa79 UPSTREAM: RISCV tmp reg cleanup resolve_jobject >> * 4bec9c69b67 CLEANUP: barrierSetNMethod_aarch64.cpp >> * b67d03a3f04 UPSTREAM: Add relaxed add&fetch for aarch64 atomics >> * a2824734d23 UPSTREAM: lir_xchg >> * 36cd39c0126 UPSTREAM: assembler_ppc CMPLI >> * 447259cea42 UPSTREAM: assembler_ppc ANDI >> * 9417323499a UPSTREAM: Add VMErrorCallback infrastructure >> >> Regarding all the changesets you see in this PR, they form the history of >> the development of Generational ZGC. It might look a bit unconventional to >> what you are used to see in openjdk development. What we have done is to use >> merges with the 'ours' strategy to ignore the previous Generational ZGC >> patche
Re: RFR: 8307058: Implementation of Generational ZGC [v2]
On Wed, 3 May 2023 10:55:49 GMT, Stefan Karlsson wrote: >> Hi all, >> >> Please review the implementation of Generational ZGC, which can be turned on >> by adding -XX:+ZGenerational in addition to using -XX:+UseZGC. Generational >> ZGC is a major rewrite of the non-generational ZGC version that exists in >> the openjdk/jdk repository. It splits the heap into two generations; the >> young generation where newly allocated objects are born, and the old >> generation where long-lived objects get promoted to. The motivation for >> introducing generations is to allow ZGC to reclaim memory faster by not >> having to walk the entire object graph every time a garbage collection is >> run. This should make Generational ZGC suitable for more workloads. In >> particular workloads that previously hit allocation stalls because of high >> allocation rates, large live sets, or limited spare machine resources, have >> the potential to work better with Generational ZGC. For an in-depth >> description of Generational ZGC, see https://openjdk.org/jeps/439. >> >> The development of Generational ZGC started around the same time as the >> development of JDK 17. At that point we forked off the Generational ZGC >> development into its own branch and let non-generational live unaffected in >> openjdk/jdk. This safe-guarded non-generational ZGC and allowed Generational >> ZGC to move unhindered, without the shackles of having to fit into another >> GC implementation's design and quirks. Since then, almost all of the ZGC >> files have been changed. Moving forward to today, when it's ready for us to >> upstream Generational ZGC, we now need to deliver Generational ZGC without >> disrupting our current user-base. We have therefore opted to initially >> include both versions of ZGC in the code base, but with the intention to >> deprecate non-generational ZGC in a future release. Existing users running >> with only -XX:+UseZGC will get the non-generational ZGC, and users that want >> the new Generational ZGC need to run with -XX:+ZGenerational in addition to >> -XX:+UseZGC. The intention is to give the users time to validate and deploy their workloads with the new GC implementation. >> >> Including both the new evolution of a GC and its legacy predecessor poses a >> few challenges for us GC developers. The first reaction could be to try to >> mash the two implementations together and sprinkle the GC code with >> conditional statements or dynamic dispatches. We have done similar >> experiments before. When ZGC was first born, we started an experiment where >> we converted G1 into getting the same features as the evolving ZGC. It was >> quite clear to us how time consuming and complex things end up being when we >> tried to keep both the original G1 working, and at the same time implemented >> the ZGC-alike G1. Given this experience, we don't see that as a viable >> solution to deliver a maintainable and evolving Generational ZGC. Our >> pragmatic suggestion to these challenges is to let Generational ZGC live >> under the current gc/z directories and let the legacy, non-generational ZGC >> be completely separated in its own directories. This way we can continue to >> move quickly with the continued develo pment of Generational ZGC and let the non-generational ZGC be mostly untouched until it gets deprecated, and eventually removed. The non-generational ZGC directory will be gc/x and all the classes of non-generational have been prefixed with X instead of Z. An alternative to this rename could be to namespace out non-generational ZGC. We experimented with that, but it was too easy to accidentally cross-compile Generational ZGC code into non-generational ZGC, so we didn't like that approach. >> >> Most of the stand-alone cleanups and enhancements outside of the ZGC code >> have already been upstreamed to openjdk/jdk. There are still a few patches >> that could/should be pushed separately, but they will be easier to >> understand by also looking at the Generational ZGC code, so they will be >> sent out after this PR has been published. The patches that could be >> published separately are: >> >> * 59d1e96af6a UPSTREAM: Introduce check_oop infrastructure to check oops in >> the oop class >> * ca9edf8aa79 UPSTREAM: RISCV tmp reg cleanup resolve_jobject >> * 4bec9c69b67 CLEANUP: barrierSetNMethod_aarch64.cpp >> * b67d03a3f04 UPSTREAM: Add relaxed add&fetch for aarch64 atomics >> * a2824734d23 UPSTREAM: lir_xchg >> * 36cd39c0126 UPSTREAM: assembler_ppc CMPLI >> * 447259cea42 UPSTREAM: assembler_ppc ANDI >> * 9417323499a UPSTREAM: Add VMErrorCallback infrastructure >> >> Regarding all the changesets you see in this PR, they form the history of >> the development of Generational ZGC. It might look a bit unconventional to >> what you are used to see in openjdk development. What we have done is to use >> merges with the 'ours' strategy to ignore the previous Generational ZGC >> patche
Re: RFR: 8307058: Implementation of Generational ZGC [v2]
On Wed, 3 May 2023 10:55:49 GMT, Stefan Karlsson wrote: >> Hi all, >> >> Please review the implementation of Generational ZGC, which can be turned on >> by adding -XX:+ZGenerational in addition to using -XX:+UseZGC. Generational >> ZGC is a major rewrite of the non-generational ZGC version that exists in >> the openjdk/jdk repository. It splits the heap into two generations; the >> young generation where newly allocated objects are born, and the old >> generation where long-lived objects get promoted to. The motivation for >> introducing generations is to allow ZGC to reclaim memory faster by not >> having to walk the entire object graph every time a garbage collection is >> run. This should make Generational ZGC suitable for more workloads. In >> particular workloads that previously hit allocation stalls because of high >> allocation rates, large live sets, or limited spare machine resources, have >> the potential to work better with Generational ZGC. For an in-depth >> description of Generational ZGC, see https://openjdk.org/jeps/439. >> >> The development of Generational ZGC started around the same time as the >> development of JDK 17. At that point we forked off the Generational ZGC >> development into its own branch and let non-generational live unaffected in >> openjdk/jdk. This safe-guarded non-generational ZGC and allowed Generational >> ZGC to move unhindered, without the shackles of having to fit into another >> GC implementation's design and quirks. Since then, almost all of the ZGC >> files have been changed. Moving forward to today, when it's ready for us to >> upstream Generational ZGC, we now need to deliver Generational ZGC without >> disrupting our current user-base. We have therefore opted to initially >> include both versions of ZGC in the code base, but with the intention to >> deprecate non-generational ZGC in a future release. Existing users running >> with only -XX:+UseZGC will get the non-generational ZGC, and users that want >> the new Generational ZGC need to run with -XX:+ZGenerational in addition to >> -XX:+UseZGC. The intention is to give the users time to validate and deploy their workloads with the new GC implementation. >> >> Including both the new evolution of a GC and its legacy predecessor poses a >> few challenges for us GC developers. The first reaction could be to try to >> mash the two implementations together and sprinkle the GC code with >> conditional statements or dynamic dispatches. We have done similar >> experiments before. When ZGC was first born, we started an experiment where >> we converted G1 into getting the same features as the evolving ZGC. It was >> quite clear to us how time consuming and complex things end up being when we >> tried to keep both the original G1 working, and at the same time implemented >> the ZGC-alike G1. Given this experience, we don't see that as a viable >> solution to deliver a maintainable and evolving Generational ZGC. Our >> pragmatic suggestion to these challenges is to let Generational ZGC live >> under the current gc/z directories and let the legacy, non-generational ZGC >> be completely separated in its own directories. This way we can continue to >> move quickly with the continued develo pment of Generational ZGC and let the non-generational ZGC be mostly untouched until it gets deprecated, and eventually removed. The non-generational ZGC directory will be gc/x and all the classes of non-generational have been prefixed with X instead of Z. An alternative to this rename could be to namespace out non-generational ZGC. We experimented with that, but it was too easy to accidentally cross-compile Generational ZGC code into non-generational ZGC, so we didn't like that approach. >> >> Most of the stand-alone cleanups and enhancements outside of the ZGC code >> have already been upstreamed to openjdk/jdk. There are still a few patches >> that could/should be pushed separately, but they will be easier to >> understand by also looking at the Generational ZGC code, so they will be >> sent out after this PR has been published. The patches that could be >> published separately are: >> >> * 59d1e96af6a UPSTREAM: Introduce check_oop infrastructure to check oops in >> the oop class >> * ca9edf8aa79 UPSTREAM: RISCV tmp reg cleanup resolve_jobject >> * 4bec9c69b67 CLEANUP: barrierSetNMethod_aarch64.cpp >> * b67d03a3f04 UPSTREAM: Add relaxed add&fetch for aarch64 atomics >> * a2824734d23 UPSTREAM: lir_xchg >> * 36cd39c0126 UPSTREAM: assembler_ppc CMPLI >> * 447259cea42 UPSTREAM: assembler_ppc ANDI >> * 9417323499a UPSTREAM: Add VMErrorCallback infrastructure >> >> Regarding all the changesets you see in this PR, they form the history of >> the development of Generational ZGC. It might look a bit unconventional to >> what you are used to see in openjdk development. What we have done is to use >> merges with the 'ours' strategy to ignore the previous Generational ZGC >> patche
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v17]
On Fri, 21 Apr 2023 20:26:12 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Correct comment on isPPC64() and refer to isLittleEndian() instead of > mentioning PPC64LE Thanks for the comment updates! - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13357#pullrequestreview-1397600716
Re: RFR: 8306543: GHA: MSVC installation is failing
On Fri, 21 Apr 2023 08:40:49 GMT, Christoph Langer wrote: > This is the plain fix for the currently observed error in GHA. > > Will handle the optional install of the MSVC toolchain in a separate item. LGTM. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13576#pullrequestreview-1395496752
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v15]
On Mon, 17 Apr 2023 20:59:06 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 17 commits: > > - Merge branch 'master' into 8304915-arch-enum > - ArchTest on Debian RISC-V 64 confirmed by reviewer > - Fixed isPPC64(). >Consolidated switch cases in ArchTest. >Moved mapping of build TARGET_OS and TARGET_CPU to the build >to avoid multiple mappings in more than one place. > - Correct mapping and test of ppc64 > - Add ppc64 as mapping to PPC64 Architecture > - Modified test to check Architecture is64bits() and isLittleEndian() >against Unsafe respective values. >Relocated code mapping OS name and arch name from PlatformProps to >OperatingSystem and Architecture. Kept the mapping of names >in the template close to where the values are filled in by the build. > - Remove unused static and import of Stabile > - Rename OperatingSystemProps to PlatformProps. >Refactor OperatingSystem initialization to use strings instead of integers. > - Revised mapping mechanism of build target architecture names to enum > values. >Unrecognized values from the build are mapped to enum value "OTHER". >Renamed PPC64LE to PPC64 to reflect only the architecture, not the > endianness. >Added an `isLittleEndian` method to return the endianness (not currently > used anywhere) > - Revert changes to jdk.accessibility AccessBridge > - ... and 7 more: https://git.openjdk.org/jdk/compare/8858d543...99a93b7e test/jdk/jdk/internal/util/ArchTest.java line 71: > 69: case "aarch64" -> AARCH64; > 70: case "riscv64" -> RISCV64; > 71: case "s390x", "s390" -> S390; // unverified This was also verified according to comments. Right, @offamitkumar? - PR Review Comment: https://git.openjdk.org/jdk/pull/13357#discussion_r1171335657
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v13]
On Wed, 12 Apr 2023 17:31:49 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed isPPC64(). > Consolidated switch cases in ArchTest. > Moved mapping of build TARGET_OS and TARGET_CPU to the build > to avoid multiple mappings in more than one place. Thanks! PPC64 parts look good and the test has passed on both endianness versions. - Marked as reviewed by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13357#pullrequestreview-1381866371
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v12]
On Tue, 11 Apr 2023 21:09:43 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Correct mapping and test of ppc64 Works on PPC64 Big Endian, now. However, little Endian fails: org.opentest4j.AssertionFailedError: Mismatch PPC64 == PPC64 vs isPPC64 ==> expected: but was: at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55) at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62) at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182) at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1152) at ArchTest.isArch(ArchTest.java:99) ... System property os.arch: "ppc64le", Architecture.current(): "PPC64" - PR Comment: https://git.openjdk.org/jdk/pull/13357#issuecomment-1504925182
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v11]
On Tue, 11 Apr 2023 18:35:56 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Add ppc64 as mapping to PPC64 Architecture Thanks for the updates! PPC64 looks basically good except of a upper / lower case issue in the test: STARTEDArchTest::nameVsCurrent 'nameVsCurrent()' org.opentest4j.AssertionFailedError: mismatch in Architecture.current vs ppc64 ==> expected: but was: ... FAILED ArchTest::nameVsCurrent 'nameVsCurrent()' JavaTest Message: JUnit Platform Failure(s): 1 - PR Comment: https://git.openjdk.org/jdk/pull/13357#issuecomment-1504064549
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v10]
On Tue, 11 Apr 2023 17:58:54 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Modified test to check Architecture is64bits() and isLittleEndian() > against Unsafe respective values. > Relocated code mapping OS name and arch name from PlatformProps to > OperatingSystem and Architecture. Kept the mapping of names > in the template close to where the values are filled in by the build. Another remark: Old JDK on s390 used "os.arch = zArch_64", current one "os.arch = s390x". @offamitkumar: You probably want to take a look. - PR Comment: https://git.openjdk.org/jdk/pull/13357#issuecomment-1503861585
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v10]
On Tue, 11 Apr 2023 17:58:54 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Modified test to check Architecture is64bits() and isLittleEndian() > against Unsafe respective values. > Relocated code mapping OS name and arch name from PlatformProps to > OperatingSystem and Architecture. Kept the mapping of names > in the template close to where the values are filled in by the build. test/jdk/jdk/internal/util/ArchTest.java line 74: > 72: case "riscv64" -> RISCV64; // unverified > 73: case "s390x", "s390" -> S390; // unverified > 74: case "ppc64le" -> PPC64; // unverified I think "ppc64" should also get mapped to "PPC64". - PR Review Comment: https://git.openjdk.org/jdk/pull/13357#discussion_r1163161387
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v9]
On Tue, 11 Apr 2023 10:26:02 GMT, ExE Boss wrote: >> test/jdk/jdk/internal/util/ArchTest.java line 128: >> >>> 126: case RISCV64 -> true; >>> 127: case S390 -> false; >>> 128: case PPC64 -> true; >> >> This is not always true. The PPC64 architecture supports both endianness >> versions. AIX and legacy linux is Big Endian while recent linux is little >> Endian (determined by platform name "os.arch = ppc64le" instead of "os.arch >> = ppc64"). Querying the endianness is also possible, of course. > > This should (probably) be correct: > Suggestion: > > case PPC64 -> !OperatingSystem.isAix() && > Architecture.isLittleEndian(); Looks correct, but makes the test pointless for any linux on PPC64. - PR Review Comment: https://git.openjdk.org/jdk/pull/13357#discussion_r1162628828
Re: RFR: 8304915: Create jdk.internal.util.Architecture enum and apply [v9]
On Sat, 8 Apr 2023 18:00:53 GMT, Roger Riggs wrote: >> Define an internal jdk.internal.util.Architecture enumeration and static >> methods to replace uses of the system property `os.arch`. >> The enumeration values are defined to match those used in the build. >> The initial values are: `X64, X86, AARCH64, RISCV64, S390, PPC64` >> Note that `amd64` and `x86_64` in the build are represented by `X64`. >> The value of the system property `os.arch` is unchanged. >> >> The API is similar to the jdk.internal.util.OperatingSystem enum created by >> #[12931](https://git.openjdk.org/jdk/pull/12931). >> Uses in `java.base` and a few others are included but other modules will be >> done in separate PRs. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Remove unused static and import of Stabile Would be great if you could support "os.arch = ppc64" for AIX and legacy linux, too. test/jdk/jdk/internal/util/ArchTest.java line 128: > 126: case RISCV64 -> true; > 127: case S390 -> false; > 128: case PPC64 -> true; This is not always true. The PPC64 architecture supports both endianness versions. AIX and legacy linux is Big Endian while recent linux is little Endian (determined by platform name "os.arch = ppc64le" instead of "os.arch = ppc64"). Querying the endianness is also possible, of course. - Changes requested by mdoerr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13357#pullrequestreview-1378905956 PR Review Comment: https://git.openjdk.org/jdk/pull/13357#discussion_r1162600674
Re: RFR: 8304434: [AIX] Update minimum xlclang version
On Fri, 17 Mar 2023 22:36:24 GMT, Tyler Steele wrote: > Minor changes to the build system to recognize and warn that an incompatible > toolchain is detected. Yes, I think it's better to define the minimum version to a compiler which will work at the point of time at which we integrate this PR. - PR Comment: https://git.openjdk.org/jdk/pull/13086#issuecomment-1481716972
Re: RFR: 8304434: [AIX] Update minimum xlclang version
On Fri, 17 Mar 2023 22:36:24 GMT, Tyler Steele wrote: > Minor changes to the build system to recognize and warn that an incompatible > toolchain is detected. I think we should get it compiling before we define the minimum level. Otherwise we may have to change it several times. - PR Comment: https://git.openjdk.org/jdk/pull/13086#issuecomment-1480740213
Re: RFR: 8304434: [AIX] Update minimum xlclang version
On Fri, 17 Mar 2023 22:36:24 GMT, Tyler Steele wrote: > Minor changes to the build system to recognize and warn that an incompatible > toolchain is detected. I think 16.1.0.0002 is pretty old. Shouldn't we better require some version we have tested? We have tested 16.1.0.13. - PR Comment: https://git.openjdk.org/jdk/pull/13086#issuecomment-1477576163
Re: RFR: 8304434: [AIX] Update minimum xlclang version
On Fri, 17 Mar 2023 22:36:24 GMT, Tyler Steele wrote: > Minor changes to the build system to recognize and warn that an incompatible > toolchain is detected. LGTM. I think the reason for this PR is another issue? If so, please link the JBS issues as "relates to". @MBaesken: Please take a look. - PR Comment: https://git.openjdk.org/jdk/pull/13086#issuecomment-1476860602
Re: RFR: JDK-8303949: gcc10 warning Linux ppc64le - note: the layout of aggregates containing vectors with 8-byte alignment has changed in GCC 5
On Fri, 10 Mar 2023 12:28:05 GMT, Matthias Baesken wrote: > When switching from gcc8 to gcc10 on Linux ppc64le, we get dozens of warnings > in hotspot and other sources like this > > adaptiveSizePolicy.cpp:262:6: note: the layout of aggregates containing > vectors with 8-byte alignment has changed in GCC 5 > > Probably it would make sense to suppress the warning, -Wno-psabi might be an > option. > Some info about this : > > https://patches.dpdk.org/project/dpdk/patch/20210623161620.49916-1-...@linux.vnet.ibm.com/ > > https://stackoverflow.com/questions/52020305/what-exactly-does-gccs-wpsabi-option-do-what-are-the-implications-of-supressi LGTM. Thanks! - Marked as reviewed by mdoerr (Reviewer). PR: https://git.openjdk.org/jdk/pull/12972
Re: RFR: 8292591: Experimentally add back barrier-less Java thread transitions [v4]
On Fri, 9 Sep 2022 07:27:43 GMT, Robbin Ehn wrote:
>> Please consider, only implemented on x64/aarch64 linux/windows.
>> (@TheRealMDoerr have now contributed PPC64)
>>
>> On my box calling clock_gettime via JNI goes from 35ns to 28ns when enabled.
>>
>> Passes t1-7 with option forced on, also passes t1-4 as is in this PR.
>
> Robbin Ehn has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains five additional commits since
> the last revision:
>
> - Merge branch 'master' into membar
> - Test fixes
> - PPC64 port courtesy Martin Doerr
> - Change header and constants handling
> - 8292591 - initial
Thanks for the update! LGTM. I hope that no other platform with older linux
kernel is still needed.
src/hotspot/os/linux/systemMemoryBarrier_linux.cpp line 83:
> 81:
> 82: void LinuxSystemMemoryBarrier::emit() {
> 83: int s = membarrier(MEMBARRIER_CMD_PRIVATE_EXPEDITED, 0, 0);
Extra whitespace. Feel free to remove it.
-
Marked as reviewed by mdoerr (Reviewer).
PR: https://git.openjdk.org/jdk/pull/10123
Re: RFR: 8292591: Experimentally add back barrier-less Java thread transitions [v3]
On Thu, 8 Sep 2022 18:13:56 GMT, Robbin Ehn wrote:
>> Please consider, only implemented on x64/aarch64 linux/windows.
>> (@TheRealMDoerr have now contributed PPC64)
>>
>> On my box calling clock_gettime via JNI goes from 35ns to 28ns when enabled.
>>
>> Passes t1-7 with option forced on, also passes t1-4 as is in this PR.
>
> Robbin Ehn has updated the pull request incrementally with one additional
> commit since the last revision:
>
> PPC64 port courtesy Martin Doerr
Thanks for integrating my PPC64 implementation. I prefer serving as reviewer.
My contribution was rather small. The implementation LGTM.
However, the test should pass in case of `MEMBARRIER_CMD_QUERY unsupported`.
Note: I believe the test should also work in product builds when adding
`commands.add("-XX:+UnlockDiagnosticVMOptions");`, but I don't insist on that.
I leave you free to decide.
-
Changes requested by mdoerr (Reviewer).
PR: https://git.openjdk.org/jdk/pull/10123
Re: RFR: 8292591: Experimentally add back barrier-less Java thread transitions [v2]
On Thu, 8 Sep 2022 09:44:44 GMT, Robbin Ehn wrote:
>> Please consider, only implemented on x64/aarch64 linux/windows.
>>
>> On my box calling clock_gettime via JNI goes from 35ns to 28ns when enabled.
>>
>> Passes t1-7 with option forced on, also passes t1-4 as is in this PR.
>
> Robbin Ehn has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Change header and constants handling
We need to support older kernel versions on PPC64 as well. And let's just make
it usable on PPC64. Improves performance of trivial native calls by about 20%.
diff --git a/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp
b/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp
index 5a55d8f4dc2..0902c0f35ed 100644
--- a/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp
+++ b/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp
@@ -2147,7 +2147,9 @@ nmethod
*SharedRuntime::generate_native_wrapper(MacroAssembler *masm,
Label no_block, sync;
// Force this write out before the read below.
-__ fence();
+if (!UseSystemMemoryBarrier) {
+ __ fence();
+}
Register sync_state_addr = r_temp_4;
Register sync_state = r_temp_5;
diff --git a/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp
b/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp
index bd4e1ce9932..260d99bef81 100644
--- a/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp
+++ b/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp
@@ -1436,7 +1436,9 @@ address
TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
__ li(R0/*thread_state*/, _thread_in_native_trans);
__ release();
__ stw(R0/*thread_state*/, thread_(thread_state));
- __ fence();
+ if (!UseSystemMemoryBarrier) {
+__ fence();
+ }
// Now before we return to java we must look for a current safepoint
// (a new safepoint can not start since we entered native_trans).
diff --git a/src/hotspot/os/linux/systemMemoryBarrier_linux.cpp
b/src/hotspot/os/linux/systemMemoryBarrier_linux.cpp
index 796b596c675..66f77818ebc 100644
--- a/src/hotspot/os/linux/systemMemoryBarrier_linux.cpp
+++ b/src/hotspot/os/linux/systemMemoryBarrier_linux.cpp
@@ -35,6 +35,8 @@
#ifndef SYS_membarrier
#if defined(AMD64)
#define SYS_membarrier 324
+ #elif defined(PPC64)
+ #define SYS_membarrier 365
#else
#error define SYS_membarrier for the arch
#endif
Btw. are spaces in front of `#` ok? Some old preprocessors might have problems
with that.
-
Changes requested by mdoerr (Reviewer).
PR: https://git.openjdk.org/jdk/pull/10123
Re: RFR: 8292591: Experimentally add back barrier-less Java thread transitions
On Thu, 1 Sep 2022 16:47:58 GMT, Robbin Ehn wrote: > Please consider, only implemented on x64/aarch64 linux/windows. > > On my box calling clock_gettime via JNI goes from 35ns to 28ns when enabled. > > Passes t1-7 with option forced on, also passes t1-4 as is in this PR. Nice to see usage of this OS feature. I like it more than the serialization page we have in 11u (with `-XX:-UseMembar`) and Critical JNI Natives (which still have some advantage). I have observed the same speedup for a trivial JNI call on my x86_64 machine. src/hotspot/os/linux/systemMemoryBarrier_linux.cpp line 36: > 34: // Syscall defined in kernel 4.3 > 35: #if !defined(SYS_membarrier) > 36: #define SYS_membarrier 324 I'd like to get rid of the platform checks in this generic linux file. This number seems to be the only platform specific part: - PR: https://git.openjdk.org/jdk/pull/10123
