Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Wed, 3 Apr 2024 02:28:08 GMT, Julian Waters wrote: >> https://github.com/openjdk/jdk/pull/18586 > > @kimbarrett I've been doing things to permit gcc/Windows, not clang. clang > has too many different distributions on Windows for me to settle on one, and > generalising all of them to be able to compile with any of the Windows clang > distributions seamlessly and without issues sounds like a nightmare :P gcc on > the other hand has just 2: MSYS2 MINGW64 with ucrt (Which is the one I'm > working on) and standalone gcc Windows builds that link to ucrt > > I haven't sent a cleanup in this area because I thought my changes were too > specific to gcc/Windows, and wouldn't help much with HotSpot in general. I've > learnt from my mistakes in the past where I caused reviewers pain in > reviewing my admittedly selfish changes to HotSpot :( > That said, if it is requested of me, I can commit some cleanups to this file. > What do you think? @TheShermanTanker It depends on the details, of course. I think there are lots of possible cleanups in this vicinity that have little or nothing to do with gcc/Windows specifically, though might help there too. And yeah, I misremembered that it was not clang/Windows but rather gcc/Windows you were working on. But the same points largely apply here. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548868243
RFR: 8329494: Serial: Merge GenMarkSweep into MarkSweep
Hi all, This patch merges the class `GenMarkSweep` into `MarkSweep`. Just simply move. It may be better to merge the class `DeadSpacer` and `Compacter` into `MarkSweep` and then rename `MarkSweep`. The `MarkSweep` will be renamed at [JDK-8329521](https://bugs.openjdk.org/browse/JDK-8329521). And the `DeadSpacer` and `Compacter` may be refactored at [JDK-8305896](https://bugs.openjdk.org/browse/JDK-8305896) and [JDK-8305898](https://bugs.openjdk.org/browse/JDK-8305898), so I don't refactor them now. The tests `make test-tier1_gc` passed locally. Thanks for taking the time to review. Best Regards, -- Guoxiong - Commit messages: - JDK-8329494 Changes: https://git.openjdk.org/jdk/pull/18589/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18589=00 Issue: https://bugs.openjdk.org/browse/JDK-8329494 Stats: 1112 lines in 8 files changed: 513 ins; 589 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/18589.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18589/head:pull/18589 PR: https://git.openjdk.org/jdk/pull/18589
Re: RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler
On Tue, 2 Apr 2024 15:45:59 GMT, Magnus Ihse Bursie wrote: > This is a retake on https://github.com/openjdk/jdk/pull/15096. > > I tried to fix the remaining issues in that PR, but could not push them. In > the end, it seemed easier to create a new branch in my own personal fork. > > The majority of the work in this PR has been done by @TheShermanTanker. I > have stepped in and fixed the remaining comments, reverted some additional > unneeded changed code, and also tried to minimize code duplication in > `awt_PrintJob.cpp`. Looks good. Thanks for taking this up for me! In the future I hope @prrace will change his mind and allow the use of C++ Libraries in awt (awt already depends on the C++ Standard Library on Windows, verifiable if you run dumpbin -DEPENDENTS on it), because I have a change in mind relying on RAII that would look much cleaner and neater - Marked as reviewed by jwaters (Committer). PR Review: https://git.openjdk.org/jdk/pull/18584#pullrequestreview-1975384014
Re: RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler [v70]
On Mon, 1 Apr 2024 10:33:41 GMT, Julian Waters wrote: >> Julian Waters has updated the pull request incrementally with four >> additional commits since the last revision: >> >> - Labels to empty line in awt_Window.cpp >> - Labels to empty line in awt_Window.cpp >> - Label to empty line in awt_Window.cpp >> - Label to empty line in awt_Window.cpp > > Bumping > @TheShermanTanker I tried to help you get this done. I added fixes to a copy > of your branch on my personal fork, but then it turned out I could not push > them to your branch. :-( > > It ended up with me creating a new PR, #18584. As a bonus, I think it might > be easier to review with a fresh start. This PR has grown quite heavy with > lots of comments and commits. > > I hope you don't feel like I'm stealing this away from you. You have done a > great job, and shown a lot of patience of carrying this all the way here. But > I also got the impression that you would appreciate my assistance in getting > the last pieces in place so we can integrate this. Not at all, I don't feel like you're stealing this from me. In fact, I should be the one apologising for giving you extra work! Thanks for taking this up, I once again apologise for making you do this instead, I've been very busy since Thursday (working on OpenJDK while in lectures at times), and during my breaks I've been too drained to continue, so i really appreciate your help :) I will keep this open until the other Pull Request has been integrated, in case this might still be needed - PR Comment: https://git.openjdk.org/jdk/pull/15096#issuecomment-2033427284
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 20:10:12 GMT, Kim Barrett wrote: >> I'm waiting for a bunch of tests to complete, so decided to just take that >> issue. > > https://github.com/openjdk/jdk/pull/18586 @kimbarrett I've been doing things to permit gcc/Windows, not clang. clang has too many different distributions on Windows for me to settle on one, and generalising all of them to be able to compile with any of the Windows clang distributions seamlessly and without issues sounds like a nightmare :P gcc on the other hand has just 2: MSYS2 MINGW64 with ucrt (Which is the one I'm working on) and standalone gcc Windows builds that link to ucrt I haven't sent a cleanup in this area because I thought my changes were too specific to gcc/Windows, and wouldn't help much with HotSpot in general. I've learnt from my mistakes in the past where I caused reviewers pain in reviewing my admittedly selfish changes to HotSpot :( That said, if it is requested of me, I can commit some cleanups to this file. What do you think? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548833671
RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler
This is a retake on https://github.com/openjdk/jdk/pull/15096. I tried to fix the remaining issues in that PR, but could not push them. In the end, it seemed easier to create a new branch in my own personal fork. The majority of the work in this PR has been done by @TheShermanTanker. I have stepped in and fixed the remaining comments, reverted some additional unneeded changed code, and also tried to minimize code duplication in `awt_PrintJob.cpp`. - Commit messages: - 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler Changes: https://git.openjdk.org/jdk/pull/18584/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18584=00 Issue: https://bugs.openjdk.org/browse/JDK-8307160 Stats: 428 lines in 12 files changed: 284 ins; 50 del; 94 mod Patch: https://git.openjdk.org/jdk/pull/18584.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18584/head:pull/18584 PR: https://git.openjdk.org/jdk/pull/18584
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 11:35:44 GMT, Joachim Kern wrote: >> linux macos and now Aix use this file. > > Who is able to explain if > `#if defined(LINUX) || defined(_ALLBSD_SOURCE) || defined(_AIX)` > in this file is equivalent to > `#if 1` See my other comments and https://bugs.openjdk.org/browse/JDK-8329546 - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548550923
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 17:01:07 GMT, Kim Barrett wrote: >> https://bugs.openjdk.org/browse/JDK-8329546 - I can take this if nobody else >> grabs it soon. > > I'm waiting for a bunch of tests to complete, so decided to just take that > issue. https://github.com/openjdk/jdk/pull/18586 - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548549705
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v2]
On Tue, 2 Apr 2024 16:29:07 GMT, Alan Bateman wrote: >> Volodymyr Paprotski has updated the pull request incrementally with one >> additional commit since the last revision: >> >> remove use of jdk.crypto.ec > > src/java.base/share/classes/module-info.java line 265: > >> 263: jdk.jfr, >> 264: jdk.unsupported, >> 265: jdk.crypto.ec; > > jdk.crypto.ec has been hollowed out since JDK 22, the sun.security.ec are in > java.base. So I don't think you need this qualified export. Thanks, fixed. (Started this when `jdk.crypto.ec` was still in use.. missed a few spots during rebase I guess) - PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1548460157
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic [v2]
> Performance. Before: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt ScoreError Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6443.934 ± 6.491 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6152.979 ± 4.954 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1895.410 ± 36.979 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1878.955 ± 45.487 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1357.810 ± 26.584 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1352.119 ± 23.547 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± > 10.970 ops/s > > Performance, no intrinsic: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt Score Error Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6529.839 ± 42.420 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6199.747 ± 133.566 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1973.676 ± 54.071 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1932.127 ± 35.920 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1355.788 ± 29.858 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1346.523 ± 28.722 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply true thrpt3 1919.574 ± > 10.591 ops/s > > Performance, **with intrinsics*... Volodymyr Paprotski has updated the pull request incrementally with one additional commit since the last revision: remove use of jdk.crypto.ec - Changes: - all: https://git.openjdk.org/jdk/pull/18583/files - new: https://git.openjdk.org/jdk/pull/18583/files/dbe6cd3b..82b6dae7 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18583=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18583=00-01 Stats: 6 lines in 2 files changed: 0 ins; 1 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/18583.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18583/head:pull/18583 PR: https://git.openjdk.org/jdk/pull/18583
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 16:52:04 GMT, Kim Barrett wrote: >> There was at one time an attempt at a gcc/Solaris port, but I think it was >> never completed, and most vestiges removed. More recently, @TheShermanTanker >> has been doing stuff to permit clang/Windows, and clang-based builds use >> this file. >> I'm kind of surprised he hasn't encountered problems and done some cleanup >> here. >> >> (and ) and 64bit integer types are standard C++ now, >> so no longer need all this conditionalization. I suggest cleaning that up as >> a >> separate precursor. That would eliminate the two !defined blocks entirely. I >> wish the other conditional includes in this block were "where needed" rather >> than in globalDefinitions_gcc.hpp, but that's a different mess. > > https://bugs.openjdk.org/browse/JDK-8329546 - I can take this if nobody else > grabs it soon. I'm waiting for a bunch of tests to complete, so decided to just take that issue. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548252193
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 16:41:40 GMT, Kim Barrett wrote: >> I cannot answer this question. >> If this line is now obsolete it was also obsolete before including AIX, >> because AIX didn't use this file beforehand. > > There was at one time an attempt at a gcc/Solaris port, but I think it was > never completed, and most vestiges removed. More recently, @TheShermanTanker > has been doing stuff to permit clang/Windows, and clang-based builds use this > file. > I'm kind of surprised he hasn't encountered problems and done some cleanup > here. > > (and ) and 64bit integer types are standard C++ now, > so no longer need all this conditionalization. I suggest cleaning that up as a > separate precursor. That would eliminate the two !defined blocks entirely. I > wish the other conditional includes in this block were "where needed" rather > than in globalDefinitions_gcc.hpp, but that's a different mess. https://bugs.openjdk.org/browse/JDK-8329546 - I can take this if nobody else grabs it soon. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548239737
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
On Tue, 2 Apr 2024 11:20:49 GMT, Joachim Kern wrote: >> src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 62: >> >>> 60: #include >>> 61: >>> 62: #if defined(LINUX) || defined(_ALLBSD_SOURCE) || defined(_AIX) >> >> What else is left? Could we just remove this line altogether now? > > I cannot answer this question. > If this line is now obsolete it was also obsolete before including AIX, > because AIX didn't use this file beforehand. There was at one time an attempt at a gcc/Solaris port, but I think it was never completed, and most vestiges removed. More recently, @TheShermanTanker has been doing stuff to permit clang/Windows, and clang-based builds use this file. I'm kind of surprised he hasn't encountered problems and done some cleanup here. (and ) and 64bit integer types are standard C++ now, so no longer need all this conditionalization. I suggest cleaning that up as a separate precursor. That would eliminate the two !defined blocks entirely. I wish the other conditional includes in this block were "where needed" rather than in globalDefinitions_gcc.hpp, but that's a different mess. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548225495
Re: RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic
On Tue, 2 Apr 2024 15:42:05 GMT, Volodymyr Paprotski wrote: > Performance. Before: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt ScoreError Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6443.934 ± 6.491 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6152.979 ± 4.954 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1895.410 ± 36.979 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1878.955 ± 45.487 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1357.810 ± 26.584 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1352.119 ± 23.547 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± > 10.970 ops/s > > Performance, no intrinsic: > > Benchmark(algorithm) (dataSize) (keyLength) > (provider) Mode Cnt Score Error Units > SignatureBench.ECDSA.signSHA256withECDSA1024 256 > thrpt3 6529.839 ± 42.420 ops/s > SignatureBench.ECDSA.signSHA256withECDSA 16384 256 > thrpt3 6199.747 ± 133.566 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA1024 256 > thrpt3 1973.676 ± 54.071 ops/s > SignatureBench.ECDSA.verify SHA256withECDSA 16384 256 > thrpt3 1932.127 ± 35.920 ops/s > Benchmark(algorithm) (keyLength) > (kpgAlgorithm) (provider) Mode Cnt ScoreError Units > o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1355.788 ± 29.858 ops/s > o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256 > EC thrpt3 1346.523 ± 28.722 ops/s > Benchmark (isMontBench) Mode Cnt Score > Error Units > PolynomialP256Bench.benchMultiply true thrpt3 1919.574 ± > 10.591 ops/s > > Performance, **with intrinsics*... src/java.base/share/classes/module-info.java line 265: > 263: jdk.jfr, > 264: jdk.unsupported, > 265: jdk.crypto.ec; jdk.crypto.ec has been hollowed out since JDK 22, the sun.security.ec are in java.base. So I don't think you need this qualified export. - PR Review Comment: https://git.openjdk.org/jdk/pull/18583#discussion_r1548199507
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v3]
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about > ill formatted printf Joachim Kern has updated the pull request incrementally with one additional commit since the last revision: version check not needed anymore - Changes: - all: https://git.openjdk.org/jdk/pull/18536/files - new: https://git.openjdk.org/jdk/pull/18536/files/689b353d..ac1335e5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18536=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18536=01-02 Stats: 6 lines in 1 file changed: 0 ins; 6 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18536.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18536/head:pull/18536 PR: https://git.openjdk.org/jdk/pull/18536
RFR: 8329538: Accelerate P256 on x86_64 using Montgomery intrinsic
Performance. Before:
Benchmark(algorithm) (dataSize) (keyLength)
(provider) Mode Cnt ScoreError Units
SignatureBench.ECDSA.signSHA256withECDSA1024 256
thrpt3 6443.934 ± 6.491 ops/s
SignatureBench.ECDSA.signSHA256withECDSA 16384 256
thrpt3 6152.979 ± 4.954 ops/s
SignatureBench.ECDSA.verify SHA256withECDSA1024 256
thrpt3 1895.410 ± 36.979 ops/s
SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
thrpt3 1878.955 ± 45.487 ops/s
Benchmark(algorithm) (keyLength)
(kpgAlgorithm) (provider) Mode Cnt ScoreError Units
o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256
EC thrpt3 1357.810 ± 26.584 ops/s
o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256
EC thrpt3 1352.119 ± 23.547 ops/s
Benchmark (isMontBench) Mode Cnt ScoreError
Units
PolynomialP256Bench.benchMultiply false thrpt3 1746.126 ± 10.970
ops/s
Performance, no intrinsic:
Benchmark(algorithm) (dataSize) (keyLength)
(provider) Mode Cnt Score Error Units
SignatureBench.ECDSA.signSHA256withECDSA1024 256
thrpt3 6529.839 ± 42.420 ops/s
SignatureBench.ECDSA.signSHA256withECDSA 16384 256
thrpt3 6199.747 ± 133.566 ops/s
SignatureBench.ECDSA.verify SHA256withECDSA1024 256
thrpt3 1973.676 ± 54.071 ops/s
SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
thrpt3 1932.127 ± 35.920 ops/s
Benchmark(algorithm) (keyLength)
(kpgAlgorithm) (provider) Mode Cnt ScoreError Units
o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256
EC thrpt3 1355.788 ± 29.858 ops/s
o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256
EC thrpt3 1346.523 ± 28.722 ops/s
Benchmark (isMontBench) Mode Cnt ScoreError
Units
PolynomialP256Bench.benchMultiply true thrpt3 1919.574 ± 10.591
ops/s
Performance, **with intrinsics**
Benchmark(algorithm) (dataSize) (keyLength)
(provider) Mode Cnt Score Error Units
SignatureBench.ECDSA.signSHA256withECDSA1024 256
thrpt3 10384.591 ± 65.274 ops/s
SignatureBench.ECDSA.signSHA256withECDSA 16384 256
thrpt3 9592.912 ± 236.411 ops/s
SignatureBench.ECDSA.verify SHA256withECDSA1024 256
thrpt3 3479.494 ± 44.578 ops/s
SignatureBench.ECDSA.verify SHA256withECDSA 16384 256
thrpt3 3402.147 ± 26.772 ops/s
Benchmark(algorithm) (keyLength)
(kpgAlgorithm) (provider) Mode Cnt ScoreError Units
o.o.b.j.c.full.KeyAgreementBench.EC.generateSecret ECDH 256
EC thrpt3 2527.678 ± 64.791 ops/s
o.o.b.j.c.small.KeyAgreementBench.EC.generateSecret ECDH 256
EC thrpt3 2541.258 ± 66.634 ops/s
Benchmark (isMontBench) Mode Cnt ScoreError
Units
PolynomialP256Bench.benchMultiply true thrpt3 3021.139 ± 98.289
ops/s
Summary on design (see code for 'ASCII art', references and details on math):
- Added a new `IntegerPolynomial` field (`MontgomeryIntegerPolynomialP256`)
with 52-bit limbs
- `getElement(*)/fromMontgomery()` to convert numbers into/out of the field
- `ECOperations` is the primary use of the new field
- flattened some extra deep nested class hierarchy (also in prep for further
other field optimizations)
- `forParameters()/multiply()/setSum()` generates numbers in the new field
- `ProjectivePoint/Montgomery{Imm|M}utable.asAffine()` to convert out of the
new field
- Added Fuzz Testing and KAT verified with OpenSSL
-
Commit messages:
- remove trailing whitespace
- Remeasure performance
- Fix rebase typo
- Address comments from Anas and thorough cleanup
- conditionalAssign intrinsic
- rebase
Changes: https://git.openjdk.org/jdk/pull/18583/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=18583=00
Issue: https://bugs.openjdk.org/browse/JDK-8329538
Stats: 2335 lines in 34 files changed: 2037 ins; 162 del; 136 mod
Patch: https://git.openjdk.org/jdk/pull/18583.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18583/head:pull/18583
PR: https://git.openjdk.org/jdk/pull/18583
Re: RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler [v70]
On Mon, 1 Apr 2024 10:33:41 GMT, Julian Waters wrote: >> Julian Waters has updated the pull request incrementally with four >> additional commits since the last revision: >> >> - Labels to empty line in awt_Window.cpp >> - Labels to empty line in awt_Window.cpp >> - Label to empty line in awt_Window.cpp >> - Label to empty line in awt_Window.cpp > > Bumping @TheShermanTanker I tried to help you get this done. I added fixes to a copy of your branch on my personal fork, but then it turned out I could not push them to your branch. :-( It ended up with me creating a new PR, https://github.com/openjdk/jdk/pull/18584. As a bonus, I think it might be easier to review with a fresh start. This PR has grown quite heavy with lots of comments and commits. I hope you don't feel like I'm stealing this away from you. You have done a great job, and shown a lot of patience of carrying this all the way here. But I also got the impression that you would appreciate my assistance in getting the last pieces in place so we can integrate this. - PR Comment: https://git.openjdk.org/jdk/pull/15096#issuecomment-2032430773
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v2]
On Tue, 2 Apr 2024 14:48:49 GMT, Martin Doerr wrote: >> My question is, do we need this block, because now already configure warns >> about an outdated compiler, or is a warning to weak and we want to force >> this error here? > > I think that building with xlc 16 is no longer possible because the old build > pipeline is no longer supported and that is already caught by configure. So, > can we even reach here with older xlc compilers? > If not, this code can get removed. Yes, of course you are right. All the compile statements will fail with xlc 16 or older. I will remove it. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548134431
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v2]
On Tue, 2 Apr 2024 11:22:54 GMT, Joachim Kern wrote: >> I'd prefer having less AIX specific parts in this file. Can this be moved >> somewhere else? Or maybe combine it with the AIX code above? > > My question is, do we need this block, because now already configure warns > about an outdated compiler, or is a warning to weak and we want to force this > error here? I think that building with xlc 16 is no longer possible because the old build pipeline is no longer supported and that is already caught by configure. So, can we even reach here with older xlc compilers? If not, this code can get removed. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1548043503
Re: RFR: 8328614: hsdis: dlsym can't find decode symbol [v2]
On Thu, 21 Mar 2024 06:58:43 GMT, Robbin Ehn wrote: >> Hi, please consider. >> >> [8327045](https://bugs.openjdk.org/browse/JDK-8327045) hide these symbols. >> Tested with gcc and clang, and llvm and binutils backend. >> >> I didn't find any use of the "DLL_ENTRY", so I removed it. >> >> Thanks, Robbin > > Robbin Ehn has updated the pull request incrementally with one additional > commit since the last revision: > > remove swap file How would you add jni.h ? - PR Comment: https://git.openjdk.org/jdk/pull/18400#issuecomment-2032202423
Re: RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v23]
On Tue, 19 Mar 2024 16:55:14 GMT, Severin Gehwolf wrote: >> Please review this patch which adds a jlink mode to the JDK which doesn't >> need the packaged modules being present. A.k.a run-time image based jlink. >> Fundamentally this patch adds an option to use `jlink` even though your JDK >> install might not come with the packaged modules (directory `jmods`). This >> is particularly useful to further reduce the size of a jlinked runtime. >> After the removal of the concept of a JRE, a common distribution mechanism >> is still the full JDK with all modules and packaged modules. However, >> packaged modules can incur an additional size tax. For example in a >> container scenario it could be useful to have a base JDK container including >> all modules, but without also delivering the packaged modules. This comes at >> a size advantage of `~25%`. Such a base JDK container could then be used to >> `jlink` application specific runtimes, further reducing the size of the >> application runtime image (App + JDK runtime; as a single image *or* >> separate bundles, depending on the app being modularized). >> >> The basic design of this approach is to add a jlink plugin for tracking >> non-class and non-resource files of a JDK install. I.e. files which aren't >> present in the jimage (`lib/modules`). This enables producing a `JRTArchive` >> class which has all the info of what constitutes the final jlinked runtime. >> >> Basic usage example: >> >> $ diff -u <(./bin/java --list-modules --limit-modules java.se) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules java.se) >> $ diff -u <(./bin/java --list-modules --limit-modules jdk.jlink) >> <(../linux-x86_64-server-release/images/jdk/bin/java --list-modules >> --limit-modules jdk.jlink) >> $ ls ../linux-x86_64-server-release/images/jdk/jmods >> java.base.jmodjava.net.http.jmod java.sql.rowset.jmod >> jdk.crypto.ec.jmod jdk.internal.opt.jmod >> jdk.jdi.jmod jdk.management.agent.jmod jdk.security.auth.jmod >> java.compiler.jmodjava.prefs.jmod java.transaction.xa.jmod >> jdk.dynalink.jmod jdk.internal.vm.ci.jmod >> jdk.jdwp.agent.jmod jdk.management.jfr.jmodjdk.security.jgss.jmod >> java.datatransfer.jmodjava.rmi.jmodjava.xml.crypto.jmod >> jdk.editpad.jmod jdk.internal.vm.compiler.jmod >> jdk.jfr.jmod jdk.management.jmodjdk.unsupported.desktop.jmod >> java.desktop.jmod java.scripting.jmod java.xml.jmod >> jdk.hotspot.agent.jmod jdk.i... > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Move CreateLinkableRuntimePlugin to build folder > > Keep runtime link supporting classes in package > jdk.tools.jlink.internal.runtimelink I'm posting this for posterity, since I did some research on the `jimage` format in light of being able to re-create an existing `jimage` file with potentially a few resources being added. One use-case for this would be to add the "diff" data to an pre-existing, optimized `jimage` in `images/jdk/lib/modules`. The way that the `jimage` write algorithm works is based on the fact that it knows the resources and bytes at `jlink` time in **full**. Therefore, it can iterate over all resources, generate an index (and header) for them and then can serialize resource bytes as well as container (folder) information for them so as to support various iteration entry points. Since it's inherently relying on the header, index and container information at `jimage` read-time the format doesn't lend itself nicely for "appending". By adding just a few resources, the header, index, and container iteration information get invalidated. Therefore, a new `jimage` would need to be created, based on the old `jimage`'s resources inferring `ResourcePoolEntry`s and then working with them. The additional resources would then get added and header/index/container info generated afresh. In order to support this, the existing `jimage` file would need to have sufficient information in it, to re-constitute a "similar" jimage from it (at the infer `ResourcePoolEntry` step). There are two specific issues that need to be overcome to support this: resource ordering and resource compression. There might be more, that I'm missing. ## Ordering of Resources `jlink` supports ordering of resources. In other words, the `--order-resources` plugin instructs `jlink` to produce a `jimage` where resources will end up in the target `jimage` in a specified order. This can be observed by listing `jimage` contents sorted by content byte offsets. For example, the default JDK build for Linux adds this expression:
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc [v2]
> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building > the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect > clang by another name, and it uses the clang toolchain in the JDK build. Thus > the old xlc toolchain was removed by > [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). > Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the > last xlc rudiment. > This means merging the AIX specific content of > utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp > into the corresponding gcc files on the on side and removing the > defined(TARGET_COMPILER_xlc) blocks in the code, because the > defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX > compiler. > The rest of the changes are needed because of using > utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about > ill formatted printf Joachim Kern has updated the pull request incrementally with one additional commit since the last revision: Followed the proposals - Changes: - all: https://git.openjdk.org/jdk/pull/18536/files - new: https://git.openjdk.org/jdk/pull/18536/files/61fd0ff2..689b353d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18536=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18536=00-01 Stats: 35 lines in 9 files changed: 0 ins; 4 del; 31 mod Patch: https://git.openjdk.org/jdk/pull/18536.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18536/head:pull/18536 PR: https://git.openjdk.org/jdk/pull/18536
Integrated: 8329289: Unify SetupJdkExecutable and SetupJdkLibrary
On Thu, 28 Mar 2024 17:59:10 GMT, Magnus Ihse Bursie wrote: > Currently a lot of code is duplicated between SetupJdkExecutable and > SetupJdkLibrary. Furthermore, some functionality is still missing from > SetupJdkExecutable that is present in SetupJdkLibrary. These functions also > have not had their documentation properly updated as they have evolved. This > PR will fix all of this. This pull request has now been integrated. Changeset: 5ac067f6 Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/5ac067f6d6e0b301b33fb287aa3f288d318df2ba Stats: 170 lines in 1 file changed: 52 ins; 83 del; 35 mod 8329289: Unify SetupJdkExecutable and SetupJdkLibrary Reviewed-by: erikj - PR: https://git.openjdk.org/jdk/pull/18537
Re: RFR: 8329289: Unify SetupJdkExecutable and SetupJdkLibrary [v2]
> Currently a lot of code is duplicated between SetupJdkExecutable and > SetupJdkLibrary. Furthermore, some functionality is still missing from > SetupJdkExecutable that is present in SetupJdkLibrary. These functions also > have not had their documentation properly updated as they have evolved. This > PR will fix all of this. Magnus Ihse Bursie has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits: - Merge branch 'master' into unify-jdk-compilation - FIx documentation - Remove FindSrcDirsForLib - Unify and simplify code - Merge SetupJdkExecutable and SetupJdkLibrary into SetupJdkNativeCompilation - Changes: https://git.openjdk.org/jdk/pull/18537/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18537=01 Stats: 170 lines in 1 file changed: 52 ins; 83 del; 35 mod Patch: https://git.openjdk.org/jdk/pull/18537.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18537/head:pull/18537 PR: https://git.openjdk.org/jdk/pull/18537
Integrated: 8329292: Fix missing cleanups in java.management and jdk.management
On Thu, 28 Mar 2024 18:22:27 GMT, Magnus Ihse Bursie wrote: > For some reason, I missed adding the new standard header for SetupJdkLibrary > in java.management and jdk.management. I also missed to remove a now > superfluous CFLAGS_JDKLIB in libmanagement_ext. This pull request has now been integrated. Changeset: 5ae849d6 Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/5ae849d66f195e96fbae9dcf06a44d8aab659181 Stats: 5 lines in 2 files changed: 4 ins; 0 del; 1 mod 8329292: Fix missing cleanups in java.management and jdk.management Reviewed-by: erikj - PR: https://git.openjdk.org/jdk/pull/18538
Re: RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler [v52]
On Fri, 22 Mar 2024 12:26:25 GMT, Magnus Ihse Bursie wrote: >> Julian Waters has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Revert Formatting in awt_Component.cpp >> - Revert Formatting in awt_Window.cpp > > src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 779: > >> 777: } >> 778: >> 779: > > Suggestion: To be clear: I recommend deleting this line, since it does not make sense to have two consecutive blank lines here. > src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 1316: > >> 1314: env->CallVoidMethod(newPaper, setImageableID, ix, iy, iw, ih); >> 1315: >> 1316: > > Suggestion: I recommend deleting this line, since it does not make sense to have two consecutive blank lines here. - PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547831604 PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547832678
Re: RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler [v46]
On Sat, 20 Jan 2024 00:38:04 GMT, Phil Race wrote:
>> Julian Waters has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains 79 commits:
>>
>> - Merge branch 'openjdk:master' into patch-10
>> - Merge branch 'openjdk:master' into patch-10
>> - Fix awt_Window.cpp
>> - Fix awt_PrintJob.cpp
>> - -Zc:stringStrings no longer needed with -permissive- flags-cflags.m4
>> - Fix awt_Window.cpp
>> - awt_Window.cpp
>> - awt_PrintJob.cpp
>> - awt_Frame.cpp
>> - Whitespace awt_Component.cpp
>> - ... and 69 more: https://git.openjdk.org/jdk/compare/35e96627...cbfbaee6
>
> src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3137:
>
>> 3135:
>> 3136: PDATA pData = JNI_GET_PDATA(self);
>> 3137: if (self == NULL) {
>
> Surely line 3136 above should have been deleted ? It is replaced by line 3143
> below.
> And then you can also directly set window, pData isn't needed, as discussed
> in similar cases above
Yes.
@TheShermanTanker This is in fact a serious bug. You try to do `pData =
JNI_GET_PDATA(self)` before the null check of self, and this will crash if self
is null.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547836085
Re: RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler [v70]
On Thu, 28 Mar 2024 07:36:04 GMT, Julian Waters wrote:
>> We should set the -permissive- flag for the Microsoft Visual C compiler, as
>> was requested by the now backed out
>> [JDK-8241499](https://bugs.openjdk.org/browse/JDK-8241499). Doing so makes
>> the Visual C compiler much less accepting of ill formed code, which will
>> improve code quality on Windows in the future.
>
> Julian Waters has updated the pull request incrementally with four additional
> commits since the last revision:
>
> - Labels to empty line in awt_Window.cpp
> - Labels to empty line in awt_Window.cpp
> - Label to empty line in awt_Window.cpp
> - Label to empty line in awt_Window.cpp
src/java.desktop/windows/native/libawt/windows/awt_Canvas.cpp line 234:
> 232: c->m_eraseBackgroundOnResize = doEraseOnResize;
> 233:
> 234:
I recommend deleting this line, since it does not make sense to have two
consecutive blank lines here.
src/java.desktop/windows/native/libawt/windows/awt_Component.cpp line 6586:
> 6584: component->GetInsets(gis->insets);
> 6585:
> 6586:
I recommend deleting this line, since it does not make sense to have two
consecutive blank lines here.
src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp line 1603:
> 1601: } else {
> 1602: f = (AwtFrame *) JNI_GET_PDATA(peer);
> 1603: if (f == nullptr) {
@prrace What is the current take on `NULL` vs `nullptr` in client code? I know
Hotspot made an effort to completely purge all `NULL` references.
The new code here uses a mix of `NULL` and `nullprt`. Should it:
a) only use `NULL`
b) only use `nullptr`
c) remain as it is
?
src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 214:
> 212: static const double POINTS_TO_LOMETRIC = (254.0 / 72.0);
> 213:
> 214:
I recommend deleting one more line, since otherwise you will now have three
consecutive blank lines.
src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 1031:
> 1029: window->RepositionSecurityWarning(env);
> 1030:
> 1031: ret:
I recommend deleting this line, since it does not make sense to have two
consecutive blank lines here.
src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3159:
> 3157: }
> 3158:
> 3159:
I recommend deleting this line, since it does not make sense to have two
consecutive blank lines here.
src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3193:
> 3191: }
> 3192:
> 3193:
I recommend deleting this line, since it does not make sense to have two
consecutive blank lines here.
src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3224:
> 3222: window->SetTranslucency(iOpacity, window->isOpaque());
> 3223:
> 3224:
I recommend deleting this line, since it does not make sense to have two
consecutive blank lines here.
src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3255:
> 3253: window->SetTranslucency(window->getOpacity(), isOpaque);
> 3254:
> 3255:
I recommend deleting this line, since it does not make sense to have two
consecutive blank lines here.
src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3293:
> 3291: uws->hBitmap);
> 3292:
> 3293:
I recommend deleting this line, since it does not make sense to have two
consecutive blank lines here.
src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3328:
> 3326: window->setFullScreenExclusiveModeState(state != 0);
> 3327:
> 3328:
I recommend deleting this line, since it does not make sense to have two
consecutive blank lines here.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547824279
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547824797
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547829415
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547830886
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547833185
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547836998
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547837189
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547837344
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547837450
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547837625
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547837761
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Tue, 2 Apr 2024 11:28:30 GMT, Joachim Kern wrote: >> src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 103: >> >>> 101: #endif >>> 102: >>> 103: #if !defined(LINUX) && !defined(_ALLBSD_SOURCE) && !defined(_AIX) >> >> I believe this whole section can be removed now. >> >> At least I have no idea who this is for. What gcc versions does OpenJDK >> still support, then, beside these platforms. Also, any gcc platform not on >> linux or bsd would have hit the #error below at line 132. > > linux macos and now Aix use this file. Who is able to explain if `#if defined(LINUX) || defined(_ALLBSD_SOURCE) || defined(_AIX)` in this file is equivalent to `#if 1` - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547692144
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 08:06:01 GMT, Thomas Stuefe wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about >> ill formatted printf > > src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 83: > >> 81: #error "xlc version not supported, macro __open_xl_version__ not found" >> 82: #endif >> 83: #endif // AIX > > Can probably be shortened like this: > > Suggestion: > > #ifdef _AIX > #if !defined(__open_xl_version__) || (__open_xl_version__ < 17) > #error "this xlc version is not supported" > #endif > #endif // AIX followed your proposal. > src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 103: > >> 101: #endif >> 102: >> 103: #if !defined(LINUX) && !defined(_ALLBSD_SOURCE) && !defined(_AIX) > > I believe this whole section can be removed now. > > At least I have no idea who this is for. What gcc versions does OpenJDK still > support, then, beside these platforms. Also, any gcc platform not on linux or > bsd would have hit the #error below at line 132. linux macos and now Aix use this file. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547677545 PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547681162
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Thu, 28 Mar 2024 17:33:29 GMT, Martin Doerr wrote: >> src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 83: >> >>> 81: #error "xlc version not supported, macro __open_xl_version__ not >>> found" >>> 82: #endif >>> 83: #endif // AIX >> >> This `#ifdef _AIX` might be obsolete, because configure will throw a warning >> if the compiler has a lower version, but it's only a warning. > > I'd prefer having less AIX specific parts in this file. Can this be moved > somewhere else? Or maybe combine it with the AIX code above? My question is, do we need this block, because now already configure warns about an outdated compiler, or is a warning to weak and we want to force this error here? - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547672502
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:39:06 GMT, Thomas Stuefe wrote: >> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building >> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect >> clang by another name, and it uses the clang toolchain in the JDK build. >> Thus the old xlc toolchain was removed by >> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701). >> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the >> last xlc rudiment. >> This means merging the AIX specific content of >> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp >> into the corresponding gcc files on the on side and removing the >> defined(TARGET_COMPILER_xlc) blocks in the code, because the >> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX >> compiler. >> The rest of the changes are needed because of using >> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about >> ill formatted printf > > src/hotspot/share/utilities/globalDefinitions_gcc.hpp line 62: > >> 60: #include >> 61: >> 62: #if defined(LINUX) || defined(_ALLBSD_SOURCE) || defined(_AIX) > > What else is left? Could we just remove this line altogether now? I cannot answer this question. If this line is now obsolete it was also obsolete before including AIX, because AIX didn't use this file beforehand. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547667349
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:25:30 GMT, Thomas Stuefe wrote:
>> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building
>> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect
>> clang by another name, and it uses the clang toolchain in the JDK build.
>> Thus the old xlc toolchain was removed by
>> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701).
>> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the
>> last xlc rudiment.
>> This means merging the AIX specific content of
>> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp
>> into the corresponding gcc files on the on side and removing the
>> defined(TARGET_COMPILER_xlc) blocks in the code, because the
>> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX
>> compiler.
>> The rest of the changes are needed because of using
>> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about
>> ill formatted printf
>
> src/hotspot/os/aix/os_aix.cpp line 1212:
>
>> 1210: st->print_cr("physical free : " SIZE_FORMAT, (unsigned
>> long)mi.real_free);
>> 1211: st->print_cr("swap total : " SIZE_FORMAT, (unsigned
>> long)mi.pgsp_total);
>> 1212: st->print_cr("swap free : " SIZE_FORMAT, (unsigned
>> long)mi.pgsp_free);
>
> A better way to do this would be to change AIX::meminfo to use size_t. We
> should have done this when introducing that API.
Done. modified `os::Aix::meminfo_t` to use `size_t` instead of `long long`
> src/hotspot/os/aix/os_aix.cpp line 1399:
>
>> 1397: os->print("[" PTR_FORMAT " - " PTR_FORMAT "] (" UINTX_FORMAT
>> 1398: " bytes, %ld %s pages), %s",
>> 1399: (uintptr_t)addr, (uintptr_t)addr + size - 1, size, size /
>> pagesize, describe_pagesize(pagesize),
>
> p2i
Done.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547603744
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547606275
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:19:33 GMT, Thomas Stuefe wrote:
>> src/hotspot/os/aix/loadlib_aix.cpp line 120:
>>
>>> 118: (lm->is_in_vm ? '*' : ' '),
>>> 119: (uintptr_t)lm->text, (uintptr_t)lm->text + lm->text_len,
>>> 120: (uintptr_t)lm->data, (uintptr_t)lm->data + lm->data_len,
>>
>> Please don't cast, use `p2i()`.
>
> Check copyrights in this file and all others. Adapt SAP and Oracle copyrights.
Done + will adopt copyrights
>> src/hotspot/os/aix/os_aix.cpp line 651:
>>
>>> 649: lt.print("Thread is alive (tid: " UINTX_FORMAT ", kernel thread
>>> id: " UINTX_FORMAT
>>> 650: ", stack [" PTR_FORMAT " - " PTR_FORMAT " (" SIZE_FORMAT
>>> "k using %luk pages)).",
>>> 651: os::current_thread_id(), (uintx) kernel_thread_id,
>>> (uintptr_t)low_address, (uintptr_t)high_address,
>>
>> Use p2i, not cast
>
> Here, and in other places too where you cast a pointer to fit into PTR_FORMAT
> or INTPTR_FORMAT
Done.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547607793
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547606610
Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v4]
On Thu, 28 Mar 2024 18:41:03 GMT, Paul Sandoz wrote: >> Hamlin Li has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fix jni includes > > Hamlin, thank you for working on this. I think integrating a sub-set of SLEEF > is valuable (not all of it makes sense e.g., DFT part). My recommendation > would be to focus on a PR that integrates the required source, rather than > taking steps towards that. > > AFAICT from browsing prior comments "integrate the source" appears to be the > generally preferred solution, but there is some understandable hesitancy > about legal aspects. IIUC from what you say this is a technically feasible > and maintainable solution. As said here: > >> We (Oracle Java Platform Group) can handle the required "paperwork > https://github.com/openjdk/jdk/pull/16234#issuecomment-1823335443 Thanks @PaulSandoz for your comment and suggestion. I will work on the solution which integrates the source of sleef into jdk. - PR Comment: https://git.openjdk.org/jdk/pull/18294#issuecomment-2031671597
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:21:43 GMT, Thomas Stuefe wrote:
>> As of [JDK-8325880](https://bugs.openjdk.org/browse/JDK-8325880), building
>> the JDK requires version 17 of IBM Open XL C/C++ (xlc). This is in effect
>> clang by another name, and it uses the clang toolchain in the JDK build.
>> Thus the old xlc toolchain was removed by
>> [JDK-8327701](https://bugs.openjdk.org/browse/JDK-8327701).
>> Now we also switch the HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc, removing the
>> last xlc rudiment.
>> This means merging the AIX specific content of
>> utilities/globalDefinitions_xlc.hpp and utilities/compilerWarnings_xlc.hpp
>> into the corresponding gcc files on the on side and removing the
>> defined(TARGET_COMPILER_xlc) blocks in the code, because the
>> defined(TARGET_COMPILER_gcc) blocks work out of the box for the new AIX
>> compiler.
>> The rest of the changes are needed because of using
>> utilities/compilerWarnings_gcc.hpp the compiler is much more nagging about
>> ill formatted printf
>
> src/hotspot/os/aix/os_aix.cpp line 314:
>
>> 312: ErrnoPreserver ep;
>> 313: log_trace(os, map)("disclaim failed: " RANGEFMT " errno=(%s)",
>> 314: RANGEFMTARGS(p, (long)maxDisclaimSize),
>
> Wait, why are these casts needed? maxDisclaimSize is size_t, RANGEFMT uses
> SIZE_FORMAT. That should work without cast.
Hi Thomas, `maxDisclaimSize` is of type `unsigned int`; therefore I get the
following warning:
os/aix/os_aix.cpp:314:42: error: format specifies type 'unsigned long' but the
argument has type 'unsigned int' [-Werror,-Wformat]
RANGEFMTARGS(p, maxDisclaimSize),
^~~
Should I keep the casts, or change the type of `maxDisclaimSize,
numFullDisclaimsNeeded, lastDisclaimSize` to `const unsigned long`?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547578012
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Tue, 2 Apr 2024 09:14:10 GMT, Joachim Kern wrote: >> Other than that, and kind of depending on your answer: How important is it >> that we catch every use of the original malloc? Can be safely mix the >> original malloc with vec_malloc if logging is not involved? >> >> I am asking, because from that it depends whether this hunk needs to appear >> right behind `#include ` or whether we can move it into the middle >> of the file together with the other AIX stuff. >> >> Because, if we move it into the middle of the file, we may miss any uses of >> malloc that may happen in system headers (would be unusual for that to >> happen but with IBM one never knows). > > Hi Thomas, > I would like to get totally rid of this, because as I mentioned IBM already > modified the `stdlib.h` header not using `#define malloc vec_malloc` any more > (and all the other vec_... defines). We have to ask the adoptium colleagues > at IBM if they already have raised their build environment by the 2 SP levels > needed. > In principle we had to do the same workaround for `calloc, free,...` too, but > they didn't show up as errors in the logging files. > These lines where never meant to stay for long. Just to be able to compile > until IBM fixes the issue, which is done now. @suchismith1993 Hi Suchi, can you please tell me when you will raise your build environment from AIX 7.2 TL5 SP5 to SP7? I' am asking you, because I want to get rid of this nasty workaround. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547473723
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 07:59:05 GMT, Thomas Stuefe wrote: >> While looking at this, I noticed that my question in >> https://github.com/openjdk/jdk/pull/14146#discussion_r1207078176 and >> followups had never been answered. Do you know the answers now? >> >> Quoting myself: >> >>> So, we do this only for malloc? Not for calloc, posix_memalign, realloc >>> etc? What about free? >>> Removing that define and hard-coding it here assumes ... pointers it >>> returns work with the unchanged free() and realloc() the system provides, >>> and will always do so. >>> I am basically worried that undefining malloc, even if it seems harmless >>> now, exposes us to difficult-to-investigate problems down the road, since >>> it depends on how the libc devs will reform those macros in the future. > > Other than that, and kind of depending on your answer: How important is it > that we catch every use of the original malloc? Can be safely mix the > original malloc with vec_malloc if logging is not involved? > > I am asking, because from that it depends whether this hunk needs to appear > right behind `#include ` or whether we can move it into the middle > of the file together with the other AIX stuff. > > Because, if we move it into the middle of the file, we may miss any uses of > malloc that may happen in system headers (would be unusual for that to happen > but with IBM one never knows). Hi Thomas, I would like to get totally rid of this, because as I mentioned IBM already modified the `stdlib.h` header not using `#define malloc vec_malloc` any more (and all the other vec_... defines). We have to ask the adoptium colleagues at IBM if they already have raised their build environment by the 2 SP levels needed. In principle we had to do the same workaround for `calloc, free,...` too, but they didn't show up as errors in the logging files. These lines where never meant to stay for long. Just to be able to compile until IBM fixes the issue, which is done now. - PR Review Comment: https://git.openjdk.org/jdk/pull/18536#discussion_r1547465986
Re: RFR: JDK-8329257: AIX: Switch HOTSPOT_TOOLCHAIN_TYPE from xlc to gcc
On Fri, 29 Mar 2024 05:23:57 GMT, Julian Waters wrote: > > The rest of the changes are needed because of using > > utilities/compilerWarnings_xlc.hpp the compiler is much more nagging about > > ill formatted printf > > Did you mean compilerWarnings_gcc.hpp? Yes, you're right. I fixed it. - PR Comment: https://git.openjdk.org/jdk/pull/18536#issuecomment-2031447588
