Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v11]
On Tue, 9 Jul 2024 12:08:50 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review the patch? >> This pr is based on previous work and discussion in [pr >> 16234](https://github.com/openjdk/jdk/pull/16234), [pr >> 18294](https://github.com/openjdk/jdk/pull/18294). >> * NOTE: This pr depends on https://github.com/openjdk/jdk/pull/19185, which >> includes a README, a script to generate sleef inline headers and generated >> sleef inline headers. >> >> Compared with previous prs, the major change in this pr is to integrate the >> source of sleef (for the steps, please check >> `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than >> depends on external sleef things (header or lib) at build or run time. >> Besides of this change, also modify the previous changes accordingly, e.g. >> remove some uncessary files or changes especially in make dir of jdk. >> >> Besides of the code changes, one important task is to handle the legal >> process. >> >> Thanks! >> >> ## Test >> tests: >> * test/jdk/jdk/incubator/vector/ >> * test/hotspot/jtreg/compiler/vectorapi/ >> >> options: >> * -XX:UseSVE=1 -XX:+EnableVectorSupport -XX:+UseVectorStubs >> * -XX:UseSVE=0 -XX:+EnableVectorSupport -XX:+UseVectorStubs >> * -XX:+EnableVectorSupport -XX:-UseVectorStubs >> >> ## Performance >> >> ### Options >> * +intrinsic: >> 'FORK=1;ITER=10;WARMUP_ITER=10;JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions >> -XX:+EnableVectorSupport -XX:+UseVectorStubs' >> * -intrinsic: >> 'FORK=1;ITER=10;WARMUP_ITER=10;JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions >> -XX:+EnableVectorSupport -XX:-UseVectorStubs' >> >> ### Float >> data >> >> Benchmark | (size) | Mode | Cnt | Error | Units | Score +intrinsic >> (UseSVE=1) | Score -intrinsic | Improvement(UseSVE=1) | Score +intrinsic >> (UseSVE=0) | Score -intrinsic | Improvement (UseSVE=0) >> -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- >> Float128Vector.ACOS | 1024 | thrpt | 10 | 0.015 | ops/ms | 245.439 | 101.483 >> | 2.419 | 245.733 | 102.033 | 2.408 >> Float128Vector.ASIN | 1024 | thrpt | 10 | 0.013 | ops/ms | 296.702 | 103.559 >> | 2.865 | 296.741 | 103.18 | 2.876 >> Float128Vector.ATAN | 1024 | thrpt | 10 | 0.004 | ops/ms | 196.862 | 49.627 >> | 3.967 | 195.891 | 49.771 | 3.936 >> Float128Vector.ATAN... > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > skip TANH I think the key question is whether we're comfortable relying on/pointing at an external repository which may or may not be there tomorrow and/or where tags may change outside of our control. The SLEEF source code looks to be around 7.5MB, give or take. That's not enormous, but it's not exactly small when keeping in mind that if we `#include` it in the jdk repo it's going to be there for every cloned repo in every project/branch and very few will actually care about it. I agree that we'd still have to include the pre-generated header files. Hence my suggestion to consider putting it under our control, but in a separate `openjdk` controlled repository. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2229457499
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v11]
On Tue, 9 Jul 2024 12:08:50 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review the patch? >> This pr is based on previous work and discussion in [pr >> 16234](https://github.com/openjdk/jdk/pull/16234), [pr >> 18294](https://github.com/openjdk/jdk/pull/18294). >> * NOTE: This pr depends on https://github.com/openjdk/jdk/pull/19185, which >> includes a README, a script to generate sleef inline headers and generated >> sleef inline headers. >> >> Compared with previous prs, the major change in this pr is to integrate the >> source of sleef (for the steps, please check >> `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than >> depends on external sleef things (header or lib) at build or run time. >> Besides of this change, also modify the previous changes accordingly, e.g. >> remove some uncessary files or changes especially in make dir of jdk. >> >> Besides of the code changes, one important task is to handle the legal >> process. >> >> Thanks! >> >> ## Test >> tests: >> * test/jdk/jdk/incubator/vector/ >> * test/hotspot/jtreg/compiler/vectorapi/ >> >> options: >> * -XX:UseSVE=1 -XX:+EnableVectorSupport -XX:+UseVectorStubs >> * -XX:UseSVE=0 -XX:+EnableVectorSupport -XX:+UseVectorStubs >> * -XX:+EnableVectorSupport -XX:-UseVectorStubs >> >> ## Performance >> >> ### Options >> * +intrinsic: >> 'FORK=1;ITER=10;WARMUP_ITER=10;JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions >> -XX:+EnableVectorSupport -XX:+UseVectorStubs' >> * -intrinsic: >> 'FORK=1;ITER=10;WARMUP_ITER=10;JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions >> -XX:+EnableVectorSupport -XX:-UseVectorStubs' >> >> ### Float >> data >> >> Benchmark | (size) | Mode | Cnt | Error | Units | Score +intrinsic >> (UseSVE=1) | Score -intrinsic | Improvement(UseSVE=1) | Score +intrinsic >> (UseSVE=0) | Score -intrinsic | Improvement (UseSVE=0) >> -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- >> Float128Vector.ACOS | 1024 | thrpt | 10 | 0.015 | ops/ms | 245.439 | 101.483 >> | 2.419 | 245.733 | 102.033 | 2.408 >> Float128Vector.ASIN | 1024 | thrpt | 10 | 0.013 | ops/ms | 296.702 | 103.559 >> | 2.865 | 296.741 | 103.18 | 2.876 >> Float128Vector.ATAN | 1024 | thrpt | 10 | 0.004 | ops/ms | 196.862 | 49.627 >> | 3.967 | 195.891 | 49.771 | 3.936 >> Float128Vector.ATAN... > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > skip TANH If we want the traceability (which I agree is good) of the SLEEF source code but want to avoid having it in the jdk repo itself (adding unnecessary "bloat" for everybody), perhaps we can consider having it in a separate repository somewhere in/under `openjdk`? It's not immediately clear to me that we need to have support in the JDK build system (configure/make) itself for building/updating the header files, as long as there's a simple, documented way of doing so. I like to think the `createSleef.sh` script is that, but I recognize that I'm biased because I wrote it. - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2229380066
Re: RFR: 8329816: Add SLEEF version 3.6.1
On Mon, 24 Jun 2024 13:30:35 GMT, Hamlin Li wrote: >> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to >> optimize vector math operations by leveraging the SLEEF library. For legal >> reasons the actual contribution of the SLEEF files needs to be handled >> separately. This enhancement adds the relevant files, enabling the rest of >> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. > > In case you need it and avoid to generate yourself, I've generated sleef > inline header of 3.6.1 for riscv, it's at > https://github.com/openjdk/jdk/pull/18605/commits/c279a3c290554892939267d9ebe88198988d31a4 @Hamlin-Li I have generated the header files (two aarch64 and the new one for riscv64) using SLEEF 3.6.1. Please make sure they match your expectations! - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2195728763
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v3]
> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to > optimize vector math operations by leveraging the SLEEF library. For legal > reasons the actual contribution of the SLEEF files needs to be handled > separately. This enhancement adds the relevant files, enabling the rest of > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. Mikael Vidstedt has updated the pull request incrementally with one additional commit since the last revision: Update README to include RISC-V - Changes: - all: https://git.openjdk.org/jdk/pull/19185/files - new: https://git.openjdk.org/jdk/pull/19185/files/b29de559..8a08ffa1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19185=02 - incr: https://webrevs.openjdk.org/?repo=jdk=19185=01-02 Stats: 10 lines in 1 file changed: 9 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19185.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19185/head:pull/19185 PR: https://git.openjdk.org/jdk/pull/19185
Re: RFR: 8329816: Add SLEEF version 3.6.1
On Fri, 10 May 2024 22:32:29 GMT, Mikael Vidstedt wrote: > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to > optimize vector math operations by leveraging the SLEEF library. For legal > reasons the actual contribution of the SLEEF files needs to be handled > separately. This enhancement adds the relevant files, enabling the rest of > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. I, too, envision that we'll be importing header files (only). That said, I'd very much prefer *not* to rename them as part of the import. If anything I could see us have architecture specific directories in which we place the respective files (and a common directory for `misc.h`), but it's not immediately clear to me that it's worth it given that the files are used in such a narrow context in the JDK. - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2128767164
Re: RFR: 8329816: Add SLEEF version 3.6.1 [v2]
> [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to > optimize vector math operations by leveraging the SLEEF library. For legal > reasons the actual contribution of the SLEEF files needs to be handled > separately. This enhancement adds the relevant files, enabling the rest of > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. Mikael Vidstedt 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 three additional commits since the last revision: - Add SLEEF version 3.6.1 - Merge branch 'master' into 8329816-sleef - 8329816: Add SLEEF version 3.6 - Changes: - all: https://git.openjdk.org/jdk/pull/19185/files - new: https://git.openjdk.org/jdk/pull/19185/files/b02efa6b..b29de559 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19185=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19185=00-01 Stats: 160121 lines in 2836 files changed: 111602 ins; 34872 del; 13647 mod Patch: https://git.openjdk.org/jdk/pull/19185.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19185/head:pull/19185 PR: https://git.openjdk.org/jdk/pull/19185
Integrated: 8332849: Update doc/testing.{md, html} (spelling and stale information)
On Thu, 23 May 2024 23:22:51 GMT, Mikael Vidstedt wrote:
> Update the testing doc to remove some stale information (AOT_MODULES, removed
> in [JDK-8264805](https://bugs.openjdk.org/browse/JDK-8264805)) and fix some
> spelling issues.
>
> Testing: tier1
This pull request has now been integrated.
Changeset: da6aa2a8
Author:Mikael Vidstedt
URL:
https://git.openjdk.org/jdk/commit/da6aa2a86c86ba5fce747b36dcb2d6001cfcc44e
Stats: 43 lines in 2 files changed: 0 ins; 28 del; 15 mod
8332849: Update doc/testing.{md,html} (spelling and stale information)
Reviewed-by: iris, ihse, erikj, djelinski
-
PR: https://git.openjdk.org/jdk/pull/19375
Re: RFR: 8332849: Update doc/testing.{md,html} (spelling and stale information) [v2]
On Fri, 24 May 2024 21:05:27 GMT, Mikael Vidstedt wrote: >> Update the testing doc to remove some stale information (AOT_MODULES, >> removed in [JDK-8264805](https://bugs.openjdk.org/browse/JDK-8264805)) and >> fix some spelling issues. >> >> Testing: tier1 > > Mikael Vidstedt has updated the pull request incrementally with one > additional commit since the last revision: > > Revert change to deliberately misspelled variables Thank you for all the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/19375#issuecomment-2135734935
Re: RFR: 8332849: Update doc/testing.{md,html} (spelling and stale information) [v2]
On Fri, 24 May 2024 12:50:59 GMT, Erik Joelsson wrote: >> doc/testing.html line 366: >> >>> 364: to setting JTREG_JOBS=1 JTREG_TIMEOUT_FACTOR=8, but using >>> 365: the keyword format means that the JTREG variable is parsed >>> 366: and verified for correctness, so JTREG="TIMEOUT_FACTOR=8" >> >> this was a deliberate typo > > Indeed, these two were intentional to demonstrate the error handling of the > JTREG make variable. Well that's embarrassing.. Thank you for the catch! Fixed (reverted). - PR Review Comment: https://git.openjdk.org/jdk/pull/19375#discussion_r1614063368
Re: RFR: 8332849: Update doc/testing.{md,html} (spelling and stale information) [v2]
> Update the testing doc to remove some stale information (AOT_MODULES, removed > in [JDK-8264805](https://bugs.openjdk.org/browse/JDK-8264805)) and fix some > spelling issues. > > Testing: tier1 Mikael Vidstedt has updated the pull request incrementally with one additional commit since the last revision: Revert change to deliberately misspelled variables - Changes: - all: https://git.openjdk.org/jdk/pull/19375/files - new: https://git.openjdk.org/jdk/pull/19375/files/859e52d1..88e930e1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19375=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19375=00-01 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/19375.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19375/head:pull/19375 PR: https://git.openjdk.org/jdk/pull/19375
RFR: 8332849: Update doc/testing.{md,html} (spelling and stale information)
Update the testing doc to remove some stale information (AOT_MODULES, removed
in [JDK-8264805](https://bugs.openjdk.org/browse/JDK-8264805)) and fix some
spelling issues.
Testing: tier1
-
Commit messages:
- 8332849: Update doc/testing.{md,html} (spelling and stale information)
Changes: https://git.openjdk.org/jdk/pull/19375/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=19375=00
Issue: https://bugs.openjdk.org/browse/JDK-8332849
Stats: 46 lines in 2 files changed: 0 ins; 28 del; 18 mod
Patch: https://git.openjdk.org/jdk/pull/19375.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19375/head:pull/19375
PR: https://git.openjdk.org/jdk/pull/19375
Re: RFR: 8329816: Add SLEEF version 3.6
On Fri, 10 May 2024 22:32:29 GMT, Mikael Vidstedt wrote: > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to > optimize vector math operations by leveraging the SLEEF library. For legal > reasons the actual contribution of the SLEEF files needs to be handled > separately. This enhancement adds the relevant files, enabling the rest of > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. Thank you Hamlin. I'll try to keep my eyes open for the announcement of the upcoming SLEEF release but do feel free to notify me if you see it first! - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-211796
Re: RFR: 8329816: Add SLEEF version 3.6
On Fri, 10 May 2024 22:32:29 GMT, Mikael Vidstedt wrote: > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to > optimize vector math operations by leveraging the SLEEF library. For legal > reasons the actual contribution of the SLEEF files needs to be handled > separately. This enhancement adds the relevant files, enabling the rest of > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. Let me suggest that we wait for the next release of SLEEF to be released in that case since that release seems to be imminent. That way we'll get both the performance fixes *and* we can include the RISC-V header files at the same time. Fair? - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2115793711
Re: RFR: 8329816: Add SLEEF version 3.6
On Fri, 10 May 2024 22:32:29 GMT, Mikael Vidstedt wrote: > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to > optimize vector math operations by leveraging the SLEEF library. For legal > reasons the actual contribution of the SLEEF files needs to be handled > separately. This enhancement adds the relevant files, enabling the rest of > [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. Looks like that change is not yet in a released version of SLEEF, and in particular not in 3.6. We do need updated approvals when we pick up new versions. Since we've gone through the process once it's typically easier/faster to do so. It will be technically easier/faster as well, now that I know how to do it and have encoded it in the createSleef.sh script. - PR Comment: https://git.openjdk.org/jdk/pull/19185#issuecomment-2108363977
RFR: 8329816: Add SLEEF version 3.6
[JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) is looking to optimize vector math operations by leveraging the SLEEF library. For legal reasons the actual contribution of the SLEEF files needs to be handled separately. This enhancement adds the relevant files, enabling the rest of [JDK-8312425](https://bugs.openjdk.org/browse/JDK-8312425) to move forward. - Commit messages: - 8329816: Add SLEEF version 3.6 Changes: https://git.openjdk.org/jdk/pull/19185/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19185=00 Issue: https://bugs.openjdk.org/browse/JDK-8329816 Stats: 14313 lines in 6 files changed: 14313 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/19185.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19185/head:pull/19185 PR: https://git.openjdk.org/jdk/pull/19185
Re: RFR: 8331939: Add custom hook for TestImage
On Wed, 8 May 2024 14:03:48 GMT, Erik Joelsson wrote: > We need a custom hook in TestImage.gmk to modify behavior when building with > custom source. Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19139#pullrequestreview-2046326105
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v2]
On Fri, 5 Apr 2024 12:17:17 GMT, Hamlin Li wrote: >> Hi, >> Can you help to review the patch? >> This pr is based on previous work and discussion in [pr >> 16234](https://github.com/openjdk/jdk/pull/16234), [pr >> 18294](https://github.com/openjdk/jdk/pull/18294). >> >> Compared with previous prs, the major change in this pr is to integrate the >> source of sleef (for the steps, please check >> `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than >> depends on external sleef things (header or lib) at build or run time. >> Besides of this change, also modify the previous changes accordingly, e.g. >> remove some uncessary files or changes especially in make dir of jdk. >> >> Besides of the code changes, one important task is to handle the legal >> process. >> >> Thanks! > > Hamlin Li has updated the pull request incrementally with two additional > commits since the last revision: > > - disable unused-function warnings; add log msg > - minor Thank you for the update and for working on this in general. I've started working on JDK-8329816, preparing the change for the SLEEF specific part of the change. Specifically, I'm currently planning on including the three SLEEF header files, the README and a legal/sleef.md file in that change. Let me know if you have any thoughts/concerns. Also, just for my understanding, would love to understand your thoughts on the future here (I apologize if this was already discussed elsewhere): It seem like SLEEF is (sort of) limited to linux at this point (the SLEEF README mentions that "Due to limited test capacities, SLEEF is currently only officially supported on Linux with gcc or llvm/clang." ). That same README does, however, indicate good test coverage on several architectures in addition to aarch64 (including x86_64, PPC, RISC-V). With that in mind, it looks like we could potentially use SLEEF for other architectures on linux in the future? And potentially additional operating systems as well? - PR Comment: https://git.openjdk.org/jdk/pull/18605#issuecomment-2045972249
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF
On Wed, 3 Apr 2024 14:40:42 GMT, Hamlin Li wrote:
> Hi,
> Can you help to review the patch?
> This pr is based on previous work and discussion in [pr
> 16234](https://github.com/openjdk/jdk/pull/16234), [pr
> 18294](https://github.com/openjdk/jdk/pull/18294).
>
> Compared with previous prs, the major change in this pr is to integrate the
> source of sleef (for the steps, please check
> `src/jdk.incubator.vector/linux/native/libvectormath/README`), rather than
> depends on external sleef things (header or lib) at build or run time.
> Besides of this change, also modify the previous changes accordingly, e.g.
> remove some uncessary files or changes especially in make dir of jdk.
>
> Besides of the code changes, one important task is to handle the legal
> process.
>
> Thanks!
make/modules/jdk.incubator.vector/Lib.gmk line 44:
> 42: $(eval $(call SetupJdkLibrary, BUILD_LIBVECTORMATH, \
> 43: NAME := vectormath, \
> 44: CFLAGS := $(CFLAGS_JDKLIB) -Wno-error=unused-function, \
Should the unused-function be passed in using `DISABLE_WARNINGS_*` instead?
src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 8601:
> 8599: }
> 8600: } else {
> 8601: log_info(library)("Failed to load native vector math library!");
Include the `ebuf` message? The corresponding x86_64 code could also use a log
message for the error case.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18605#discussion_r1552502695
PR Review Comment: https://git.openjdk.org/jdk/pull/18605#discussion_r1552499482
Re: RFR: 8320799: Bump minimum boot jdk to JDK 22
On Mon, 1 Apr 2024 16:16:50 GMT, Mikael Vidstedt wrote: > With the JDK 22 GA out it's time to bump the minimum boot JDK version for > mainline/JDK 23. > > Testing: tier1-5, GHA Thank you for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/18563#issuecomment-2035049134
Integrated: 8320799: Bump minimum boot jdk to JDK 22
On Mon, 1 Apr 2024 16:16:50 GMT, Mikael Vidstedt wrote: > With the JDK 22 GA out it's time to bump the minimum boot JDK version for > mainline/JDK 23. > > Testing: tier1-5, GHA This pull request has now been integrated. Changeset: 023f7f17 Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/023f7f176b32ffa38dd599ea110c2b9c18886b74 Stats: 12 lines in 3 files changed: 0 ins; 0 del; 12 mod 8320799: Bump minimum boot jdk to JDK 22 Reviewed-by: iris, erikj, ihse - PR: https://git.openjdk.org/jdk/pull/18563
RFR: 8320799: Bump minimum boot jdk to JDK 22
With the JDK 22 GA out it's time to bump the minimum boot JDK version for mainline/JDK 23. Testing: tier1-5, GHA - Commit messages: - 8320799: Bump minimum boot jdk to JDK 22 Changes: https://git.openjdk.org/jdk/pull/18563/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18563=00 Issue: https://bugs.openjdk.org/browse/JDK-8320799 Stats: 12 lines in 3 files changed: 0 ins; 0 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/18563.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18563/head:pull/18563 PR: https://git.openjdk.org/jdk/pull/18563
Re: RFR: 8017234: Hotspot should stop using mapfiles [v5]
On Thu, 22 Feb 2024 16:15:23 GMT, Magnus Ihse Bursie wrote: >> **Summary:** Finally get rid of the mapfiles in Hotspot, and replace it with >> compiler options and `JNIEXPORT` on all platforms. >> >> The bug that this PR solves, >> [JDK-8017234](https://bugs.openjdk.org/browse/JDK-8017234), was created in >> 2013. Even back then the use of mapfiles in Hotspot was dated, so this is >> really good riddance with old rubbish. >> >> This code touches on central but not well understood parts of the Hotspot >> dynamic library, which has contributed to why this bug has stayed unresolved >> for so long. I will need to explain this fix in more detail than usually >> necessary. (Please bare with me if this gets long.) I also anticipate that >> not all solutions that I've picked will be accepted, and we'll have to >> discuss how to proceed. I think it is better to have actual concrete code to >> discuss around, rather than starting by an abstract discussion. To keep this >> description short, I will post the discussion as a comment to the PR. >> >> I have run this PR through tier 1-3 in our CI system. I have also carefully >> checked how the resulting dynamic library differs with this patch (not much; >> see discussion below). For build system changes, this is often the most >> relevant metric. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Use #pragma instead of HIDDEN define Not a review but I did want to mention that I proved to myself that your addition of `.hidden` for the assembly file symbols is the right thing to do. In particular, when we compile the .cpp files with the `-fvisibility=hidden` flag that's exactly what g++ does in the background. Just a thought: Perhaps worth introducing some kind of macro that captures defining a symbol in assembly code (something that does `.globl` + `.hidden` on Linux and `.private_extern` on `__APPLE__`? Also, separate from this change: perhaps worth turning on whitespace checking (no tabs) for .S files? - PR Comment: https://git.openjdk.org/jdk/pull/17955#issuecomment-1960276436
Re: RFR: 8326371: [BACKOUT] Clean up NativeCompilation.gmk and its newly created parts
On Wed, 21 Feb 2024 00:59:18 GMT, David Holmes wrote: > Revert "8325963: Clean up NativeCompilation.gmk and its newly created parts" > > This reverts commit 1bd91cdebee1e9ec78ecf185529923eef40ff89c due to code > signing failures on macOS. > > Thanks Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17938#pullrequestreview-1891863168
Integrated: 8325800: Drop unused cups declaration from Oracle build configuration
On Tue, 13 Feb 2024 20:47:01 GMT, Mikael Vidstedt wrote: > The cups dependency was used for Solaris builds, and has been unused since > [JDK-8244224](https://bugs.openjdk.org/browse/JDK-8244224). > > Testing: tier1 This pull request has now been integrated. Changeset: 8765b176 Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/8765b176f97dbf334836f0aa6acd921d114304a9 Stats: 7 lines in 1 file changed: 0 ins; 6 del; 1 mod 8325800: Drop unused cups declaration from Oracle build configuration Reviewed-by: erikj - PR: https://git.openjdk.org/jdk/pull/17837
Re: RFR: 8325800: Drop unused cups declaration from Oracle build configuration
On Tue, 13 Feb 2024 20:47:01 GMT, Mikael Vidstedt wrote: > The cups dependency was used for Solaris builds, and has been unused since > [JDK-8244224](https://bugs.openjdk.org/browse/JDK-8244224). > > Testing: tier1 Thank you for the review! - PR Comment: https://git.openjdk.org/jdk/pull/17837#issuecomment-1942580652
RFR: 8325800: Drop unused cups declaration from Oracle build configuration
The cups dependency was used for Solaris builds, and has been unused since [JDK-8244224](https://bugs.openjdk.org/browse/JDK-8244224). Testing: tier1 - Commit messages: - 8325800: Drop unused cups declaration from Oracle build configuration Changes: https://git.openjdk.org/jdk/pull/17837/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17837=00 Issue: https://bugs.openjdk.org/browse/JDK-8325800 Stats: 7 lines in 1 file changed: 0 ins; 6 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17837.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17837/head:pull/17837 PR: https://git.openjdk.org/jdk/pull/17837
Integrated: 8325570: Update to Graphviz 9.0.0
On Fri, 9 Feb 2024 19:02:13 GMT, Mikael Vidstedt wrote:
> Graphviz (aka. dotty/dot) is used when building "full" docs, and in
> particular for creating various module graph images (.svg). This change
> upgrades the Graphviz version used to 9.0.0 (latest).
>
> In particular, the change:
>
> * Updates the createGraphvizBundle.sh script (currently broken) to build
> graphviz from source
> * Updates doc/building.{md,html} to reflect the role of Graphviz and Pandoc
> in the build
> * The version of of the graphviz dependency used when building at Oracle (in
> jib-profiles.js)
>
> Since, in addition to the changes in this PR itself, the exact version of
> Graphviz has an effect on the generated images, I have uploaded the docs
> generated by graphviz 9.0.0 here:
> https://cr.openjdk.org/~mikael/graphviz-9.0.0/cmp-v1/docs/api/. For baseline
> the latest jdk23 docs, which uses graphviz 2.38.0, should do the trick:
> https://download.java.net/java/early_access/jdk23/docs/api/.
>
> For example, picking a random .svg file:
>
> baseline (graphviz 2.38.0):
> https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/Signature/RefTypeSig-sealed-graph.svg
> new (graphviz 9.0.0):
> https://cr.openjdk.org/~mikael/graphviz-9.0.0/cmp-v1/docs/api/java.base/java/lang/classfile/Signature/RefTypeSig-sealed-graph.svg
>
>
> Testing: tier1, manual inspection of a few of the generated .svg files
>
> As far as I can tell there are only very minor differences between the old
> (2.38.0) and new (9.0.0) .svg files. In particular, it seems like the new
> graphs are ever so slightly (5-10% or so) larger, but otherwise appear to be
> identical.
This pull request has now been integrated.
Changeset: 7c697123
Author:Mikael Vidstedt
URL:
https://git.openjdk.org/jdk/commit/7c6971239dd9af2a62aefb1163328c66c4507ef1
Stats: 123 lines in 4 files changed: 86 ins; 1 del; 36 mod
8325570: Update to Graphviz 9.0.0
Reviewed-by: erikj, pminborg, ihse, mchung, iris
-
PR: https://git.openjdk.org/jdk/pull/17794
Re: RFR: 8325570: Update to Graphviz 9.0.0
On Fri, 9 Feb 2024 19:02:13 GMT, Mikael Vidstedt wrote:
> Graphviz (aka. dotty/dot) is used when building "full" docs, and in
> particular for creating various module graph images (.svg). This change
> upgrades the Graphviz version used to 9.0.0 (latest).
>
> In particular, the change:
>
> * Updates the createGraphvizBundle.sh script (currently broken) to build
> graphviz from source
> * Updates doc/building.{md,html} to reflect the role of Graphviz and Pandoc
> in the build
> * The version of of the graphviz dependency used when building at Oracle (in
> jib-profiles.js)
>
> Since, in addition to the changes in this PR itself, the exact version of
> Graphviz has an effect on the generated images, I have uploaded the docs
> generated by graphviz 9.0.0 here:
> https://cr.openjdk.org/~mikael/graphviz-9.0.0/cmp-v1/docs/api/. For baseline
> the latest jdk23 docs, which uses graphviz 2.38.0, should do the trick:
> https://download.java.net/java/early_access/jdk23/docs/api/.
>
> For example, picking a random .svg file:
>
> baseline (graphviz 2.38.0):
> https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/Signature/RefTypeSig-sealed-graph.svg
> new (graphviz 9.0.0):
> https://cr.openjdk.org/~mikael/graphviz-9.0.0/cmp-v1/docs/api/java.base/java/lang/classfile/Signature/RefTypeSig-sealed-graph.svg
>
>
> Testing: tier1, manual inspection of a few of the generated .svg files
>
> As far as I can tell there are only very minor differences between the old
> (2.38.0) and new (9.0.0) .svg files. In particular, it seems like the new
> graphs are ever so slightly (5-10% or so) larger, but otherwise appear to be
> identical.
Thank you for all the reviews!
-
PR Comment: https://git.openjdk.org/jdk/pull/17794#issuecomment-1939516107
Re: RFR: 8325570: Update to Graphviz 9.0.0
On Fri, 9 Feb 2024 19:32:01 GMT, Erik Joelsson wrote: > Quick question, since configure scans for dot, will platform-specific pandoc > (like Windows pandoc) lead to different image outputs? AFAIU pandoc is not involved in the generation of the .svg images. The .dot files are generated by the taglet build tool (https://github.com/openjdk/jdk/tree/master/make/jdk/src/classes/build/tools/taglet) and graphviz (dot) is used to convert the .dot files to .svg images. - PR Comment: https://git.openjdk.org/jdk/pull/17794#issuecomment-1936491417
RFR: 8325570: Update to Graphviz 9.0.0
Graphviz (aka. dotty/dot) is used when building "full" docs, and in particular
for creating various module graph images (.svg). This change upgrades the
Graphviz version used to 9.0.0 (latest).
In particular, the change:
* Updates the createGraphvizBundle.sh script (currently broken) to build
graphviz from source
* Updates doc/building.{md,html} to reflect the role of Graphviz and Pandoc in
the build
* The version of of the graphviz dependency used when building at Oracle (in
jib-profiles.js)
Since, in addition to the changes in this PR itself, the exact version of
Graphviz has an effect on the generated images, I have uploaded the docs
generated by graphviz 9.0.0 here:
https://cr.openjdk.org/~mikael/graphviz-9.0.0/cmp-v1/docs/api/. For baseline
the latest jdk23 docs, which uses graphviz 2.38.0, should do the trick:
https://download.java.net/java/early_access/jdk23/docs/api/.
For example, picking a random .svg file:
baseline (graphviz 2.38.0):
https://download.java.net/java/early_access/jdk23/docs/api/java.base/java/lang/classfile/Signature/RefTypeSig-sealed-graph.svg
new (graphviz 9.0.0):
https://cr.openjdk.org/~mikael/graphviz-9.0.0/cmp-v1/docs/api/java.base/java/lang/classfile/Signature/RefTypeSig-sealed-graph.svg
Testing: tier1, manual inspection of a few of the generated .svg files
As far as I can tell there are only very minor differences between the old
(2.38.0) and new (9.0.0) .svg files. In particular, it seems like the new
graphs are ever so slightly (5-10% or so) larger, but otherwise appear to be
identical.
-
Commit messages:
- 8325570: Update to Graphviz 9.0.0
Changes: https://git.openjdk.org/jdk/pull/17794/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=17794=00
Issue: https://bugs.openjdk.org/browse/JDK-8325570
Stats: 123 lines in 4 files changed: 86 ins; 1 del; 36 mod
Patch: https://git.openjdk.org/jdk/pull/17794.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17794/head:pull/17794
PR: https://git.openjdk.org/jdk/pull/17794
Re: [jdk22] RFR: 8319547: Remove EA from the JDK 22 version string with first RC promotion
On Tue, 6 Feb 2024 00:14:40 GMT, Jesper Wilhelmsson wrote: > Removed ea from the version string Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk22/pull/106#pullrequestreview-1864071852
Re: RFR: 8274300: Address dsymutil warning by excluding platform specific files
On Tue, 9 Jan 2024 07:50:36 GMT, David Holmes wrote: > > We run dsymutils on the libraries (e.g. `libjvm.dylib`) to extract the > > debug symbols. > > So how does it know about empty files??? Does an empty src file create an > empty object file which in turns creates an empty section in the library? Right: (effectively) empty source file -> (effectively) empty object file -> (effectively) empty library. - PR Comment: https://git.openjdk.org/jdk/pull/17287#issuecomment-1883640395
Re: RFR: 8274300: Address dsymutil warning by excluding platform specific files
On Mon, 8 Jan 2024 02:46:03 GMT, David Holmes wrote: > Are we passing the wrong set of files to dsymutils? I'm guessing it is > actually passed a set of object files not source files. We run dsymutils on the libraries (e.g. `libjvm.dylib`) to extract the debug symbols. - PR Comment: https://git.openjdk.org/jdk/pull/17287#issuecomment-1881758241
Re: RFR: 8274300: Address dsymutil warning by excluding platform specific files
On Fri, 5 Jan 2024 21:05:49 GMT, Mikael Vidstedt wrote: > When building on macOS, dsymutil generates a warning for some files: > > `warning: no debug symbols in executable (-arch x86_64)` > > It turns out that the files in question have one thing in common: they're > empty. Specifically, the source files have include guards which, on macOS, > end up excluding all the actual source code, leaving an effectively empty > source file and hence empty object file. > > This change excludes the relevant files from the builds/platforms where > they're not needed. > > Testing: tier1,builds-tier[2-5] Thank you for the reviews. - PR Comment: https://git.openjdk.org/jdk/pull/17287#issuecomment-1881758863
Integrated: 8274300: Address dsymutil warning by excluding platform specific files
On Fri, 5 Jan 2024 21:05:49 GMT, Mikael Vidstedt wrote: > When building on macOS, dsymutil generates a warning for some files: > > `warning: no debug symbols in executable (-arch x86_64)` > > It turns out that the files in question have one thing in common: they're > empty. Specifically, the source files have include guards which, on macOS, > end up excluding all the actual source code, leaving an effectively empty > source file and hence empty object file. > > This change excludes the relevant files from the builds/platforms where > they're not needed. > > Testing: tier1,builds-tier[2-5] This pull request has now been integrated. Changeset: 8a4dc79e Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/8a4dc79e1a40e7115e2971af81623b6b0368f41c Stats: 11 lines in 2 files changed: 9 ins; 0 del; 2 mod 8274300: Address dsymutil warning by excluding platform specific files Reviewed-by: erikj - PR: https://git.openjdk.org/jdk/pull/17287
Re: RFR: 8274300: Address dsymutil warning by excluding platform specific files
On Mon, 8 Jan 2024 02:41:29 GMT, David Holmes wrote: >> When building on macOS, dsymutil generates a warning for some files: >> >> `warning: no debug symbols in executable (-arch x86_64)` >> >> It turns out that the files in question have one thing in common: they're >> empty. Specifically, the source files have include guards which, on macOS, >> end up excluding all the actual source code, leaving an effectively empty >> source file and hence empty object file. >> >> This change excludes the relevant files from the builds/platforms where >> they're not needed. >> >> Testing: tier1,builds-tier[2-5] > > make/test/JtregNativeHotspot.gmk line 862: > >> 860: ifeq ($(call And, $(call isTargetOs, linux) $(call isTargetCpu, >> aarch64)), false) >> 861: BUILD_HOTSPOT_JTREG_EXCLUDE += libTestSVEWithJNI.c >> 862: endif > > I would have put this inside the existing Linux section ie move to before > the endif at line 858, and then just check for aarch64. It's the other way around - this test is excluded for all platforms _but_ linux-aarch64 (note the `false`). - PR Review Comment: https://git.openjdk.org/jdk/pull/17287#discussion_r1445235539
RFR: 8274300: Address dsymutil warning by excluding platform specific files
When building on macOS, dsymutil generates a warning for some files: `warning: no debug symbols in executable (-arch x86_64)` It turns out that the files in question have one thing in common: they're empty. Specifically, the source files have include guards which, on macOS, end up excluding all the actual source code, leaving an effectively empty source file and hence empty object file. This change excludes the relevant files from the builds/platforms where they're not needed. Testing: tier1,builds-tier[2-5] - Commit messages: - Update copyright year - 8274300: Address dsymutil warning by excluding platform specific files Changes: https://git.openjdk.org/jdk/pull/17287/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17287=00 Issue: https://bugs.openjdk.org/browse/JDK-8274300 Stats: 11 lines in 2 files changed: 9 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17287.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17287/head:pull/17287 PR: https://git.openjdk.org/jdk/pull/17287
Integrated: 8322873: Duplicate -ljava -ljvm options for libinstrument
On Tue, 2 Jan 2024 21:44:04 GMT, Mikael Vidstedt wrote: > The libinstrument library is linked against libjava and libjvm, twice. This > is mostly harmless but Xcode 15.x generates a warning: > > Creating support/modules_libs/java.instrument/libinstrument.dylib from 12 > file(s) > ld: warning: ignoring duplicate libraries: '-ljava', '-ljvm' > > In particular, note that the `JDKLIB_LIBS` variable passed in to `LIBS` > already contains the -ljava -ljvm options. Digging through the history it > seems like this issue was introduced with > [JDK-8141290](https://bugs.openjdk.org/browse/JDK-8141290). > > Testing: tier1,builds-tier[2-5] This pull request has now been integrated. Changeset: 296c5b64 Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/296c5b645a2ecd8293a02233962c4a316a506c52 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 8322873: Duplicate -ljava -ljvm options for libinstrument Reviewed-by: dholmes, jwaters, erikj - PR: https://git.openjdk.org/jdk/pull/17230
Re: RFR: 8322873: Duplicate -ljava -ljvm options for libinstrument
On Tue, 2 Jan 2024 21:44:04 GMT, Mikael Vidstedt wrote: > The libinstrument library is linked against libjava and libjvm, twice. This > is mostly harmless but Xcode 15.x generates a warning: > > Creating support/modules_libs/java.instrument/libinstrument.dylib from 12 > file(s) > ld: warning: ignoring duplicate libraries: '-ljava', '-ljvm' > > In particular, note that the `JDKLIB_LIBS` variable passed in to `LIBS` > already contains the -ljava -ljvm options. Digging through the history it > seems like this issue was introduced with > [JDK-8141290](https://bugs.openjdk.org/browse/JDK-8141290). > > Testing: tier1,builds-tier[2-5] Thank you for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/17230#issuecomment-1875852891
RFR: 8322873: Duplicate -ljava -ljvm options for libinstrument
The libinstrument library is linked against libjava and libjvm, twice. This is mostly harmless but Xcode 15.x generates a warning: Creating support/modules_libs/java.instrument/libinstrument.dylib from 12 file(s) ld: warning: ignoring duplicate libraries: '-ljava', '-ljvm' In particular, note that the `JDKLIB_LIBS` variable passed in to `LIBS` already contains the -ljava -ljvm options. Digging through the history it seems like this issue was introduced with [JDK-8141290](https://bugs.openjdk.org/browse/JDK-8141290). Testing: tier1,builds-tier[2-5] (in progress) - Commit messages: - 8322873: Duplicate -ljava -ljvm options for libinstrument Changes: https://git.openjdk.org/jdk/pull/17230/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17230=00 Issue: https://bugs.openjdk.org/browse/JDK-8322873 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17230.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17230/head:pull/17230 PR: https://git.openjdk.org/jdk/pull/17230
Re: RFR: 8320942: Only set openjdk-target when cross compiling linux-aarch64
On Wed, 29 Nov 2023 02:25:20 GMT, Mikael Vidstedt wrote: > When building linux-aarch64 at Oracle using jib, > --openjdk-target=aarch64-linux-gnu is always specified regardless of if > building natively or cross compiling (on linux-x64). Among other things this > has the (harmless) effect of tricking configure into thinking that it's cross > compiling even for native builds: > > checking whether we are cross compiling... yes > > The reason is that the target value (aarch64-linux-gnu) doesn't match what > config.guess returns (aarch64-unknown-linux-gnu). An explicit target is only > needed when cross compiling and should not be specified for native builds. Thank you for the reviews! Since the question of what exactly constitutes cross compilation is orthogonal to this change I'll go ahead and integrate it. - PR Comment: https://git.openjdk.org/jdk/pull/16873#issuecomment-1832522525
Integrated: 8320942: Only set openjdk-target when cross compiling linux-aarch64
On Wed, 29 Nov 2023 02:25:20 GMT, Mikael Vidstedt wrote: > When building linux-aarch64 at Oracle using jib, > --openjdk-target=aarch64-linux-gnu is always specified regardless of if > building natively or cross compiling (on linux-x64). Among other things this > has the (harmless) effect of tricking configure into thinking that it's cross > compiling even for native builds: > > checking whether we are cross compiling... yes > > The reason is that the target value (aarch64-linux-gnu) doesn't match what > config.guess returns (aarch64-unknown-linux-gnu). An explicit target is only > needed when cross compiling and should not be specified for native builds. This pull request has now been integrated. Changeset: 454b1165 Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/454b11653c9e6718ee45233851e714a896013ec8 Stats: 5 lines in 1 file changed: 2 ins; 2 del; 1 mod 8320942: Only set openjdk-target when cross compiling linux-aarch64 Reviewed-by: ihse, erikj - PR: https://git.openjdk.org/jdk/pull/16873
RFR: 8320942: Only set openjdk-target when cross compiling linux-aarch64
When building linux-aarch64 at Oracle using jib, --openjdk-target=aarch64-linux-gnu is always specified regardless of if building natively or cross compiling (on linux-x64). Among other things this has the (harmless) effect of tricking configure into thinking that it's cross compiling even for native builds: checking whether we are cross compiling... yes The reason is that the target value (aarch64-linux-gnu) doesn't match what config.guess returns (aarch64-unknown-linux-gnu). An explicit target is only needed when cross compiling and should not be specified for native builds. - Commit messages: - 8320942: Only set openjdk-target when cross compiling Changes: https://git.openjdk.org/jdk/pull/16873/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16873=00 Issue: https://bugs.openjdk.org/browse/JDK-8320942 Stats: 5 lines in 1 file changed: 2 ins; 2 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16873.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16873/head:pull/16873 PR: https://git.openjdk.org/jdk/pull/16873
Re: RFR: JDK-8320932: [BACKOUT] dsymutil command leaves around temporary directories
On Tue, 28 Nov 2023 21:48:13 GMT, Erik Joelsson wrote: > This is a backout of > [JDK-8320863](https://bugs.openjdk.org/browse/JDK-8320863). That fix only > works on a very limited selection of Xcode versions, and is causing the build > to fail on other versions, including at least 12, 13 and 15. Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/16864#pullrequestreview-1754127260
Integrated: 8320212: Disable GCC stringop-overflow warning for affected files
On Fri, 17 Nov 2023 00:31:36 GMT, Mikael Vidstedt wrote: > In JDK-8319818 the stringop-overflow warnings were disabled for linux-aarch64 > (fastdebug). With the changes in JDK-8319883 additional stringop-overflow > warnings are produced with GCC 13.2.0, this time for linux-x64-zero > (fastdebug). The warnings are related to GCC thinking JavaThread:current (and > Thread::current) may return nullptr where in fact they can't. I tried several > ways to convince GCC about this fact but in the end failed. > > This change disables the warning for the affected files (only). I'm not in > love with that solution but I've run out of ideas at this point. An > alternative would be to disable the warning globally, which has its own set > of pros and cons. This pull request has now been integrated. Changeset: a1e7a302 Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/a1e7a302c8a3d7a1069659653042476b20becabe Stats: 11 lines in 1 file changed: 4 ins; 7 del; 0 mod 8320212: Disable GCC stringop-overflow warning for affected files Reviewed-by: ihse, dcubed - PR: https://git.openjdk.org/jdk/pull/16702
Re: RFR: 8320212: Disable GCC stringop-overflow warning for affected files
On Fri, 17 Nov 2023 00:31:36 GMT, Mikael Vidstedt wrote: > In JDK-8319818 the stringop-overflow warnings were disabled for linux-aarch64 > (fastdebug). With the changes in JDK-8319883 additional stringop-overflow > warnings are produced with GCC 13.2.0, this time for linux-x64-zero > (fastdebug). The warnings are related to GCC thinking JavaThread:current (and > Thread::current) may return nullptr where in fact they can't. I tried several > ways to convince GCC about this fact but in the end failed. > > This change disables the warning for the affected files (only). I'm not in > love with that solution but I've run out of ideas at this point. An > alternative would be to disable the warning globally, which has its own set > of pros and cons. Thank you for the reviews. - PR Comment: https://git.openjdk.org/jdk/pull/16702#issuecomment-1817138681
Re: RFR: 8320212: Disable GCC stringop-overflow warning for affected files
On Fri, 17 Nov 2023 15:36:29 GMT, Magnus Ihse Bursie wrote: > Looks okay from a build perspective. In the end, I guess some hotspot > developers need to weigh in if this is acceptable. > > I presume you have tried to analyze what makes the Thread:current access > different in these files, so that gcc only complains at these instances? Indeed, I spent quite a lot of time trying to understand what makes these specific files (cases/places) special but in the end had to give up. If there is a pattern I can't spot it. - PR Comment: https://git.openjdk.org/jdk/pull/16702#issuecomment-1816867229
RFR: 8320212: Disable GCC stringop-overflow warning for affected files
In JDK-8319818 the stringop-overflow warnings were disabled for linux-aarch64 (fastdebug). With the changes in JDK-8319883 additional stringop-overflow warnings are produced with GCC 13.2.0, this time for linux-x64-zero (fastdebug). The warnings are related to GCC thinking JavaThread:current (and Thread::current) may return nullptr where in fact they can't. I tried several ways to convince GCC about this fact but in the end failed. This change disables the warning for the affected files (only). I'm not in love with that solution but I've run out of ideas at this point. An alternative would be to disable the warning globally, which has its own set of pros and cons. - Commit messages: - 8320212: Disable GCC stringop-overflow warning for affected files Changes: https://git.openjdk.org/jdk/pull/16702/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16702=00 Issue: https://bugs.openjdk.org/browse/JDK-8320212 Stats: 11 lines in 1 file changed: 4 ins; 7 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16702.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16702/head:pull/16702 PR: https://git.openjdk.org/jdk/pull/16702
Re: RFR: 8320258: Refresh building.md
On Thu, 16 Nov 2023 17:40:36 GMT, Magnus Ihse Bursie wrote: > The contents of the build README has been poorly kept up to date in places. > With the move to Github, the need to have markdown syntax that looks good on > Github has become apparent. The entire document has been in need for a while > of a comprehensive oversight to be made more consistent in language and > formatting. > > Here is a summary of the most important changes: > > * The entire section on cross-compiling have been restructured to be more > logical, and some parts have been rewritten, so it reads more coherent. > * A section on "make doctor" has been added > * The fixpath.sh script has been explained more in detail (and references to > the old fixpath binary removed) > * Information about MSYS2 and WSL has been updated > * References to how things were done in the past, like the hg forest have > been removed > * Version numbers have been updated where needed > * Some specific paragraphs about e.g. boot JDK detection have been clarified > * Example output have been updated > * Remaining references to `run-test` have been updated to just `test` > * Internal links have been fixed in a few places > * A few remaining places that confuses "OpenJDK" with the "JDK" have been > fixed > * Markdown formatting has been improved in some places > * The entire file has gotten more normalized markdown formatting (fix bullet > list indentation, add empty lines surrounding the code block markers, etc) > > Note that this PR will can possibly create a merge conflict with 8317357 and > 8264425; my intention is to integrate these two first and then resolve any > conflicts in this PR. doc/building.md line 42: > 40: > 41: If you just want to use the JDK and not build it yourself, this document > is not > 42: for you. See for instance [OpenJDK > installation](http://openjdk.org/install) Switch to https? Same for other uses of http:// later in the file? - PR Review Comment: https://git.openjdk.org/jdk/pull/16696#discussion_r1396333621
Integrated: 8319570: Change to GCC 13.2.0 for building on Linux at Oracle
On Tue, 7 Nov 2023 23:37:06 GMT, Mikael Vidstedt wrote: > Oracle is updating the version of GCC for building the JDK on Linux to 13.2.0. > > > **Note: The changes discussed below were broken out into a separate change > and integrated through #16584.** > > Apart from the "obvious" changes, I'll add some color to the CompileJvm.gmk > changes. In particular, I ran into two different types of new warnings with > GCC 13.2.0: > > 1. linux-aarch64-debug + stringop-overflow > > `src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: > 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 > bytes into a region of size 0 overflows the destination > [-Werror=stringop-overflow=]` > > Only reproduces with fastdebug on linux-aarch64. I tried to understand why > the warning is generated and how the code could be fixed but eventually had > to give up.. I ended up disabling the warning for linux-aarch64-debug > specifically but open to feedback and other alternatives. > > 2. linux + zero + dangling-pointer > > > `src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of > local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' > [-Werror=dangling-pointer=]` > > The linux/zero build generates lots and lots of dangling pointer warnings. As > with the first warning I tried to understand why but also gave up in the end. > Like the first warning I disabled it instead, for zero builds. Again > appreciating feedback/suggestions. This pull request has now been integrated. Changeset: 1802cb56 Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/1802cb566e956febebc181da26a666bea4942e87 Stats: 24 lines in 4 files changed: 13 ins; 0 del; 11 mod 8319570: Change to GCC 13.2.0 for building on Linux at Oracle Reviewed-by: ihse, dholmes - PR: https://git.openjdk.org/jdk/pull/16550
Re: RFR: 8319570: Change to GCC 13.2.0 for building on Linux at Oracle [v4]
On Mon, 13 Nov 2023 17:55:12 GMT, Mikael Vidstedt wrote: >> Oracle is updating the version of GCC for building the JDK on Linux to >> 13.2.0. >> >> >> **Note: The changes discussed below were broken out into a separate change >> and integrated through #16584.** >> >> Apart from the "obvious" changes, I'll add some color to the CompileJvm.gmk >> changes. In particular, I ran into two different types of new warnings with >> GCC 13.2.0: >> >> 1. linux-aarch64-debug + stringop-overflow >> >> `src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: >> 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 >> bytes into a region of size 0 overflows the destination >> [-Werror=stringop-overflow=]` >> >> Only reproduces with fastdebug on linux-aarch64. I tried to understand why >> the warning is generated and how the code could be fixed but eventually had >> to give up.. I ended up disabling the warning for linux-aarch64-debug >> specifically but open to feedback and other alternatives. >> >> 2. linux + zero + dangling-pointer >> >> >> `src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of >> local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' >> [-Werror=dangling-pointer=]` >> >> The linux/zero build generates lots and lots of dangling pointer warnings. >> As with the first warning I tried to understand why but also gave up in the >> end. Like the first warning I disabled it instead, for zero builds. Again >> appreciating feedback/suggestions. > > Mikael Vidstedt has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains four commits: > > - Merge branch 'master' into 8319570-gcc-13.2.0 > - Update copyright years > - Move ASSERT ResourceMark constructor to resourceArea.cpp to avoid dangling > pointer warning with zero > - 8319570: Change to GCC 13.2.0 for building on Linux at Oracle Thank you for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/16550#issuecomment-1809251421
Re: RFR: 8319570: Change to GCC 13.2.0 for building on Linux at Oracle [v4]
> Oracle is updating the version of GCC for building the JDK on Linux to 13.2.0. > > Apart from the "obvious" changes, I'll add some color to the CompileJvm.gmk > changes. In particular, I ran into two different types of new warnings with > GCC 13.2.0: > > 1. linux-aarch64-debug + stringop-overflow > > `src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: > 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 > bytes into a region of size 0 overflows the destination > [-Werror=stringop-overflow=]` > > Only reproduces with fastdebug on linux-aarch64. I tried to understand why > the warning is generated and how the code could be fixed but eventually had > to give up.. I ended up disabling the warning for linux-aarch64-debug > specifically but open to feedback and other alternatives. > > 2. linux + zero + dangling-pointer > > > `src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of > local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' > [-Werror=dangling-pointer=]` > > The linux/zero build generates lots and lots of dangling pointer warnings. As > with the first warning I tried to understand why but also gave up in the end. > Like the first warning I disabled it instead, for zero builds. Again > appreciating feedback/suggestions. Mikael Vidstedt has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits: - Merge branch 'master' into 8319570-gcc-13.2.0 - Update copyright years - Move ASSERT ResourceMark constructor to resourceArea.cpp to avoid dangling pointer warning with zero - 8319570: Change to GCC 13.2.0 for building on Linux at Oracle - Changes: https://git.openjdk.org/jdk/pull/16550/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16550=03 Stats: 24 lines in 4 files changed: 13 ins; 0 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/16550.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16550/head:pull/16550 PR: https://git.openjdk.org/jdk/pull/16550
Integrated: 8319818: Address GCC 13.2.0 warnings (stringop-overflow and dangling-pointer)
On Thu, 9 Nov 2023 16:07:53 GMT, Mikael Vidstedt wrote: > This PR is splitting out the GCC 13.2.0 warning related changes from #16550, > excluding the Oracle/devkit parts, for clarity and to make potential > backports more straightforward. > > GCC 13.2.0 generates two new warnings: > > * linux-aarch64-debug > > src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: > 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 > bytes into a region of size 0 overflows the destination > [-Werror=stringop-overflow=] > > I did not find any way to adjust the code to avoid this warning, so I instead > chose to disable it in `CompileJvm.gmk` for linux-aarch64-(fast)debug only. > > * linux-zero > > src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of > local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' > [-Werror=dangling-pointer=] > > Lots and lots of warnings related to ResourceMark. Thanks to @stefank for > suggesting moving the ASSERT implementation of the ResourceMark constructor > to the .cpp file. With that change there's no need to explicitly disable the > warning. This pull request has now been integrated. Changeset: c0507af5 Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/c0507af5a4d867940b3aee1ac0fc8188b5536825 Stats: 32 lines in 3 files changed: 19 ins; 10 del; 3 mod 8319818: Address GCC 13.2.0 warnings (stringop-overflow and dangling-pointer) Reviewed-by: ihse, dholmes - PR: https://git.openjdk.org/jdk/pull/16584
Re: RFR: 8319818: Address GCC 13.2.0 warnings (stringop-overflow and dangling-pointer) [v2]
On Fri, 10 Nov 2023 18:08:17 GMT, Mikael Vidstedt wrote: >> This PR is splitting out the GCC 13.2.0 warning related changes from #16550, >> excluding the Oracle/devkit parts, for clarity and to make potential >> backports more straightforward. >> >> GCC 13.2.0 generates two new warnings: >> >> * linux-aarch64-debug >> >> src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: >> 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 >> bytes into a region of size 0 overflows the destination >> [-Werror=stringop-overflow=] >> >> I did not find any way to adjust the code to avoid this warning, so I >> instead chose to disable it in `CompileJvm.gmk` for >> linux-aarch64-(fast)debug only. >> >> * linux-zero >> >> src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of >> local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' >> [-Werror=dangling-pointer=] >> >> Lots and lots of warnings related to ResourceMark. Thanks to @stefank for >> suggesting moving the ASSERT implementation of the ResourceMark constructor >> to the .cpp file. With that change there's no need to explicitly disable the >> warning. > > Mikael Vidstedt has updated the pull request incrementally with one > additional commit since the last revision: > > Add comment for stringop-overflow warning > > Co-authored-by: Hao Sun Thank you for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/16584#issuecomment-1808666187
Re: RFR: 8319818: Address GCC 13.2.0 warnings (stringop-overflow and dangling-pointer) [v2]
On Fri, 10 Nov 2023 01:25:32 GMT, Hao Sun wrote: >> Mikael Vidstedt has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add comment for stringop-overflow warning >> >> Co-authored-by: Hao Sun > > make/hotspot/lib/CompileJvm.gmk line 93: > >> 91: ifeq ($(DEBUG_LEVEL), fastdebug) >> 92: ifeq ($(call And, $(call isTargetOs, linux) $(call isTargetCpu, >> aarch64)), true) >> 93: DISABLED_WARNINGS_gcc += stringop-overflow > > Do you think it would be better if we add some comments here? > Suggestion: > > # False positive warnings for atomic_linux_aarch64.hpp on GCC >= 13 > DISABLED_WARNINGS_gcc += stringop-overflow I was also surprised to see the `dangling-pointer` warning just disappear, but I guess I'll take the win. Thank you for the code suggestion. We have not been consistent when it comes to (not) adding comments to the disabled warnings but perhaps this is a good time to start. - PR Review Comment: https://git.openjdk.org/jdk/pull/16584#discussion_r1389718228
Re: RFR: 8319818: Address GCC 13.2.0 warnings (stringop-overflow and dangling-pointer) [v2]
> This PR is splitting out the GCC 13.2.0 warning related changes from #16550, > excluding the Oracle/devkit parts, for clarity and to make potential > backports more straightforward. > > GCC 13.2.0 generates two new warnings: > > * linux-aarch64-debug > > src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: > 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 > bytes into a region of size 0 overflows the destination > [-Werror=stringop-overflow=] > > I did not find any way to adjust the code to avoid this warning, so I instead > chose to disable it in `CompileJvm.gmk` for linux-aarch64-(fast)debug only. > > * linux-zero > > src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of > local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' > [-Werror=dangling-pointer=] > > Lots and lots of warnings related to ResourceMark. Thanks to @stefank for > suggesting moving the ASSERT implementation of the ResourceMark constructor > to the .cpp file. With that change there's no need to explicitly disable the > warning. Mikael Vidstedt has updated the pull request incrementally with one additional commit since the last revision: Add comment for stringop-overflow warning Co-authored-by: Hao Sun - Changes: - all: https://git.openjdk.org/jdk/pull/16584/files - new: https://git.openjdk.org/jdk/pull/16584/files/47ace39f..e2baad09 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16584=01 - incr: https://webrevs.openjdk.org/?repo=jdk=16584=00-01 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16584.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16584/head:pull/16584 PR: https://git.openjdk.org/jdk/pull/16584
Re: RFR: 8319818: Address GCC 13.2.0 warnings (stringop-overflow and dangling-pointer)
On Thu, 9 Nov 2023 16:07:53 GMT, Mikael Vidstedt wrote: > This PR is splitting out the GCC 13.2.0 warning related changes from #16550, > excluding the Oracle/devkit parts, for clarity and to make potential > backports more straightforward. > > GCC 13.2.0 generates two new warnings: > > * linux-aarch64-debug > > src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: > 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 > bytes into a region of size 0 overflows the destination > [-Werror=stringop-overflow=] > > I did not find any way to adjust the code to avoid this warning, so I instead > chose to disable it in `CompileJvm.gmk` for linux-aarch64-(fast)debug only. > > * linux-zero > > src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of > local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' > [-Werror=dangling-pointer=] > > Lots and lots of warnings related to ResourceMark. Thanks to @stefank for > suggesting moving the ASSERT implementation of the ResourceMark constructor > to the .cpp file. With that change there's no need to explicitly disable the > warning. My plan is the rest of #16550 - devkit creation logic and the actual (Oracle) bump. - PR Comment: https://git.openjdk.org/jdk/pull/16584#issuecomment-1804426349
Re: RFR: 8319818: Address GCC 13.2.0 warnings (stringop-overflow and dangling-pointer)
On Thu, 9 Nov 2023 16:07:53 GMT, Mikael Vidstedt wrote: > This PR is splitting out the GCC 13.2.0 warning related changes from #16550, > excluding the Oracle/devkit parts, for clarity and to make potential > backports more straightforward. > > GCC 13.2.0 generates two new warnings: > > * linux-aarch64-debug > > src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: > 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 > bytes into a region of size 0 overflows the destination > [-Werror=stringop-overflow=] > > I did not find any way to adjust the code to avoid this warning, so I instead > chose to disable it in `CompileJvm.gmk` for linux-aarch64-(fast)debug only. > > * linux-zero > > src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of > local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' > [-Werror=dangling-pointer=] > > Lots and lots of warnings related to ResourceMark. Thanks to @stefank for > suggesting moving the ASSERT implementation of the ResourceMark constructor > to the .cpp file. With that change there's no need to explicitly disable the > warning. I respectfully disagree. These changes are all needed for GCC 13.2.0 so can and should go hand in hand. - PR Comment: https://git.openjdk.org/jdk/pull/16584#issuecomment-1804326193
Re: RFR: 8319570: Change to GCC 13.2.0 for building on Linux at Oracle [v3]
On Thu, 9 Nov 2023 05:20:18 GMT, David Holmes wrote: >> Mikael Vidstedt has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update copyright years > > src/hotspot/share/memory/resourceArea.cpp line 1: > >> 1: /* > > @vidmik I hate to be a pain but could we separate out the source code change > from this nominal build/configuration change please. It will allow for easier > backporting. Good idea - I created JDK-8319818 / #16584 to cover (only) the warnings. - PR Review Comment: https://git.openjdk.org/jdk/pull/16550#discussion_r1388241495
RFR: 8319818: Address GCC 13.2.0 warnings (stringop-overflow and dangling-pointer)
This PR is splitting out the GCC 13.2.0 warning related changes from #16550, excluding the Oracle/devkit parts, for clarity and to make potential backports more straightforward. GCC 13.2.0 generates two new warnings: * linux-aarch64-debug src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=] I did not find any way to adjust the code to avoid this warning, so I instead chose to disable it in `CompileJvm.gmk` for linux-aarch64-(fast)debug only. * linux-zero src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' [-Werror=dangling-pointer=] Lots and lots of warnings related to ResourceMark. Thanks to @stefank for suggesting moving the ASSERT implementation of the ResourceMark constructor to the .cpp file. With that change there's no need to explicitly disable the warning. - Commit messages: - 8319818: Address GCC 13.2.0 warnings (stringop-overflow and dangling-pointer) Changes: https://git.openjdk.org/jdk/pull/16584/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16584=00 Issue: https://bugs.openjdk.org/browse/JDK-8319818 Stats: 31 lines in 3 files changed: 18 ins; 10 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16584.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16584/head:pull/16584 PR: https://git.openjdk.org/jdk/pull/16584
Re: RFR: 8319570: Change to GCC 13.2.0 for building on Linux at Oracle [v3]
> Oracle is updating the version of GCC for building the JDK on Linux to 13.2.0. > > Apart from the "obvious" changes, I'll add some color to the CompileJvm.gmk > changes. In particular, I ran into two different types of new warnings with > GCC 13.2.0: > > 1. linux-aarch64-debug + stringop-overflow > > `src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: > 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 > bytes into a region of size 0 overflows the destination > [-Werror=stringop-overflow=]` > > Only reproduces with fastdebug on linux-aarch64. I tried to understand why > the warning is generated and how the code could be fixed but eventually had > to give up.. I ended up disabling the warning for linux-aarch64-debug > specifically but open to feedback and other alternatives. > > 2. linux + zero + dangling-pointer > > > `src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of > local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' > [-Werror=dangling-pointer=]` > > The linux/zero build generates lots and lots of dangling pointer warnings. As > with the first warning I tried to understand why but also gave up in the end. > Like the first warning I disabled it instead, for zero builds. Again > appreciating feedback/suggestions. Mikael Vidstedt has updated the pull request incrementally with one additional commit since the last revision: Update copyright years - Changes: - all: https://git.openjdk.org/jdk/pull/16550/files - new: https://git.openjdk.org/jdk/pull/16550/files/19a9a1b7..2ad37c20 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16550=02 - incr: https://webrevs.openjdk.org/?repo=jdk=16550=01-02 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/16550.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16550/head:pull/16550 PR: https://git.openjdk.org/jdk/pull/16550
Re: RFR: 8319570: Change to GCC 13.2.0 for building on Linux at Oracle [v2]
On Wed, 8 Nov 2023 21:30:24 GMT, Magnus Ihse Bursie wrote: >> FWIW I just followed the preexisting pattern in the same file, e.g. >> https://github.com/openjdk/jdk/blob/7d25f1c6cb770e21cfad8096c1637a24e65fab8c/make/hotspot/lib/CompileJvm.gmk#L1100. >> @magicus Let me know if you want me to change it. > >> [T]he And macro was actually added by you > > Oh.  Ohh... That was a bit stupid of me, then. :) But I just checked, I > found 4 cases of `$(call And)` vs 11 cases of using `+` in our makefiles. So > it is a clear overweight for the `+` syntax, but is not the sole pattern as I > thought. > > So yes, it is okay to keep it as it is. Leaving it as-is then. Thank you. - PR Review Comment: https://git.openjdk.org/jdk/pull/16550#discussion_r1387257535
Re: RFR: 8319570: Change to GCC 13.2.0 for building on Linux at Oracle [v2]
On Wed, 8 Nov 2023 22:10:20 GMT, Mikael Vidstedt wrote: >> Oracle is updating the version of GCC for building the JDK on Linux to >> 13.2.0. >> >> Apart from the "obvious" changes, I'll add some color to the CompileJvm.gmk >> changes. In particular, I ran into two different types of new warnings with >> GCC 13.2.0: >> >> 1. linux-aarch64-debug + stringop-overflow >> >> `src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: >> 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 >> bytes into a region of size 0 overflows the destination >> [-Werror=stringop-overflow=]` >> >> Only reproduces with fastdebug on linux-aarch64. I tried to understand why >> the warning is generated and how the code could be fixed but eventually had >> to give up.. I ended up disabling the warning for linux-aarch64-debug >> specifically but open to feedback and other alternatives. >> >> 2. linux + zero + dangling-pointer >> >> >> `src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of >> local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' >> [-Werror=dangling-pointer=]` >> >> The linux/zero build generates lots and lots of dangling pointer warnings. >> As with the first warning I tried to understand why but also gave up in the >> end. Like the first warning I disabled it instead, for zero builds. Again >> appreciating feedback/suggestions. > > Mikael Vidstedt has updated the pull request incrementally with one > additional commit since the last revision: > > Move ASSERT ResourceMark constructor to resourceArea.cpp to avoid dangling > pointer warning with zero > For `linux + zero + dangling-pointer`, your current solution is somehow > coarse-grained. Perhaps, we may use `PRAGMA_DANGLING_POINTER_IGNORED`. See > #13751 and #13789. @shqking Good feedback, thank you! I tried to use `PRAGMA_DANGLING_POINTER_IGNORED` to silence the warning but ran into issues using it in the header files - seems like there may be limitations to where pragma can be used and in particular it doesn't seem to have any effect in header files and/or inside of `#if`/#ifdef` blocks. That basically means that in order to fix this particular warning we would have to put the PRAGMAs in all the various places in the .cpp files instead, which turns out to be _lots_ of places. Not particularly appetizing. However, @stefank suggested another, better solution: move the relevant `ASSERT` specific `ResourceMark` constructor code to the `resourceArea.cpp` file instead. Since it's "just" used for `ASSERT` builds it's not really performance sensitive, so can't be that important to inline. I did exactly that and - lo and behold - I didn't even have to silence the warning in the .cpp file, it just works. I just pushed a change that moves the constructor implantation to `resourceArea.cpp` and removes the dangling pointer warning logic from `CompileJvm.gmk` again. Have a look and let me know what you think. - PR Comment: https://git.openjdk.org/jdk/pull/16550#issuecomment-1802763593
Re: RFR: 8319570: Change to GCC 13.2.0 for building on Linux at Oracle [v2]
> Oracle is updating the version of GCC for building the JDK on Linux to 13.2.0. > > Apart from the "obvious" changes, I'll add some color to the CompileJvm.gmk > changes. In particular, I ran into two different types of new warnings with > GCC 13.2.0: > > 1. linux-aarch64-debug + stringop-overflow > > `src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: > 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 > bytes into a region of size 0 overflows the destination > [-Werror=stringop-overflow=]` > > Only reproduces with fastdebug on linux-aarch64. I tried to understand why > the warning is generated and how the code could be fixed but eventually had > to give up.. I ended up disabling the warning for linux-aarch64-debug > specifically but open to feedback and other alternatives. > > 2. linux + zero + dangling-pointer > > > `src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of > local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' > [-Werror=dangling-pointer=]` > > The linux/zero build generates lots and lots of dangling pointer warnings. As > with the first warning I tried to understand why but also gave up in the end. > Like the first warning I disabled it instead, for zero builds. Again > appreciating feedback/suggestions. Mikael Vidstedt has updated the pull request incrementally with one additional commit since the last revision: Move ASSERT ResourceMark constructor to resourceArea.cpp to avoid dangling pointer warning with zero - Changes: - all: https://git.openjdk.org/jdk/pull/16550/files - new: https://git.openjdk.org/jdk/pull/16550/files/be29a987..19a9a1b7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16550=01 - incr: https://webrevs.openjdk.org/?repo=jdk=16550=00-01 Stats: 27 lines in 3 files changed: 12 ins; 14 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16550.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16550/head:pull/16550 PR: https://git.openjdk.org/jdk/pull/16550
Re: RFR: 8319570: Change to GCC 13.2.0 for building on Linux at Oracle
On Wed, 8 Nov 2023 13:24:21 GMT, Magnus Ihse Bursie wrote: >> Oracle is updating the version of GCC for building the JDK on Linux to >> 13.2.0. >> >> Apart from the "obvious" changes, I'll add some color to the CompileJvm.gmk >> changes. In particular, I ran into two different types of new warnings with >> GCC 13.2.0: >> >> 1. linux-aarch64-debug + stringop-overflow >> >> `src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: >> 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 >> bytes into a region of size 0 overflows the destination >> [-Werror=stringop-overflow=]` >> >> Only reproduces with fastdebug on linux-aarch64. I tried to understand why >> the warning is generated and how the code could be fixed but eventually had >> to give up.. I ended up disabling the warning for linux-aarch64-debug >> specifically but open to feedback and other alternatives. >> >> 2. linux + zero + dangling-pointer >> >> >> `src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of >> local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' >> [-Werror=dangling-pointer=]` >> >> The linux/zero build generates lots and lots of dangling pointer warnings. >> As with the first warning I tried to understand why but also gave up in the >> end. Like the first warning I disabled it instead, for zero builds. Again >> appreciating feedback/suggestions. > > make/hotspot/lib/CompileJvm.gmk line 92: > >> 90: >> 91: ifeq ($(DEBUG_LEVEL), fastdebug) >> 92: ifeq ($(call And, $(call isTargetOs, linux) $(call isTargetCpu, >> aarch64)), true) > > The idiomatic way we have expressed this in other places in the JDK build is: > Suggestion: > > ifeq ($(call isTargetOs, linux)+$(call isTargetCpu, aarch64), true+true) FWIW I just followed the preexisting pattern in the same file, e.g. https://github.com/openjdk/jdk/blob/7d25f1c6cb770e21cfad8096c1637a24e65fab8c/make/hotspot/lib/CompileJvm.gmk#L1100. @magicus Let me know if you want me to change it. - PR Review Comment: https://git.openjdk.org/jdk/pull/16550#discussion_r1386979331
RFR: 8319570: Change to GCC 13.2.0 for building on Linux at Oracle
Oracle is updating the version of GCC for building the JDK on Linux to 13.2.0. Apart from the "obvious" changes, I'll add some color to the CompileJvm.gmk changes. In particular, I ran into two different types of new warnings with GCC 13.2.0: 1. linux-aarch64-debug + stringop-overflow `src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=]` Only reproduces with fastdebug on linux-aarch64. I tried to understand why the warning is generated and how the code could be fixed but eventually had to give up.. I ended up disabling the warning for linux-aarch64-debug specifically but open to feedback and other alternatives. 2. linux + zero + dangling-pointer `src/hotspot/share/runtime/thread.hpp:579:77: error: storing the address of local variable 'rm' in '*_thr_current.Thread::_current_resource_mark' [-Werror=dangling-pointer=]` The linux/zero build generates lots and lots of dangling pointer warnings. As with the first warning I tried to understand why but also gave up in the end. Like the first warning I disabled it instead, for zero builds. Again appreciating feedback/suggestions. - Commit messages: - 8319570: Change to GCC 13.2.0 for building on Linux at Oracle Changes: https://git.openjdk.org/jdk/pull/16550/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16550=00 Issue: https://bugs.openjdk.org/browse/JDK-8319570 Stats: 34 lines in 5 files changed: 23 ins; 0 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/16550.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16550/head:pull/16550 PR: https://git.openjdk.org/jdk/pull/16550
Integrated: 8319573: Change to Visual Studio 17.6.5 for building on Windows at Oracle
On Mon, 6 Nov 2023 23:00:39 GMT, Mikael Vidstedt wrote: > Oracle is updating the version of Visual Studio for building the JDK on > Windows to Visual Studio 2022 17.6.5. This pull request has now been integrated. Changeset: 806529aa Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/806529aa77e1977360cd3885b595797bea98e920 Stats: 7 lines in 3 files changed: 0 ins; 0 del; 7 mod 8319573: Change to Visual Studio 17.6.5 for building on Windows at Oracle Reviewed-by: erikj - PR: https://git.openjdk.org/jdk/pull/16533
RFR: 8319573: Change to Visual Studio 17.6.5 for building on Windows at Oracle
Oracle is updating the version of Visual Studio for building the JDK on Windows to Visual Studio 2022 17.6.5. - Commit messages: - 8319573: Change to Visual Studio 17.6.5 for building on Windows at Oracle Changes: https://git.openjdk.org/jdk/pull/16533/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16533=00 Issue: https://bugs.openjdk.org/browse/JDK-8319573 Stats: 7 lines in 3 files changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/16533.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16533/head:pull/16533 PR: https://git.openjdk.org/jdk/pull/16533
Re: RFR: 8318481: linux-arm32 attribute warning for offset_of
On Wed, 18 Oct 2023 23:02:18 GMT, Mikael Vidstedt wrote: > The linux-arm32 build generates a somewhat spectacular number of warnings, > all for the same code - the offset_of macro in hotspot: > > > src/hotspot/share/utilities/globalDefinitions_gcc.hpp:144:40: warning: > requested alignment 16 is larger than 8 [-Wattributes] > alignas(16) char space[sizeof (klass)]; > > > I was hoping to find a simple fix for the code in question but I'm not sure > what that would look like. Until proven otherwise I propose that we disable > the warning for arm32. It was pointed out that this issue has already been reported and is not specific to arm32. I'm going to withdraw this PR and explore alternative solutions. - PR Comment: https://git.openjdk.org/jdk/pull/16256#issuecomment-1771613371
Withdrawn: 8318481: linux-arm32 attribute warning for offset_of
On Wed, 18 Oct 2023 23:02:18 GMT, Mikael Vidstedt wrote: > The linux-arm32 build generates a somewhat spectacular number of warnings, > all for the same code - the offset_of macro in hotspot: > > > src/hotspot/share/utilities/globalDefinitions_gcc.hpp:144:40: warning: > requested alignment 16 is larger than 8 [-Wattributes] > alignas(16) char space[sizeof (klass)]; > > > I was hoping to find a simple fix for the code in question but I'm not sure > what that would look like. Until proven otherwise I propose that we disable > the warning for arm32. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/16256
RFR: 8318481: linux-arm32 attribute warning for offset_of
The linux-arm32 build generates a somewhat spectacular number of warnings, all for the same code - the offset_of macro in hotspot: src/hotspot/share/utilities/globalDefinitions_gcc.hpp:144:40: warning: requested alignment 16 is larger than 8 [-Wattributes] alignas(16) char space[sizeof (klass)]; I was hoping to find a simple fix for the code in question but I'm not sure what that would look like. Until proven otherwise I propose that we disable the warning for arm32. - Commit messages: - 8318481: linux-arm32 attribute warning for offset_of Changes: https://git.openjdk.org/jdk/pull/16256/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16256=00 Issue: https://bugs.openjdk.org/browse/JDK-8318481 Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16256.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16256/head:pull/16256 PR: https://git.openjdk.org/jdk/pull/16256
Integrated: 8317970: Bump target macosx-x64 version to 11.00.00
On Wed, 11 Oct 2023 17:49:14 GMT, Mikael Vidstedt wrote: > macOS 10.x is no longer receiving updates - the most recent/last release was > 10.15.7 back in July of 2022. It's time to bump the target macOS version > (min/max) for macosx-x64. macOS 11.x is still receiving updates. > > This change updates the target version for macosx-x64 to 11.00.00, which is > the same version used for macosx-aarch64. This pull request has now been integrated. Changeset: 72c4dcbf Author: Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/72c4dcbfeefcd664f5e3175b24e395c1f36a05fd Stats: 22 lines in 3 files changed: 0 ins; 17 del; 5 mod 8317970: Bump target macosx-x64 version to 11.00.00 Reviewed-by: erikj, prr, ihse - PR: https://git.openjdk.org/jdk/pull/16155
Re: RFR: 8317970: Bump target macosx-x64 version to 11.00.00 [v4]
On Fri, 13 Oct 2023 18:01:44 GMT, Mikael Vidstedt wrote: >> macOS 10.x is no longer receiving updates - the most recent/last release was >> 10.15.7 back in July of 2022. It's time to bump the target macOS version >> (min/max) for macosx-x64. macOS 11.x is still receiving updates. >> >> This change updates the target version for macosx-x64 to 11.00.00, which is >> the same version used for macosx-aarch64. > > Mikael Vidstedt has updated the pull request incrementally with one > additional commit since the last revision: > > Drop redundant MACOSX_METAL_VERSION_MIN variable Thank you for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/16155#issuecomment-1762153757
Re: RFR: 8317970: Bump target macosx-x64 version to 11.00.00 [v4]
> macOS 10.x is no longer receiving updates - the most recent/last release was > 10.15.7 back in July of 2022. It's time to bump the target macOS version > (min/max) for macosx-x64. macOS 11.x is still receiving updates. > > This change updates the target version for macosx-x64 to 11.00.00, which is > the same version used for macosx-aarch64. Mikael Vidstedt has updated the pull request incrementally with one additional commit since the last revision: Drop redundant MACOSX_METAL_VERSION_MIN variable - Changes: - all: https://git.openjdk.org/jdk/pull/16155/files - new: https://git.openjdk.org/jdk/pull/16155/files/56a95f67..e0e6d4e9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16155=03 - incr: https://webrevs.openjdk.org/?repo=jdk=16155=02-03 Stats: 6 lines in 1 file changed: 0 ins; 5 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16155.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16155/head:pull/16155 PR: https://git.openjdk.org/jdk/pull/16155
Re: RFR: 8317970: Bump target macosx-x64 version to 11.00.00 [v3]
> macOS 10.x is no longer receiving updates - the most recent/last release was > 10.15.7 back in July of 2022. It's time to bump the target macOS version > (min/max) for macosx-x64. macOS 11.x is still receiving updates. > > This change updates the target version for macosx-x64 to 11.00.00, which is > the same version used for macosx-aarch64. Mikael Vidstedt 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 six additional commits since the last revision: - Merge branch 'master' into 8317970-macosx-x64-ver-11 - Update stale comment (again) - Update stale comment - macOS - Copyright year - Bump target macosx-x64 version to 11.00.00 - Changes: - all: https://git.openjdk.org/jdk/pull/16155/files - new: https://git.openjdk.org/jdk/pull/16155/files/71334787..56a95f67 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16155=02 - incr: https://webrevs.openjdk.org/?repo=jdk=16155=01-02 Stats: 5985 lines in 336 files changed: 3061 ins; 1628 del; 1296 mod Patch: https://git.openjdk.org/jdk/pull/16155.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16155/head:pull/16155 PR: https://git.openjdk.org/jdk/pull/16155
Integrated: 8318039: GHA: Bump macOS and Xcode versions
On Thu, 12 Oct 2023 19:25:13 GMT, Mikael Vidstedt wrote: > In GHA, the versions of macOS (note: the version used for build/test, **not** > the target macOS version we compile for) and Xcode are starting to show age. > It's time to update to more modern versions. > > This change bumps the macOS to 13 (from 11) and Xcode to 14.3.1 (from > 12.5.1/11.7). This is a prerequisite change for JDK-8317970 / #16155, without > which the build SIGSEGVs (presumably some since-fixed issue in Xcode 12.5.1). This pull request has now been integrated. Changeset: 605c9767 Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/605c9767291ddf1c409c3e805ffb3182899d06c2 Stats: 6 lines in 3 files changed: 0 ins; 0 del; 6 mod 8318039: GHA: Bump macOS and Xcode versions Reviewed-by: erikj, prr, ihse, clanger - PR: https://git.openjdk.org/jdk/pull/16171
Re: RFR: 8318039: GHA: Bump macOS and Xcode versions
On Thu, 12 Oct 2023 19:25:13 GMT, Mikael Vidstedt wrote: > In GHA, the versions of macOS (note: the version used for build/test, **not** > the target macOS version we compile for) and Xcode are starting to show age. > It's time to update to more modern versions. > > This change bumps the macOS to 13 (from 11) and Xcode to 14.3.1 (from > 12.5.1/11.7). This is a prerequisite change for JDK-8317970 / #16155, without > which the build SIGSEGVs (presumably some since-fixed issue in Xcode 12.5.1). Thank you for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/16171#issuecomment-1761889525
RFR: 8318039: GHA: Bump macOS and Xcode versions
In GHA, the versions of macOS (note: the version used for build/test, **not** the target macOS version we compile for) and Xcode are starting to show age. It's time to update to more modern versions. This change bumps the macOS to 13 (from 11) and Xcode to 14.3.1 (from 12.5.1/11.7). This is a prerequisite change for JDK-8317970 / #16155, without which the build SIGSEGVs (presumably some since-fixed issue in Xcode 12.5.1). - Commit messages: - 8318039: GHA: Bump macOS and Xcode versions Changes: https://git.openjdk.org/jdk/pull/16171/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16171=00 Issue: https://bugs.openjdk.org/browse/JDK-8318039 Stats: 7 lines in 3 files changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/16171.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16171/head:pull/16171 PR: https://git.openjdk.org/jdk/pull/16171
Re: RFR: 8317970: Bump target macosx-x64 version to 11.00.00 [v2]
> macOS 10.x is no longer receiving updates - the most recent/last release was > 10.15.7 back in July of 2022. It's time to bump the target macOS version > (min/max) for macosx-x64. macOS 11.x is still receiving updates. > > This change updates the target version for macosx-x64 to 11.00.00, which is > the same version used for macosx-aarch64. Mikael Vidstedt has updated the pull request incrementally with two additional commits since the last revision: - Update stale comment (again) - Update stale comment - Changes: - all: https://git.openjdk.org/jdk/pull/16155/files - new: https://git.openjdk.org/jdk/pull/16155/files/ce75188b..71334787 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16155=01 - incr: https://webrevs.openjdk.org/?repo=jdk=16155=00-01 Stats: 5 lines in 1 file changed: 0 ins; 2 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16155.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16155/head:pull/16155 PR: https://git.openjdk.org/jdk/pull/16155
RFR: 8317970: Bump target macosx-x64 version to 11.00.00
macOS 10.x is no longer receiving updates - the most recent/last release was 10.15.7 back in July of 2022. It's time to bump the target macOS version (min/max) for macosx-x64. macOS 11.x is still receiving updates. This change updates the target version for macosx-x64 to 11.00.00, which is the same version used for macosx-aarch64. - Commit messages: - macOS - Copyright year - Bump target macosx-x64 version to 11.00.00 Changes: https://git.openjdk.org/jdk/pull/16155/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16155=00 Issue: https://bugs.openjdk.org/browse/JDK-8317970 Stats: 18 lines in 3 files changed: 0 ins; 10 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/16155.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16155/head:pull/16155 PR: https://git.openjdk.org/jdk/pull/16155
Integrated: 8317560: Change to Xcode 14.3.1 for building on macOS at Oracle
On Thu, 5 Oct 2023 17:05:33 GMT, Mikael Vidstedt wrote: > This change bumps the version of Xcode used for building at Oracle to Xcode > 14.3.1 and updates the docs accordingly. The change also updates the devkit > creation logic to work around issues with macOS. This pull request has now been integrated. Changeset: a64794b1 Author: Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/a64794b1eda99fd20d318e77554d92a29fdb5661 Stats: 14 lines in 4 files changed: 0 ins; 1 del; 13 mod 8317560: Change to Xcode 14.3.1 for building on macOS at Oracle Reviewed-by: erikj - PR: https://git.openjdk.org/jdk/pull/16061
RFR: 8317560: Change to Xcode 14.3.1 for building on macOS at Oracle
This change bumps the version of Xcode used for building at Oracle to Xcode 14.3.1 and updates the docs accordingly. The change also updates the devkit creation logic to work around issues with macOS. - Commit messages: - 8317560: Change to Xcode 14.3.1 for building on macOS at Oracle Changes: https://git.openjdk.org/jdk/pull/16061/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16061=00 Issue: https://bugs.openjdk.org/browse/JDK-8317560 Stats: 14 lines in 4 files changed: 0 ins; 1 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/16061.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16061/head:pull/16061 PR: https://git.openjdk.org/jdk/pull/16061
Re: RFR: 8306630: Bump minimum boot jdk to JDK 21
On Tue, 26 Sep 2023 21:22:07 GMT, Mikael Vidstedt wrote: > With the JDK 21 GA out it's time to bump the minimum boot JDK version for > mainline/JDK 22. > > Testing: tier1-5, GHA Thank you for the reviews! - PR Comment: https://git.openjdk.org/jdk/pull/15934#issuecomment-1738269698
Integrated: 8306630: Bump minimum boot jdk to JDK 21
On Tue, 26 Sep 2023 21:22:07 GMT, Mikael Vidstedt wrote: > With the JDK 21 GA out it's time to bump the minimum boot JDK version for > mainline/JDK 22. > > Testing: tier1-5, GHA This pull request has now been integrated. Changeset: 83c0e451 Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/83c0e4516064846c956d9a760338e71be1593f6f Stats: 9 lines in 3 files changed: 0 ins; 0 del; 9 mod 8306630: Bump minimum boot jdk to JDK 21 Reviewed-by: darcy, erikj, iris, shade - PR: https://git.openjdk.org/jdk/pull/15934
RFR: 8306630: Bump minimum boot jdk to JDK 21
With the JDK 21 GA out it's time to bump the minimum boot JDK version for mainline/JDK 22. Testing: tier1-5, GHA (in progress) - Commit messages: - 8306630: Bump minimum boot jdk to JDK 21 Changes: https://git.openjdk.org/jdk/pull/15934/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15934=00 Issue: https://bugs.openjdk.org/browse/JDK-8306630 Stats: 9 lines in 3 files changed: 0 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/15934.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15934/head:pull/15934 PR: https://git.openjdk.org/jdk/pull/15934
Re: RFR: 8303427: Fixpath confused if unix root contains "/jdk"
On Tue, 29 Aug 2023 00:16:46 GMT, Erik Joelsson wrote: > On Windows, when a directory exists in the "unix" root with the same name as > a directory in the "test" dir, fixpath will corrupt test arguments to jtreg > (and possibly other arguments as well). Fixpath sees a string like this: > > test/jdk/foo > > It looks for the first `/` and checks if the first element following that, > until the next `/`, is an existing directory in the unix filesystem root. In > this case, the reporter had a directory named "/jdk" which satisfies this > heuristic check. This makes fixpath assume that the `/jdk/foo` part is an > absolute unix path that needs to be rewritten to a Windows path. > > My suggested fix is to look at the prefix part, "test" in this case, and see > if that itself is a valid path in the current working directory, as that > would indicate that the string is intended to be a relative path. > > I think we also need to account for possible prefixes with `:` and `=` here > to handle a string like: > > jtreg:test/jdk/foo > > In that case we need to remove anything up to the last `:` before we try to > match it as a relative directory. (Same thing applies to `=`) > > Changing the heuristics of fixpath is rather sensitive and risky. I would > appreciate help from people using Windows with trying this patch with some of > your regular workflows. Marked as reviewed by mikael (Reviewer). Just because I'm curious: in which cases would the prefix contain `=` (equals)? - PR Review: https://git.openjdk.org/jdk/pull/15461#pullrequestreview-1605277412 PR Comment: https://git.openjdk.org/jdk/pull/15461#issuecomment-1701464048
Re: RFR: 8314444: Update jib-profiles.js to use JMH 1.37 devkit
On Wed, 16 Aug 2023 15:52:09 GMT, Claes Redestad wrote: > Use most recent devkit when using jib. > > Test: staged, configured from scratch and verified micros locally Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15313#pullrequestreview-1581046370
Integrated: 8314166: Update googletest to v1.14.0
On Fri, 11 Aug 2023 22:24:00 GMT, Mikael Vidstedt wrote: > As part of the ongoing work to enable -Wconversion with gcc there are a > number of occurrences of code in the GoogleTest code itself which need > fixing. These have been addressed in version 1.14.0 of GoogleTest. This > change is picking up that version, adjusting the relevant build configuration > files for GHA and Oracle builds. > > Note: This change updates the documentation and configuration so that 1.14.0 > is the minimum required version. Strictly speaking 1.14.0 is not required > until -Wconversion is _actually_ enabled but since that is presumably just > around the corner.. > > Testing: GHA, tier1-5 (still in progress) This pull request has now been integrated. Changeset: f66c73d3 Author:Mikael Vidstedt URL: https://git.openjdk.org/jdk/commit/f66c73d34b1e02681f46eb3cd78126c05014f845 Stats: 10 lines in 5 files changed: 0 ins; 0 del; 10 mod 8314166: Update googletest to v1.14.0 Reviewed-by: kbarrett, stuefe, shade, erikj - PR: https://git.openjdk.org/jdk/pull/15253
Re: RFR: 8314166: Update googletest to v1.14.0 [v2]
On Tue, 15 Aug 2023 17:05:28 GMT, Mikael Vidstedt wrote: >> As part of the ongoing work to enable -Wconversion with gcc there are a >> number of occurrences of code in the GoogleTest code itself which need >> fixing. These have been addressed in version 1.14.0 of GoogleTest. This >> change is picking up that version, adjusting the relevant build >> configuration files for GHA and Oracle builds. >> >> Note: This change updates the documentation and configuration so that 1.14.0 >> is the minimum required version. Strictly speaking 1.14.0 is not required >> until -Wconversion is _actually_ enabled but since that is presumably just >> around the corner.. >> >> Testing: GHA, tier1-5 (still in progress) > > Mikael Vidstedt 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 two additional > commits since the last revision: > > - Merge branch 'master' into 8314166-gtest-1.14.0 > - Update googletest to v1.14.0 Thank you for the reviews. - PR Comment: https://git.openjdk.org/jdk/pull/15253#issuecomment-1679512321
Re: RFR: 8314166: Update googletest to v1.14.0 [v2]
> As part of the ongoing work to enable -Wconversion with gcc there are a > number of occurrences of code in the GoogleTest code itself which need > fixing. These have been addressed in version 1.14.0 of GoogleTest. This > change is picking up that version, adjusting the relevant build configuration > files for GHA and Oracle builds. > > Note: This change updates the documentation and configuration so that 1.14.0 > is the minimum required version. Strictly speaking 1.14.0 is not required > until -Wconversion is _actually_ enabled but since that is presumably just > around the corner.. > > Testing: GHA, tier1-5 (still in progress) Mikael Vidstedt 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 two additional commits since the last revision: - Merge branch 'master' into 8314166-gtest-1.14.0 - Update googletest to v1.14.0 - Changes: - all: https://git.openjdk.org/jdk/pull/15253/files - new: https://git.openjdk.org/jdk/pull/15253/files/36217d9e..acd7ec37 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15253=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15253=00-01 Stats: 6213 lines in 224 files changed: 3185 ins; 398 del; 2630 mod Patch: https://git.openjdk.org/jdk/pull/15253.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15253/head:pull/15253 PR: https://git.openjdk.org/jdk/pull/15253
RFR: 8314166: Update googletest to v1.14.0
As part of the ongoing work to enable -Wconversion with gcc there are a number of occurrences of code in the GoogleTest code itself which need fixing. These have been addressed in version 1.14.0 of GoogleTest. This change is picking up that version, adjusting the relevant build configuration files for GHA and Oracle builds. Note: This change updates the documentation and configuration so that 1.14.0 is the minimum required version. Strictly speaking 1.14.0 is not required until -Wconversion is _actually_ enabled but since that is presumably just around the corner.. Testing: GHA, tier1-5 (still in progress) - Commit messages: - Update googletest to v1.14.0 Changes: https://git.openjdk.org/jdk/pull/15253/files Webrev: https://webrevs.openjdk.org/?repo=jdk=15253=00 Issue: https://bugs.openjdk.org/browse/JDK-8314166 Stats: 10 lines in 5 files changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/15253.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15253/head:pull/15253 PR: https://git.openjdk.org/jdk/pull/15253
Re: [jdk21] RFR: 8312985: Remove EA from the JDK 21 version string with first RC promotion on August 10, 2023
On Mon, 7 Aug 2023 23:11:27 GMT, Jesper Wilhelmsson wrote: > JDK-8312985: Remove EA from the JDK 21 version string with first RC promotion > on August 10, 2023 Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk21/pull/165#pullrequestreview-1566261619
Re: RFR: 8311545: Allow test symbol files to be kept in the test image
On Fri, 7 Jul 2023 02:38:37 GMT, David Holmes wrote: > Please review this simple change to allow a test to request that its symbol > files be copied across into the test image. > > Testing: > - sanity test - tiers 1-3 > - direct testing in the context of > [JDK-8311541](https://bugs.openjdk.org/browse/JDK-8311541) > > Thanks Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14797#pullrequestreview-1527344527
Re: RFR: 8311545: Allow test symbol files to be kept in the test image
On Fri, 7 Jul 2023 02:38:37 GMT, David Holmes wrote: > Please review this simple change to allow a test to request that its symbol > files be copied across into the test image. > > Testing: > - sanity test - tiers 1-3 > - direct testing in the context of > [JDK-8311541](https://bugs.openjdk.org/browse/JDK-8311541) > > Thanks Just double-checking: this change is just adding the support, there are (currently) no active uses of it? - PR Comment: https://git.openjdk.org/jdk/pull/14797#issuecomment-1626108231
Re: [jdk21] RFR: 8308585: AC_REQUIRE: `PLATFORM_EXTRACT_TARGET_AND_BUILD' was expanded before it was required
On Fri, 30 Jun 2023 09:40:11 GMT, Erik Joelsson wrote: > This is a clean backport of what was initially a configure warning fix, but > turned out to also fix the original intention of JEP 449: deprecating the > Windows 32-bit port. Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk21/pull/85#pullrequestreview-1507646764
Re: RFR: 8310384: Add hooks for custom image creation [v2]
On Wed, 21 Jun 2023 17:23:15 GMT, Erik Joelsson wrote: >> We have a need for creating custom variants of the JDK image and would like >> to be able to reuse the existing Images.gmk for doing so. To be able to do >> that, we need to define some variables that are suitable for overrides. >> Specifically, we need: >> >> EXTRA_MODULES: a list of extra java module names to include in the "jdk" >> image. >> >> EXTRA_JMODS_DIR: a list of extra directories to add to the module path and >> look for jmods in, ahead the default jmods dir. >> >> JDK_IMAGE_SUPPORT_DIR: override the location of the support dir. Without >> overriding this, multiple calls to Images.gmk will overwrite each others >> intermediate files and dependency files. > > Erik Joelsson has updated the pull request incrementally with one additional > commit since the last revision: > > Applied same changes to jre image logic for symmetry Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14563#pullrequestreview-1491419856
Re: RFR: 8310384: Add hooks for custom image creation
On Tue, 20 Jun 2023 13:34:24 GMT, Erik Joelsson wrote: > We have a need for creating custom variants of the JDK image and would like > to be able to reuse the existing Images.gmk for doing so. To be able to do > that, we need to define some variables that are suitable for overrides. > Specifically, we need: > > EXTRA_MODULES: a list of extra java module names to include in the "jdk" > image. > > EXTRA_JMODS_DIR: a list of extra directories to add to the module path and > look for jmods in, ahead the default jmods dir. > > JDK_IMAGE_SUPPORT_DIR: override the location of the support dir. Without > overriding this, multiple calls to Images.gmk will overwrite each others > intermediate files and dependency files. Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14563#pullrequestreview-1491270859
Re: RFR: 8310379: Relax prerequisites for java.base-jmod target
On Tue, 20 Jun 2023 13:16:59 GMT, Erik Joelsson wrote: > The top level target "java.base-jmod" currently has all other jmod targets on > the prerequisites list. This is because we store a checksum for every non > upgradeable module in java.base and most of the modules aren't upgradeable. > But, since we do have upgradeable modules, those shouldn't be on the > prerequisites list for java.base-jmod. > > Fixing this won't impact the build much, but certainly won't hurt either. Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14561#pullrequestreview-1491256167
Re: RFR: 8310376: Extend SetupTarget macro with DIR parameter
On Tue, 20 Jun 2023 13:07:11 GMT, Erik Joelsson wrote: > To allow for more flexibility when using the SetupTarget macro in a custom > extension of Main.gmk, I would like to add a DIR parameter, which is the > directory the sub make command CDs into before calling make. It defaults to > $(TOPDIR)/make. This parameter should be set when the target makefile of the > call is located in a different directory. Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14560#pullrequestreview-1489246116
Re: RFR: 8310369: UTIL_ARG_WITH fails when arg is disabled [v2]
On Tue, 20 Jun 2023 13:10:16 GMT, Erik Joelsson wrote: >> I've recently tried to use UTIL_ARG_WITH for new configure arguments in a >> project repository and discovered some issues. The project in question may >> or may not end up in mainline at some point in the future, but I think >> fixing these general issues in UTIL_ARG_WITH is worth it independent of my >> specific use case. >> >> For TYPE "directory" the check if the value is a valid directory is supposed >> to optionally check for files in the CHECK_FOR_FILES list. The default value >> of this list is ":" (due to autoconf peculiarities) but the check is >> performed if the value is non empty. This means that if you call >> UTIL_ARG_WITH with TYPE "directory" and no CHECK_FOR_FILES, it will always >> fail because there is no file ":" in the given directory. This patch changes >> the conditional to check for ":" instead of the empty string. >> >> When an optional arg is defined, the validation check is still being >> performed when the arg has been disabled (--without-arg). This makes it >> impossible to disable something of for example TYPE "directory" as the >> directory check will fail. The current configure script in OpenJDK only has >> macro calls of type "string" and "literal" where this doesn't cause >> problems, because an empty string as value passes validation. This patch >> moves the validation so that it's only performed when the arg isn't disabled. > > Erik Joelsson has updated the pull request incrementally with one additional > commit since the last revision: > > copyright year Marked as reviewed by mikael (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14558#pullrequestreview-1489245625
