Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v7]

[Jorn Vernee]

On Tue, 21 Nov 2023 19:30:30 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Fix Typos
>  - Remove unnecessary includes

Marked as reviewed by jvernee (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1743738561

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v7]

[suchismith1993]

On Tue, 21 Nov 2023 19:30:30 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Fix Typos
>  - Remove unnecessary includes

Yes.The test cases for lookup of symbols have passed.
I think the bexpfull option helps in exporting the right symbols being
referenced.


On Wednesday, November 22, 2023, Jorn Vernee ***@***.***>
wrote:

> Note that on Windows we also have a lookup mechanism on the Java side:
> https://github.com/openjdk/jdk/blob/2c31ca525b1cd70c3dfcb0463c8c98
> 4bdd7c886a/src/java.base/share/classes/jdk/internal/
> foreign/SystemLookup.java#L159
>
> On Windows we need to load the global array, and then grab functions from
> the array. Why isn't that needed on AIX? Is dlsym able to find the
> statically linked functions as well?
>
> —
> Reply to this email directly, view it on GitHub
> , or
> unsubscribe
> 
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

-

PR Comment: https://git.openjdk.org/jdk/pull/16414#issuecomment-1822243041

Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v2]

[Eric Liu]

On Tue, 21 Nov 2023 13:29:32 GMT, Eric Liu  wrote:

>> Got it. I will fix it soon. Thanks!
>
> compiler/vectorapi and jdk/incubator/vector passed. Full test is running. I 
> would report the result when it has been finished.

Full jtreg passed without new failure.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16670#discussion_r1401594486

Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v4]

[Eric Liu]

> Vector API defines zero-extend operations [1], which are going to be 
> intrinsified and generated to `VectorUCastNode` by C2. This patch adds 
> backend implementation for `VectorUCastNode` on AArch64.
> 
> The micro benchmark shows significant performance improvement. In my test 
> machine (SVE, 256-bit), the result is shown as below:
> 
> 
> 
>   Benchmark Before After   Units   Gain
>   VectorZeroExtend.byte2Int 3168.251   243012.399  ops/ms  75.70
>   VectorZeroExtend.byte2Long3212.201   216291.588  ops/ms  66.33
>   VectorZeroExtend.byte2Short   3391.968   182655.365  ops/ms  52.85
>   VectorZeroExtend.int2Long 1012.19780448.553  ops/ms  78.48
>   VectorZeroExtend.short2Int1812.471   153416.828  ops/ms  83.65
>   VectorZeroExtend.short2Long   1788.382   129794.814  ops/ms  71.58
> 
> 
> On other Neon systems, we can get similar performance boost as a result of 
> intrinsification success.
> 
> Since `VectorUCastNode` only used in Vector API's zero extension currently, 
> this patch also adds assertion on nodes' definitions to clarify their usages.
> 
> [TEST]
> compiler/vectorapi and jdk/incubator/vector passed on NEON and SVE machines.
> 
> [1] 
> https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorOperators.java#L726

Eric Liu has updated the pull request incrementally with one additional commit 
since the last revision:

  small fix
  
  Change-Id: Icfe9619af1c9e7d5ea8cac457ccebb4eec5c34ad

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16670/files
  - new: https://git.openjdk.org/jdk/pull/16670/files/68748e7f..268b71db

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16670&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16670&range=02-03

  Stats: 17 lines in 1 file changed: 0 ins; 9 del; 8 mod
  Patch: https://git.openjdk.org/jdk/pull/16670.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16670/head:pull/16670

PR: https://git.openjdk.org/jdk/pull/16670

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v7]

[Jorn Vernee]

On Tue, 21 Nov 2023 19:30:30 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Fix Typos
>  - Remove unnecessary includes

Note that on Windows we also have a lookup mechanism on the Java side: 
https://github.com/openjdk/jdk/blob/2c31ca525b1cd70c3dfcb0463c8c984bdd7c886a/src/java.base/share/classes/jdk/internal/foreign/SystemLookup.java#L159

On Windows we need to load the global array, and then grab functions from the 
array. Why isn't that needed on AIX? Is `dlsym` able to find the statically 
linked functions as well?

-

PR Comment: https://git.openjdk.org/jdk/pull/16414#issuecomment-1822205442

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v7]

[David Holmes]

On Tue, 21 Nov 2023 19:30:30 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Fix Typos
>  - Remove unnecessary includes

This is much more palatable and good to be consistent with what is done on 
Windows. Thank.

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1743602440

Integrated: 8320131: Zero build fails on macOS after JDK-8254693

[Hao Sun]

On Wed, 22 Nov 2023 02:06:29 GMT, Hao Sun  wrote:

> The fix is trivial. We should include the standard C header `stdlib.h` [1], 
> rather than non-standard one `malloc.h`.
> 
> [1] https://en.cppreference.com/w/c/memory/malloc

This pull request has now been integrated.

Changeset: b3616c9a
Author:Hao Sun 
URL:   
https://git.openjdk.org/jdk/commit/b3616c9ac09a29824441dea4588ce53fa443067d
Stats: 2 lines in 1 file changed: 1 ins; 1 del; 0 mod

8320131: Zero build fails on macOS after JDK-8254693

Reviewed-by: dholmes, jvernee

-

PR: https://git.openjdk.org/jdk/pull/16775

Re: RFR: 8320131: Zero build fails on macOS after JDK-8254693

[Hao Sun]

On Wed, 22 Nov 2023 02:06:29 GMT, Hao Sun  wrote:

> The fix is trivial. We should include the standard C header `stdlib.h` [1], 
> rather than non-standard one `malloc.h`.
> 
> [1] https://en.cppreference.com/w/c/memory/malloc

Thanks for your reviews.

-

PR Comment: https://git.openjdk.org/jdk/pull/16775#issuecomment-1822140880

Re: RFR: 8311906: Improve robustness of String constructors with mutable array inputs [v12]

[Roger Riggs]

> Strings, after construction, are immutable but may be constructed from 
> mutable arrays of bytes, characters, or integers.
> The string constructors should guard against the effects of mutating the 
> arrays during construction that might invalidate internal invariants for the 
> correct behavior of operations on the resulting strings. In particular, a 
> number of operations have optimizations for operations on pairs of latin1 
> strings and pairs of non-latin1 strings, while operations between latin1 and 
> non-latin1 strings use a more general implementation. 
> 
> The changes include:
> 
> - Adding a warning to each constructor with an array as an argument to 
> indicate that the results are indeterminate 
>   if the input array is modified before the constructor returns. 
>   The resulting string may contain any combination of characters sampled from 
> the input array.
> 
> - Ensure that strings that are represented as non-latin1 contain at least one 
> non-latin1 character.
>   For latin1 inputs, whether the arrays contain ASCII, ISO-8859-1, UTF8, or 
> another encoding decoded to latin1 the scanning and compression is unchanged.
>   If a non-latin1 character is found, the string is represented as non-latin1 
> with the added verification that a non-latin1 character is present at the 
> same index.
>   If that character is found to be latin1, then the input array has been 
> modified and the result of the scan may be incorrect.
>   Though a ConcurrentModificationException could be thrown, the risk to an 
> existing application of an unexpected exception should be avoided.
>   Instead, the non-latin1 copy of the input is re-scanned and compressed; 
> that scan determines whether the latin1 or the non-latin1 representation is 
> returned.
> 
> - The methods that scan for non-latin1 characters and their intrinsic 
> implementations are updated to return the index of the non-latin1 character.
> 
> - String construction from StringBuilder and CharSequence must also be 
> guarded as their contents may be modified during construction.

Roger Riggs has updated the pull request incrementally with one additional 
commit since the last revision:

  Apply StringUTF16.coderFromArrayLen

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16425/files
  - new: https://git.openjdk.org/jdk/pull/16425/files/0256b9e0..d201344b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16425&range=11
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16425&range=10-11

  Stats: 5 lines in 2 files changed: 0 ins; 0 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/16425.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16425/head:pull/16425

PR: https://git.openjdk.org/jdk/pull/16425

Re: RFR: 8310159: Bulk copy with Unsafe::arrayCopy is slower compared to memcpy [v6]

[Sandhya Viswanathan]

On Tue, 21 Nov 2023 21:03:20 GMT, Steve Dohrmann  wrote:

>> Update: the XorTest::xor results shown in this message used test code from 
>> PR commit 7cc272e862791 which was based on Maurizio Cimadamore's commit 
>> a788f066af17.  The XorTest has since been updated and XorTest::copy is no 
>> longer needed and has been removed from this pull request.  See comment 
>> [here](https://github.com/openjdk/jdk/pull/16575#issuecomment-1820006548) 
>> for updated performance data. 
>> 
>> Below is baseline data collected using a modified version of the 
>> java.lang.foreign.xor micro benchmark referenced by @mcimadamore  in the bug 
>> report.  I collected data on an Ubuntu 22.04 laptop with a Tigerlake 
>> i7-1185G7, which does support AVX512. 
>> 
>> Baseline data
>> Benchmark (arrayKind)  (sizeKind)  Mode  Cnt   Score  
>> Error  Units
>> --
>> XorTest.copy ELEMENTS   SMALL  avgt   30   584737355.767 ± 
>> 60414308.540  ns/op
>> XorTest.copy ELEMENTS  MEDIUM  avgt   30   272248995.683 ±  
>> 2924954.498  ns/op
>> XorTest.copy ELEMENTS   LARGE  avgt   30  1019200210.900 ± 
>> 28334453.652  ns/op
>> XorTest.copy   REGION   SMALL  avgt   30 7399944.164 ±   
>> 216821.819  ns/op
>> XorTest.copy   REGION  MEDIUM  avgt   3020591454.558 ±   
>> 147398.572  ns/op
>> XorTest.copy   REGION   LARGE  avgt   3021649266.051 ±   
>> 179263.875  ns/op
>> XorTest.copy CRITICAL   SMALL  avgt   30   51079.357 ±  
>> 542.482  ns/op
>> XorTest.copy CRITICAL  MEDIUM  avgt   302496.961 ±   
>> 11.375  ns/op
>> XorTest.copy CRITICAL   LARGE  avgt   30 515.454 ±
>> 5.831  ns/op
>> XorTest.copy  FOREIGN   SMALL  avgt   30 7558432.075 ±
>> 79489.276  ns/op
>> XorTest.copy  FOREIGN  MEDIUM  avgt   3019730666.341 ±   
>> 500505.099  ns/op
>> XorTest.copy  FOREIGN   LARGE  avgt   3034616758.085 ±   
>> 340300.726  ns/op
>> XorTest.xor  ELEMENTS   SMALL  avgt   30   219832692.489 ±  
>> 2329417.319  ns/op
>> XorTest.xor  ELEMENTS  MEDIUM  avgt   30   505138197.167 ±  
>> 3818334.424  ns/op
>> XorTest.xor  ELEMENTS   LARGE  avgt   30  1189608474.667 ±  
>> 5877981.900  ns/op
>> XorTest.xorREGION   SMALL  avgt   3064093872.804 ±   
>> 599704.491  ns/op
>> XorTest.xorREGION  MEDIUM  avgt   3081544576.454 ±  
>> 1406342.118  ns/op
>> XorTest.xorREGION   LARGE  avgt   3090091424.883 ±   
>> 775577.613  ns/op
>> XorTest.xor  CRITICAL   SMALL  avgt ...
>
> Steve Dohrmann has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 11 commits:
> 
>  - Merge branch 'master' into memcpy
>  - Updates based on reviewer (sviswa7) comments including
>- use asserts instead of conditionals in two logically unreachable blocks
>- remove unused function parmeters
>- use 64-byte vector path in pre-loop masked write
>  - Merge branch 'master' into memcpy
>  - Update full name
>Previous commit (fcbbc0d7880) added 
> org.openjdk.bench.java.lang.ArrayCopyAlignedLarge benchmark
>  - - remerge upstream master
>- remove ::copy test from XorTest
>  - Merge branch 'master' into memcpy
>  - - fix whitespace error
>  - Merge branch 'master' of https://git.openjdk.org/jdk into memcpy
>  - - bug fix: only generate / use large copy code if MaxVectorSize == 64
>  - - fix whitespace issues
>- fix xor test foreign impl constructor signature
>  - ... and 1 more: https://git.openjdk.org/jdk/compare/e47cf611...02ad27fa

Thanks a lot for taking care of all the review comments. The PR looks good to 
me now.

-

Marked as reviewed by sviswanathan (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16575#pullrequestreview-1743562444

Re: RFR: 8320131: Zero build fails on macOS after JDK-8254693

[Jorn Vernee]

On Wed, 22 Nov 2023 02:06:29 GMT, Hao Sun  wrote:

> The fix is trivial. We should include the standard C header `stdlib.h` [1], 
> rather than non-standard one `malloc.h`.
> 
> [1] https://en.cppreference.com/w/c/memory/malloc

Thanks.

-

Marked as reviewed by jvernee (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16775#pullrequestreview-1743488802

Re: RFR: 8320131: Zero build fails on macOS after JDK-8254693

[David Holmes]

On Wed, 22 Nov 2023 02:06:29 GMT, Hao Sun  wrote:

> The fix is trivial. We should include the standard C header `stdlib.h` [1], 
> rather than non-standard one `malloc.h`.
> 
> [1] https://en.cppreference.com/w/c/memory/malloc

Looks good and trivial. Thanks for fixing.

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16775#pullrequestreview-1743469194

RFR: 8320131: Zero build fails on macOS after JDK-8254693

[Hao Sun]

The fix is trivial. We should include the standard C header `stdlib.h` [1], 
rather than non-standard one `malloc.h`.

[1] https://en.cppreference.com/w/c/memory/malloc

-

Commit messages:
 - 8320131: Zero build fails on macOS after JDK-8254693

Changes: https://git.openjdk.org/jdk/pull/16775/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16775&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8320131
  Stats: 2 lines in 1 file changed: 1 ins; 1 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/16775.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16775/head:pull/16775

PR: https://git.openjdk.org/jdk/pull/16775

Re: RFR: 8318776: Require supports_cx8 to always be true [v4]

[David Holmes]

On Tue, 21 Nov 2023 16:58:21 GMT, Aleksey Shipilev  wrote:

>> This looks great!
>
>> Thanks for the review @fisk ! I have to wait for a few Zero related PRs to 
>> get integrated then re-merge, before I can integrate.
> 
> Zero patches were pushed, please re-merge. I checked current mainline works 
> well with at least linux-arm-zero-fastdebug, and I would like to re-test it 
> with this patch.

@shipilev  I have re-merged and update the Zero changes (ifdef around 
`_saupports_cx8`).

@viktorklang-ora and/or @DougLea could I ask you to look at the 
`java.util.concurrent.AtomicLongFieldUpdater` changes please.

-

PR Comment: https://git.openjdk.org/jdk/pull/16625#issuecomment-1821983975

Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v3]

[Eric Liu]

On Tue, 21 Nov 2023 15:07:48 GMT, Andrew Haley  wrote:

>> Eric Liu has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   add _sve_xunpk & remove dead code
>>   
>>   Change-Id: Ic19836feb8a73ea7e65443794f2a0eb1363f6e2f
>
> src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 1412:
> 
>> 1410:   _sve_xunpk(is_unsigned, /* is_high */ false, dst, S, dst);
>> 1411:   _sve_xunpk(is_unsigned, /* is_high */ false, dst, D, dst);
>> 1412:   break;
> 
> Why is this change here? It doesn't do anything. Does it?

`is_unsigned` is also used in this function. It is used in VectorUCastNode for 
zero extending.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16670#discussion_r1401423194

Re: RFR: 8318776: Require supports_cx8 to always be true [v5]

[David Holmes]

> As discussed in JBS all platforms (some tweaks to Zero are in progress) 
> actually do support `cx8` i.e. 64-bit compare-and-exchange, so we can strip 
> out the locked-based alternatives to using it and just add a guarantee that 
> it is true at runtime. And all platforms except some ARM variants set 
> `SUPPORTS_NATIVE_CX8`, so we can greatly simplify things. Summary of changes:
> - `_supports_cx8` field is only needed when `SUPPORTS_NATIVE_CX8` is not 
> defined
> - Assertions for `supports_cx8()` are removed
> - Compiler predicates requiring `supports_cx8()` are removed
> - Access backend is greatly simplified without the need for lock-based 
> alternative
> - `java.util.concurrent.AtomicLongFieldUpdater` is simplified without the 
> need for a lock-based alternative
> 
> I did consider moving all the ARM `kuser_helper` related code to be only 
> defined when `SUPPORTS_NATIVE_CX8` is not defined, but there was a 
> theoretical risk this could change the behaviour if ARMv7 binaries were run 
> on other ARM CPU's. I added a note to that effect in 
> vm_version_linux_arm32.cpp so the ARM port maintainers could clean this up 
> further if desired.
> 
> Testing:
> - All Oracle tiers 1-5 builds (which includes an ARMv7 build)
> - GHA builds/tests
> - Oracle tiers 1-3 sanity testing
> 
> Zero changes coming in via 
> [JDK-8319777](https://bugs.openjdk.org/browse/JDK-8319777) will be merged 
> when they arrive.
> 
> Thanks.

David Holmes 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 seven additional commits since 
the last revision:

 - Merge with master and update Zero code accordingly
 - Merge branch 'master' into 8318776-supports_cx8
 - Remove unnecessary includes of vm_version.hpp.
   Fix copyright years.
 - Remove cx8 comment as no longer relevant (the spinlock is used regardless of 
cx8)
 - Remove suports_cx8() checks from gtest
 - Remove test for VMSupportsCX8
 - 8318776: Require supports_cx8 to always be true

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16625/files
  - new: https://git.openjdk.org/jdk/pull/16625/files/597cef53..aad0a4c4

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16625&range=04
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16625&range=03-04

  Stats: 621905 lines in 1279 files changed: 89413 ins; 471113 del; 61379 mod
  Patch: https://git.openjdk.org/jdk/pull/16625.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16625/head:pull/16625

PR: https://git.openjdk.org/jdk/pull/16625

Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]

[Xiaohong Gong]

On Wed, 22 Nov 2023 00:05:26 GMT, Paul Sandoz  wrote:

> Have you considered the possibility of copying the sleef source to the 
> OpenJDK repository and thereby it becomes part of the build process? I don't 
> know how straightforward that is technically and IANAL but I think it's worth 
> exploring.
> 

Hi @PaulSandoz ! Thanks for the suggestion! Copying the sleef source sounds 
good. However, I actually have no idea about how to handle the third-party 
licence in OpenJDK project. Do you have any idea about this area? Some 
suggestions/guidence   from the JDK team will be much helpful. Thanks!

-

PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1821965636

Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]

[Xiaohong Gong]

On Tue, 21 Nov 2023 18:14:41 GMT, Paul Sandoz  wrote:

> > This looks good. As far as I can tell the choice you've made of accuracy 
> > matches what we need to meet the spec.
> 
> Same here . Sinh/cosh/tanh/expm1 are specified to be within 2.5 ulps of the 
> exact result, but i believe sleef does not offer that option, it's either 
> within 1 or 3.5, so we have to pick the former. AFAICT sleef does not say 
> anything about monotonicity, but we relax semi-monotonicity for all but sqrt 
> (we defer to IEEE 754).

Yes, that's why we at least have to use the 1up in sleef here.

-

PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1821956522

Re: RFR: 8318650: Optimized subword gather for x86 targets. [v7]

[Quan Anh Mai]

On Wed, 15 Nov 2023 02:17:58 GMT, Jatin Bhateja  wrote:

>> Hi All,
>> 
>> This patch optimizes sub-word gather operation for x86 targets with AVX2 and 
>> AVX512 features.
>> 
>> Following is the summary of changes:-
>> 
>> 1) Intrinsify sub-word gather with high performance backend implementation 
>> based on hybrid algorithm which initially partially unrolls scalar loop to 
>> accumulates values from gather indices into a quadword(64bit) slice followed 
>> by vector permutation to place the slice into appropriate vector lanes, it 
>> prevents code bloating and generates compact
>> JIT sequence. This coupled with savings from expansive array allocation in 
>> existing java implementation translates into significant performance of 
>> 1.3-5x gains with included micro.
>> 
>> 
>> ![image](https://github.com/openjdk/jdk/assets/59989778/e25ba4ad-6a61-42fa-9566-452f741a9c6d)
>> 
>> 
>> 2) Patch was also compared against modified java fallback implementation by 
>> replacing temporary array allocation with zero initialized vector and a 
>> scalar loops which inserts gathered values into vector. But, vector insert 
>> operation in higher vector lanes is a three step process which first 
>> extracts the upper vector 128 bit lane, updates it with gather subword value 
>> and then inserts the lane back to its original position. This makes inserts 
>> into higher order lanes costly w.r.t to proposed solution. In addition 
>> generated JIT code for modified fallback implementation was very bulky. This 
>> may impact in-lining decisions into caller contexts.
>> 
>> 3) Some minor adjustments in existing gather instruction pattens for 
>> double/quad words.
>> 
>> 
>> Kindly review and share your feedback.
>> 
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix incorrect comment

But since accesses are aligned they must lie in the same page, which means if a 
byte is accessible then the whole double word is accessible

-

PR Comment: https://git.openjdk.org/jdk/pull/16354#issuecomment-1821952667

Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]

[Paul Sandoz]

On Wed, 15 Nov 2023 01:32:00 GMT, Xiaohong Gong  wrote:

>> Currently the vector floating-point math APIs like 
>> `VectorOperators.SIN/COS/TAN...` are not intrinsified on AArch64 platform, 
>> which causes large performance gap on AArch64. Note that those APIs are 
>> optimized by C2 compiler on X86 platforms by calling Intel's SVML code [1]. 
>> To close the gap, we would like to optimize these APIs for AArch64 by 
>> calling a third-party vector library called libsleef [2], which are 
>> available in mainstream Linux distros (e.g. [3] [4]).
>> 
>> SLEEF supports multiple accuracies. To match Vector API's requirement and 
>> implement the math ops on AArch64, we 1) call 1.0 ULP accuracy with FMA 
>> instructions used stubs in libsleef for most of the operations by default, 
>> and 2) add the vector calling convention to apply with the runtime calls to 
>> stub code in libsleef. Note that for those APIs that libsleef does not 
>> support 1.0 ULP, we choose 0.5 ULP instead.
>> 
>> To help loading the expected libsleef library, this patch also adds an 
>> experimental JVM option (i.e. `-XX:UseSleefLib`) for AArch64 platforms. 
>> People can use it to denote the libsleef path/name explicitly. By default, 
>> it points to the system installed library. If the library does not exist or 
>> the dynamic loading of it in runtime fails, the math vector ops will 
>> fall-back to use the default scalar version without error. But a warning is 
>> printed out if people specifies a nonexistent library explicitly.
>> 
>> Note that this is a part of the original proposed patch in panama-dev [5], 
>> just with some initial review comments addressed. And now we'd like to get 
>> some wider feedbacks from more hotspot experts.
>> 
>> [1] https://github.com/openjdk/jdk/pull/3638
>> [2] https://sleef.org/
>> [3] https://packages.fedoraproject.org/pkgs/sleef/sleef/
>> [4] https://packages.debian.org/bookworm/libsleef3
>> [5] https://mail.openjdk.org/pipermail/panama-dev/2022-December/018172.html
>
> Xiaohong Gong has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Add a bundled native lib in jdk as a bridge to libsleef
>  - Merge 'jdk:master' into JDK-8312425
>  - Disable sleef by default
>  - Merge 'jdk:master' into JDK-8312425
>  - 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF

Have you considered the possibility of copying the sleef source to the OpenJDK 
repository and thereby it becomes part of the build process? I don't know how 
straightforward that is technically and IANAL but I think it's worth exploring.

Also it may enable us to use sleef for other platforms where we have gaps 
(looking at Table 1.1 of https://sleef.org/). Further out it should inspire us 
to do a Java Vector API port to as indicated in a prior comment.  

> Yes, libsleef is used to build the binary if found. And at runtime, if the 
> libsleef with right version is not found, `dlopen` to the libvmath.so will 
> fail. And then all the operations will be fall-back to the java default 
> implementation. X86_64 has also bundled the Intel's SVML binary to jdk image 
> at build time. And we use the same way loading/opening the library at runtime.

-

PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1821885433

Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref

[Roger Riggs]

On Mon, 13 Nov 2023 22:31:16 GMT, Brent Christian  wrote:

> Classes in the `java.lang.ref` package would benefit from an update to bring 
> the spec in line with how the VM already behaves. The changes would focus on 
> _happens-before_ edges at some key points during reference processing.
> 
> A couple key things we want to be able to say are:
> - `Reference.reachabilityFence(x)` _happens-before_ reference processing 
> occurs for 'x'.
> - `Cleaner.register()` _happens-before_ the Cleaner thread runs the 
> registered cleaning action.
> 
> This will bring Cleaner in line (or close) with the memory visibility 
> guarantees made for finalizers in [JLS 
> 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5):
> _"There is a happens-before edge from the end of a constructor of an object 
> to the start of a finalizer (§12.6) for that object."_

src/java.base/share/classes/java/lang/ref/Reference.java line 552:

> 550:  * of this method.
> 551:  * Invocation of this method does not itself initiate reference 
> processing,
> 552:  * garbage collection, or finalization.

My understanding was that it is not the object instance that is being guarded, 
only that the reference holding the object is considered a strong root and is 
only used to delimit a range of bytecodes for which the reference is considered 
to be strong.
In particular, the invocation of the method itself has no semantics, only that 
a control flow could reach that statement and the reference was considered 
strong as long as the reference was in a scope that included the reachability 
fence.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1401310991

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v7]

[Magnus Ihse Bursie]

On Tue, 21 Nov 2023 19:30:30 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Fix Typos
>  - Remove unnecessary includes

Marked as reviewed by ihse (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1743290191

Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref

[Roger Riggs]

On Wed, 15 Nov 2023 22:31:35 GMT, Stuart Marks  wrote:

>> Classes in the `java.lang.ref` package would benefit from an update to bring 
>> the spec in line with how the VM already behaves. The changes would focus on 
>> _happens-before_ edges at some key points during reference processing.
>> 
>> A couple key things we want to be able to say are:
>> - `Reference.reachabilityFence(x)` _happens-before_ reference processing 
>> occurs for 'x'.
>> - `Cleaner.register()` _happens-before_ the Cleaner thread runs the 
>> registered cleaning action.
>> 
>> This will bring Cleaner in line (or close) with the memory visibility 
>> guarantees made for finalizers in [JLS 
>> 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5):
>> _"There is a happens-before edge from the end of a constructor of an object 
>> to the start of a finalizer (§12.6) for that object."_
>
> src/java.base/share/classes/java/lang/ref/Reference.java line 489:
> 
>> 487:  * If this reference was already enqueued (by the garbage 
>> collector, or a
>> 488:  * previous call to {@code enqueue}), this method is not 
>> successful,
>> 489:  * and returns false.
> 
> If we're going to talk about successful and unsuccessful calls, we should 
> define what success is first. I guess that would be something like: if this 
> ref is registered with a queue and not already enqueued, it is enqueued 
> successfully and the method returns true. Otherwise, not successful, and 
> returns false.

This could be worded as a post condition, after the call the ref is enqueued 
with the queue; the return is true iff the ref was not previously enqueued.
The enqueuing is not conditional (assuming the queue is non-null).

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1401302710

Re: RFR: 8310159: Bulk copy with Unsafe::arrayCopy is slower compared to memcpy [v5]

[Steve Dohrmann]

On Tue, 21 Nov 2023 01:14:49 GMT, Sandhya Viswanathan 
 wrote:

>> Steve Dohrmann has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains ten commits:
>> 
>>  - Merge branch 'master' into memcpy
>>  - Update full name
>>Previous commit (fcbbc0d7880) added 
>> org.openjdk.bench.java.lang.ArrayCopyAlignedLarge benchmark
>>  - - remerge upstream master
>>- remove ::copy test from XorTest
>>  - Merge branch 'master' into memcpy
>>  - - fix whitespace error
>>  - Merge branch 'master' of https://git.openjdk.org/jdk into memcpy
>>  - - bug fix: only generate / use large copy code if MaxVectorSize == 64
>>  - - fix whitespace issues
>>- fix xor test foreign impl constructor signature
>>  - - initial commit -- optimize large array cases in 
>> StubGenerator::generate_disjoint_copy_avx3_masked
>>  - add src address prefetches
>>  - switch to non-temporal writes
>>  - added modified jmh benchmark based on xor benchmark from Maurizio 
>> Cimadamore
>
> src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp line 768:
> 
>> 766: }
>> 767: __ movq(temp3, temp2);
>> 768: copy64_masked_avx(to, from, xmm1, k2, temp3, temp4, temp1, shift, 
>> 0);
> 
> The last argument should be "true" or "1" instead of "0" or "false".  This is 
> as temp3 (length) could be less than 32 as well. This case is only handled 
> when use64byteVector argument is true.

Thanks, done.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16575#discussion_r1401229608

Re: RFR: JDK-8319662 ForkJoinPool trims worker threads too slowly [v3]

[Viktor Klang]

On Tue, 21 Nov 2023 17:12:39 GMT, Doug Lea  wrote:

>> This update cascades timeouts to trim subsequent workers after the first  
>> keepAlive inactive period.
>
> Doug Lea 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:
> 
>  - Merge branch 'openjdk:master' into JDK-8319662
>  - tweak cascades; reinstate an @Contended; resolve JDK-8319498
>  - Support cascading idle timeouts

src/java.base/share/classes/java/util/concurrent/ForkJoinPool.java line 3116:

> 3114: } catch (Exception ex) {
> 3115: throw new RuntimeException(ex);
> 3116: }

Suggestion:

try {
return task.join();
} catch (RuntimeException | Error unchecked) {
throw unchecked;
} catch (Exception checked) {
throw new RuntimeException(checked);
}

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16725#discussion_r1401218927

Re: RFR: 8319668: Fixup of jar filename typo in BadFactoryTest.sh

[Eirik Bjorsnos]

On Thu, 9 Nov 2023 16:49:41 GMT, Gaurav Chaudhari  wrote:

> The file test/jdk/javax/script/JDK_8196959/BadFactoryTest.sh contains a typo. 
> When running without security manager, the test references 'badfactoty.jar' 
> instead of 'badfactory.jar'. This change addresses this by correcting the jar 
> name.

The typo you found here reveals that the test runs fine without having the jar 
file on the classpath. All the necessary class files already seem to be in 
`${TESTCLASSES}`

So it's not clear to me why this test uses a jar file at all, it does not seem 
necessary.

Perhaps you could  clean this up in this PR? That would include:

- Removing the code which creates the JAR
- Updating the two `-classpath` options to just `"${TESTCLASSES}"`

If you feel this is out of scope for this PR, I'm fine with that as well.

-

PR Comment: https://git.openjdk.org/jdk/pull/16585#issuecomment-1821701137

Re: RFR: 8310159: Bulk copy with Unsafe::arrayCopy is slower compared to memcpy [v6]

[Steve Dohrmann]

> Update: the XorTest::xor results shown in this message used test code from PR 
> commit 7cc272e862791 which was based on Maurizio Cimadamore's commit 
> a788f066af17.  The XorTest has since been updated and XorTest::copy is no 
> longer needed and has been removed from this pull request.  See comment 
> [here](https://github.com/openjdk/jdk/pull/16575#issuecomment-1820006548) for 
> updated performance data. 
> 
> Below is baseline data collected using a modified version of the 
> java.lang.foreign.xor micro benchmark referenced by @mcimadamore  in the bug 
> report.  I collected data on an Ubuntu 22.04 laptop with a Tigerlake 
> i7-1185G7, which does support AVX512. 
> 
> Baseline data
> Benchmark (arrayKind)  (sizeKind)  Mode  Cnt   Score  
> Error  Units
> --
> XorTest.copy ELEMENTS   SMALL  avgt   30   584737355.767 ± 
> 60414308.540  ns/op
> XorTest.copy ELEMENTS  MEDIUM  avgt   30   272248995.683 ±  
> 2924954.498  ns/op
> XorTest.copy ELEMENTS   LARGE  avgt   30  1019200210.900 ± 
> 28334453.652  ns/op
> XorTest.copy   REGION   SMALL  avgt   30 7399944.164 ±   
> 216821.819  ns/op
> XorTest.copy   REGION  MEDIUM  avgt   3020591454.558 ±   
> 147398.572  ns/op
> XorTest.copy   REGION   LARGE  avgt   3021649266.051 ±   
> 179263.875  ns/op
> XorTest.copy CRITICAL   SMALL  avgt   30   51079.357 ±  
> 542.482  ns/op
> XorTest.copy CRITICAL  MEDIUM  avgt   302496.961 ±   
> 11.375  ns/op
> XorTest.copy CRITICAL   LARGE  avgt   30 515.454 ±
> 5.831  ns/op
> XorTest.copy  FOREIGN   SMALL  avgt   30 7558432.075 ±
> 79489.276  ns/op
> XorTest.copy  FOREIGN  MEDIUM  avgt   3019730666.341 ±   
> 500505.099  ns/op
> XorTest.copy  FOREIGN   LARGE  avgt   3034616758.085 ±   
> 340300.726  ns/op
> XorTest.xor  ELEMENTS   SMALL  avgt   30   219832692.489 ±  
> 2329417.319  ns/op
> XorTest.xor  ELEMENTS  MEDIUM  avgt   30   505138197.167 ±  
> 3818334.424  ns/op
> XorTest.xor  ELEMENTS   LARGE  avgt   30  1189608474.667 ±  
> 5877981.900  ns/op
> XorTest.xorREGION   SMALL  avgt   3064093872.804 ±   
> 599704.491  ns/op
> XorTest.xorREGION  MEDIUM  avgt   3081544576.454 ±  
> 1406342.118  ns/op
> XorTest.xorREGION   LARGE  avgt   3090091424.883 ±   
> 775577.613  ns/op
> XorTest.xor  CRITICAL   SMALL  avgt   3057231375.744 ±   
> 438223.342  ns/op
> XorTest.x...

Steve Dohrmann has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 11 commits:

 - Merge branch 'master' into memcpy
 - Updates based on reviewer (sviswa7) comments including
   - use asserts instead of conditionals in two logically unreachable blocks
   - remove unused function parmeters
   - use 64-byte vector path in pre-loop masked write
 - Merge branch 'master' into memcpy
 - Update full name
   Previous commit (fcbbc0d7880) added 
org.openjdk.bench.java.lang.ArrayCopyAlignedLarge benchmark
 - - remerge upstream master
   - remove ::copy test from XorTest
 - Merge branch 'master' into memcpy
 - - fix whitespace error
 - Merge branch 'master' of https://git.openjdk.org/jdk into memcpy
 - - bug fix: only generate / use large copy code if MaxVectorSize == 64
 - - fix whitespace issues
   - fix xor test foreign impl constructor signature
 - ... and 1 more: https://git.openjdk.org/jdk/compare/e47cf611...02ad27fa

-

Changes: https://git.openjdk.org/jdk/pull/16575/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16575&range=05
  Stats: 259 lines in 5 files changed: 259 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/16575.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16575/head:pull/16575

PR: https://git.openjdk.org/jdk/pull/16575

Re: RFR: 8310159: Bulk copy with Unsafe::arrayCopy is slower compared to memcpy [v5]

[Steve Dohrmann]

On Tue, 21 Nov 2023 01:10:40 GMT, Sandhya Viswanathan 
 wrote:

>> Steve Dohrmann has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains ten commits:
>> 
>>  - Merge branch 'master' into memcpy
>>  - Update full name
>>Previous commit (fcbbc0d7880) added 
>> org.openjdk.bench.java.lang.ArrayCopyAlignedLarge benchmark
>>  - - remerge upstream master
>>- remove ::copy test from XorTest
>>  - Merge branch 'master' into memcpy
>>  - - fix whitespace error
>>  - Merge branch 'master' of https://git.openjdk.org/jdk into memcpy
>>  - - bug fix: only generate / use large copy code if MaxVectorSize == 64
>>  - - fix whitespace issues
>>- fix xor test foreign impl constructor signature
>>  - - initial commit -- optimize large array cases in 
>> StubGenerator::generate_disjoint_copy_avx3_masked
>>  - add src address prefetches
>>  - switch to non-temporal writes
>>  - added modified jmh benchmark based on xor benchmark from Maurizio 
>> Cimadamore
>
> src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp line 753:
> 
>> 751:   Label L_pre_main_post_large;
>> 752: 
>> 753:   if (MaxVectorSize == 64) {
> 
> This should be an assert here instead of if check as this method shouldn't be 
> called if MaxVectorSize is < 64.

Thanks, done.

> src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp line 777:
> 
>> 775: 
>> 776: __ BIND(L_main_pre_loop_large);
>> 777: __ subq(temp1, loop_size[shift]);  // whay is this here
> 
> Spurious comment.

Thanks, done

> src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp line 797:
> 
>> 795: __ jcc(Assembler::lessEqual, L_exit_large);
>> 796: arraycopy_avx3_special_cases_256(xmm1, k2, from, to, temp1, shift,
>> 797:  temp4, temp3, L_entry_large, 
>> L_exit_large);
> 
> When we come here to arraycopy_avx3_special_cases_256 only up to 256 bytes 
> need to be copied so we don't need to go back to L_entry_large.

Thanks, done

> src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp line 1058:
> 
>> 1056:   };
>> 1057: 
>> 1058:   if (MaxVectorSize == 64) {
> 
> This should be an assert here instead of if check as this method shouldn't be 
> called if MaxVectorSize is < 64.

Thanks, done.

> src/hotspot/cpu/x86/stubGenerator_x86_64_arraycopy.cpp line 1175:
> 
>> 1173: void StubGenerator::copy256_avx3(Register dst, Register src, Register 
>> index, XMMRegister xmm1,
>> 1174: XMMRegister xmm2, XMMRegister xmm3, 
>> XMMRegister xmm4,
>> 1175: bool conjoint, int shift, int offset) {
> 
> The conjoint parameter is not used so could be removed from this function.

Thanks, done.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16575#discussion_r1401179711
PR Review Comment: https://git.openjdk.org/jdk/pull/16575#discussion_r1401178991
PR Review Comment: https://git.openjdk.org/jdk/pull/16575#discussion_r1401180641
PR Review Comment: https://git.openjdk.org/jdk/pull/16575#discussion_r1401180258
PR Review Comment: https://git.openjdk.org/jdk/pull/16575#discussion_r1401177983

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v31]

[Jonathan Gibbons]

On Tue, 21 Nov 2023 18:51:55 GMT, Jim Laskey  wrote:

> The comments are attached to the modifiers (first thing it encounters.) I’m 
> sure the javadoc toolset has a method that gets the comments from the right 
> thing.

In the AST created by the parser, doc comments should be attached to 
_declarations_ (`JCClassDecl`, `JCMethodDecl`, `JCVariableDecl` etc) not 
modifiers.  There is nothing in the javadoc toolset that _gets the comments 
from the right thing_.

-

PR Comment: https://git.openjdk.org/jdk/pull/16461#issuecomment-1821645764

Re: RFR: 8310994: Add JFR event for selection operations

[Alan Bateman]

On Fri, 17 Nov 2023 16:22:55 GMT, Tim Prinzing  wrote:

> Added mirror event with static methods: jdk.internal.event.SelectionEvent 
> that provides the duration of select calls and the count of how many keys are 
> available.
> 
> Emit the event from SelectorImpl::lockAndDoSelect
> 
> Test at jdk.jfr.event.io.TestSelectionEvents

src/java.base/share/classes/sun/nio/ch/SelectorImpl.java line 141:

> 139: SelectionEvent.commit(start, duration, n);
> 140: }
> 141: return n;

I'd prefer if the existing code moved to implLockAndDoSelect so that 
lockAndDoSelect commits the event outside of the locked region.

test/jdk/jdk/jfr/event/io/TestSelectionEvents.java line 70:

> 68: SocketChannel sc2 = ssc.accept()) {
> 69: 
> 70: Selector sel = Selector.open();

You can add this to the try block so that the selector is closed too.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16710#discussion_r1397596209
PR Review Comment: https://git.openjdk.org/jdk/pull/16710#discussion_r1397593894

RFR: 8310994: Add JFR event for selection operations

[Tim Prinzing]

Added mirror event with static methods: jdk.internal.event.SelectionEvent that 
provides the duration of select calls and the count of how many keys are 
available.

Emit the event from SelectorImpl::lockAndDoSelect

Test at jdk.jfr.event.io.TestSelectionEvents

-

Commit messages:
 - remove trailing whitespace
 - event logic outside of the lock, selector in try block
 - remove unused import
 - fix TestConfigure failure
 - add event defaults
 - Merge branch 'master' into JDK-8310994
 - minor test cleanup
 - Merge branch 'master' into JDK-8310994
 - 8310994: Add JFR event for selection operations

Changes: https://git.openjdk.org/jdk/pull/16710/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16710&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8310994
  Stats: 265 lines in 9 files changed: 264 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/16710.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16710/head:pull/16710

PR: https://git.openjdk.org/jdk/pull/16710

Re: RFR: 8319668: Fixup of jar filename typo in BadFactoryTest.sh

[Eirik Bjorsnos]

On Tue, 21 Nov 2023 16:36:26 GMT, Alan Bateman  wrote:

>> The file test/jdk/javax/script/JDK_8196959/BadFactoryTest.sh contains a 
>> typo. When running without security manager, the test references 
>> 'badfactoty.jar' instead of 'badfactory.jar'. This change addresses this by 
>> correcting the jar name.
>
> Looks okay. This test is begging to be re-written in Java, maybe some day.
> 
> I assume the copyright header will be updated before this change is 
> integrated.

> Hi @AlanBateman, do I have to update the copyright year to 2023 manually and 
> amend the commit before `/integrate` ?

@Deigue 

Not sure if this was a question on the formatting of the copyright header, but 
the year should not simply be updated to `2023`, but instead to the range 
`2018, 2023`. So in your case, the first line should read:


# Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.


Once you push that commit, a reviewer can approve the PR and you can 
`/integrate`

-

PR Comment: https://git.openjdk.org/jdk/pull/16585#issuecomment-1821612336

Re: RFR: 8319668: Fixup of jar filename typo in BadFactoryTest.sh

[Gaurav Chaudhari]

On Tue, 21 Nov 2023 16:36:26 GMT, Alan Bateman  wrote:

> Looks okay. This test is begging to be re-written in Java, maybe some day.
> 
> I assume the copyright header will be updated before this change is 
> integrated.

Hi @AlanBateman, do I have to update the copyright year to 2023 manually and 
amend the commit before `/integrate` ?

-

PR Comment: https://git.openjdk.org/jdk/pull/16585#issuecomment-1821594845

Re: RFR: 8318650: Optimized subword gather for x86 targets. [v7]

[Jatin Bhateja]

On Wed, 15 Nov 2023 02:17:58 GMT, Jatin Bhateja  wrote:

>> Hi All,
>> 
>> This patch optimizes sub-word gather operation for x86 targets with AVX2 and 
>> AVX512 features.
>> 
>> Following is the summary of changes:-
>> 
>> 1) Intrinsify sub-word gather with high performance backend implementation 
>> based on hybrid algorithm which initially partially unrolls scalar loop to 
>> accumulates values from gather indices into a quadword(64bit) slice followed 
>> by vector permutation to place the slice into appropriate vector lanes, it 
>> prevents code bloating and generates compact
>> JIT sequence. This coupled with savings from expansive array allocation in 
>> existing java implementation translates into significant performance of 
>> 1.3-5x gains with included micro.
>> 
>> 
>> ![image](https://github.com/openjdk/jdk/assets/59989778/e25ba4ad-6a61-42fa-9566-452f741a9c6d)
>> 
>> 
>> 2) Patch was also compared against modified java fallback implementation by 
>> replacing temporary array allocation with zero initialized vector and a 
>> scalar loops which inserts gathered values into vector. But, vector insert 
>> operation in higher vector lanes is a three step process which first 
>> extracts the upper vector 128 bit lane, updates it with gather subword value 
>> and then inserts the lane back to its original position. This makes inserts 
>> into higher order lanes costly w.r.t to proposed solution. In addition 
>> generated JIT code for modified fallback implementation was very bulky. This 
>> may impact in-lining decisions into caller contexts.
>> 
>> 3) Some minor adjustments in existing gather instruction pattens for 
>> double/quad words.
>> 
>> 
>> Kindly review and share your feedback.
>> 
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix incorrect comment

> I have not thought about this thoroughly so it may be incorrect. Can we 
> delegate the gather to `int` and extract the result from it.
> 
> ```
> vpand(xtmp1, idx, bt == T_SHORT ? -2 : -4); // align the index so that the 
> address is aligned for int accesses
> vpgatherdd(xtmp2, mask, Address(base, xtmp1, bt == T_SHORT ? times_2 : 
> times_1, offset));
> vpand(xtmp1, idx, bt == T_SHORT ? 1 : 3); // Need to align the requested 
> elements
> vpslld(xtmp1, xtmp1, bt == T_SHORT ? 4 : 3);
> vpsrlvd(xtmp1, xtmp2, xtmp1);
> vpmovdw(dst, xtmp1);
> ```



> I have not thought about this thoroughly so it may be incorrect. Can we 
> delegate the gather to `int` and extract the result from it.
> 
> ```
> vpand(xtmp1, idx, bt == T_SHORT ? -2 : -4); // align the index so that the 
> address is aligned for int accesses
> vpgatherdd(xtmp2, mask, Address(base, xtmp1, bt == T_SHORT ? times_2 : 
> times_1, offset));
> vpand(xtmp1, idx, bt == T_SHORT ? 1 : 3); // Need to align the requested 
> elements
> vpslld(xtmp1, xtmp1, bt == T_SHORT ? 4 : 3);
> vpsrlvd(xtmp1, xtmp2, xtmp1);
> vpmovdw(dst, xtmp1);
> ```

Double word gather will always try to access 4 contiguous byte from a 
normalized index, and will not be able to prevent access violations if other 3 
bytes in double word are non accessible.

-

PR Comment: https://git.openjdk.org/jdk/pull/16354#issuecomment-1821586982

Re: RFR: JDK-8319569: Several java/util tests should be updated to accept VM flags [v3]

[Lance Andersen]

On Mon, 20 Nov 2023 19:15:18 GMT, Justin Lu  wrote:

>> Please review this PR which allows these _j.util_ tests to launch new JVM 
>> processes with VM flags,
>> 
>> This is primarily done using by switching to 
>> `ProcessTools::createTestJavaProcessBuilder`.
>
> Justin Lu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Pass test.java.opts to PropertiesTest.sh

Marked as reviewed by lancea (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/16705#pullrequestreview-1742959505

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v31]

[Jim Laskey]

On Tue, 21 Nov 2023 19:16:06 GMT, Alan Bateman  wrote:

>> Jim Laskey has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update Class.java
>
> src/java.base/share/classes/java/lang/Class.java line 4823:
> 
>> 4821:  * signatures in a class."
>> 4822:  * {@link SecurityException SecurityExceptions} can halt
>> 4823:  * the search. In this case, a null is returned.
> 
> I see the latest update changes this to return null if denied by the SM. This 
> seems very inconsistent with the other methods so I'm wondering why this has 
> changed?

Requested by CSR.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16461#discussion_r1401089920

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v7]

[suchismith1993]

> The math library in AIX specifically, is a static archive. Doing a -lm wont 
> suffice, because when the symbols are looked up using dlsym or accessing 
> native code through Java, it will lead to failures.
> Hence we had to come up with a list of symbols to allow math library symbols 
> to be accesible.
> Also, there are parts of libc library that are static too, and hence those 
> symbols also are present in this list.
> Without this change, the StdLibTest and multiple other tests which make 
> native function calls using FFI, fail with NoSuchElementException.
> 
> 
> 1. Adding required compiler flags.
> 2. Adding required symbols.
> 
> 
> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)

suchismith1993 has updated the pull request incrementally with two additional 
commits since the last revision:

 - Fix Typos
 - Remove unnecessary includes

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16414/files
  - new: https://git.openjdk.org/jdk/pull/16414/files/48a69c59..efbb245a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=06
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=05-06

  Stats: 6 lines in 1 file changed: 1 ins; 3 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/16414.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16414/head:pull/16414

PR: https://git.openjdk.org/jdk/pull/16414

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v7]

[Martin Doerr]

On Tue, 21 Nov 2023 19:30:30 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Fix Typos
>  - Remove unnecessary includes

Thanks for cleaning it up! LGTM.

-

Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1742925562

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v31]

[Alan Bateman]

On Tue, 21 Nov 2023 17:52:49 GMT, Jim Laskey  wrote:

>> Address changes from JEP 445 to JEP 463.
>> 
>> - Move from a SYNTHETIC unnamed class to a MANDATED implicit class.
>> 
>> - Don't mark class on read.
>> 
>> - Remove reflection and annotation processing related to unnamed classes.
>> 
>> - Simplify main method search.
>
> Jim Laskey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update Class.java

src/java.base/share/classes/java/lang/Class.java line 4823:

> 4821:  * signatures in a class."
> 4822:  * {@link SecurityException SecurityExceptions} can halt
> 4823:  * the search. In this case, a null is returned.

I see the latest update changes this to return null if denied by the SM. This 
seems very inconsistent with the other methods so I'm wondering why this has 
changed?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16461#discussion_r1401056196

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v31]

[Pavel Rappo]

On Tue, 21 Nov 2023 18:59:51 GMT, Pavel Rappo  wrote:

> I'll try to figure out more.

So this method seems to be a proximate-ish cause for that behaviour: 
`JavacParser.topLevelMethodOrFieldDeclaration`. If it gets `null` or comment to 
attach to `JCTree.JCMethodDecl` later depends on the presence of modifiers. 
This is not the case with normal class methods, where the comments are robustly 
attached regardless of whether modifiers are present.

-

PR Comment: https://git.openjdk.org/jdk/pull/16461#issuecomment-1821538640

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v31]

[Pavel Rappo]

On Tue, 21 Nov 2023 18:51:55 GMT, Jim Laskey  wrote:

> I’m sure the javadoc toolset has a method that gets the comments from the 
> right thing.

That's exactly where those comments disappear from, javadoc. Comments of normal 
class methods and comments of methods of implicitly declared class seem to be 
tree-ified differently. Can these comments become lost during 
copying/reconstruction? I'll try to figure out more.

-

PR Comment: https://git.openjdk.org/jdk/pull/16461#issuecomment-1821492952

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v31]

[Jim Laskey]

On Tue, 21 Nov 2023 17:52:49 GMT, Jim Laskey  wrote:

>> Address changes from JEP 445 to JEP 463.
>> 
>> - Move from a SYNTHETIC unnamed class to a MANDATED implicit class.
>> 
>> - Don't mark class on read.
>> 
>> - Remove reflection and annotation processing related to unnamed classes.
>> 
>> - Simplify main method search.
>
> Jim Laskey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update Class.java

The comments are attached to the modifiers (first thing it encounters.) I’m 
sure the javadoc toolset has a method that gets the comments from the right 
thing.

-

PR Comment: https://git.openjdk.org/jdk/pull/16461#issuecomment-1821475235

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v19]

[Pavel Rappo]

On Tue, 21 Nov 2023 12:58:42 GMT, Jim Laskey  wrote:

>> @JimLaskey, in my experiments for JDK-8308715 (Create a mechanism for 
>> Implicitly Declared Class javadoc), I found that 
>> `javax.lang.model.util.Elements.getOrigin` reports `Origin.EXPLICIT` for the 
>> implicitly declared class and `Origin.MANDATED` for that class' constructor. 
>> Shouldn't they both be `Origin.MANDATED` because the elements are created 
>> from source, not class files, which have some limitations on `ACC_MANDATED`?
>
> @pavelrappo Classes can’t be MANDATED. Looking at the code I see that 
> EXPLICIT is the default return value when no other conditions apply.

@JimLaskey, I note that something weird is happening in relation to comments on 
method declarations. Comments on field declarations might be affected too, but 
I haven't checked.

Comments on method declarations are correctly captured only if those 
declarations are minimal. That is, if they consist of a result, name, and 
parameters. Whenever modifiers like public, protected, private, static, or 
final appear, the comments disappear.

Briefly looking at `com.sun.tools.javac.parser.JavacParser` code (which I have 
no clue what is doing), I assume the comments are getting misattributed to some 
other tokens.

-

PR Comment: https://git.openjdk.org/jdk/pull/16461#issuecomment-1821458574

Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]

[Paul Sandoz]

On Mon, 23 Oct 2023 09:02:35 GMT, Xiaohong Gong  wrote:

> This looks good. As far as I can tell the choice you've made of accuracy 
> matches what we need to meet the spec. 

Same here . Sinh/cosh/tanh/expm1 are specified to be within 2.5 ulps of the 
exact result, but i believe sleef does not offer that option, it's either 
within 1 or 3.5, so we have to pick the former. AFAICT sleef does not say 
anything about monotonicity, but we relax semi-monotonicity for all but sqrt 
(we defer to IEEE 754).

-

PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1821422985

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v6]

[Martin Doerr]

On Tue, 21 Nov 2023 17:54:08 GMT, suchismith1993  wrote:

>> src/java.base/aix/native/libsyslookup/syslookup.c line 30:
>> 
>>> 28: #include 
>>> 29: #include 
>>> 30: #include 
>> 
>> Are string.h and stdlib.h needed? I can't see them in the comments below.
>
> string.h is needed for strlen. Let me check for stdlib.h by excluding it.

ah, a comment above strlen would help.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400966451

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v6]

[Martin Doerr]

On Tue, 21 Nov 2023 17:49:43 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Comments
>  - Change comments

src/java.base/aix/native/libsyslookup/syslookup.c line 30:

> 28: #include 
> 29: #include 
> 30: #include 

Are string.h and stdlib.h needed? I can't see them in the comments below.

src/java.base/aix/native/libsyslookup/syslookup.c line 33:

> 31: #include 
> 32: 
> 33: // Addresses of functions to referenced using static linking.

What does "functions to referenced" mean? Typo?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400959365
PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400961995

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v6]

[suchismith1993]

On Tue, 21 Nov 2023 17:50:04 GMT, Martin Doerr  wrote:

>> suchismith1993 has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - Comments
>>  - Change comments
>
> src/java.base/aix/native/libsyslookup/syslookup.c line 30:
> 
>> 28: #include 
>> 29: #include 
>> 30: #include 
> 
> Are string.h and stdlib.h needed? I can't see them in the comments below.

string.h is needed for strlen. Let me check for stdlib.h by excluding it.

> src/java.base/aix/native/libsyslookup/syslookup.c line 33:
> 
>> 31: #include 
>> 32: 
>> 33: // Addresses of functions to referenced using static linking.
> 
> What does "functions to referenced" mean? Typo?

To be Referenced*. Is that the right comment ?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400964039
PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400964522

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v31]

[Jim Laskey]

> Address changes from JEP 445 to JEP 463.
> 
> - Move from a SYNTHETIC unnamed class to a MANDATED implicit class.
> 
> - Don't mark class on read.
> 
> - Remove reflection and annotation processing related to unnamed classes.
> 
> - Simplify main method search.

Jim Laskey has updated the pull request incrementally with one additional 
commit since the last revision:

  Update Class.java

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16461/files
  - new: https://git.openjdk.org/jdk/pull/16461/files/b14c6a54..fd0c92f8

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16461&range=30
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16461&range=29-30

  Stats: 34 lines in 1 file changed: 9 ins; 17 del; 8 mod
  Patch: https://git.openjdk.org/jdk/pull/16461.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16461/head:pull/16461

PR: https://git.openjdk.org/jdk/pull/16461

Integrated: 8318364: Add an FFM-based implementation of harfbuzz OpenType layout

[Phil Race]

On Tue, 29 Aug 2023 22:04:40 GMT, Phil Race  wrote:

> 8318364: Add an FFM-based implementation of harfbuzz OpenType layout

This pull request has now been integrated.

Changeset: f69e6653
Author:Phil Race 
URL:   
https://git.openjdk.org/jdk/commit/f69e6653f86a7dd781db6c8523f114c0d3f7ccbc
Stats: 1376 lines in 7 files changed: 1373 ins; 0 del; 3 mod

8318364: Add an FFM-based implementation of harfbuzz OpenType layout

Reviewed-by: jdv, psadhukhan

-

PR: https://git.openjdk.org/jdk/pull/15476

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v6]

[suchismith1993]

> The math library in AIX specifically, is a static archive. Doing a -lm wont 
> suffice, because when the symbols are looked up using dlsym or accessing 
> native code through Java, it will lead to failures.
> Hence we had to come up with a list of symbols to allow math library symbols 
> to be accesible.
> Also, there are parts of libc library that are static too, and hence those 
> symbols also are present in this list.
> Without this change, the StdLibTest and multiple other tests which make 
> native function calls using FFI, fail with NoSuchElementException.
> 
> 
> 1. Adding required compiler flags.
> 2. Adding required symbols.
> 
> 
> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)

suchismith1993 has updated the pull request incrementally with two additional 
commits since the last revision:

 - Comments
 - Change comments

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16414/files
  - new: https://git.openjdk.org/jdk/pull/16414/files/e0b22d47..48a69c59

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=04-05

  Stats: 5 lines in 1 file changed: 0 ins; 4 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/16414.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16414/head:pull/16414

PR: https://git.openjdk.org/jdk/pull/16414

Re: RFR: 8319571: Update jni/nullCaller/NullCallerTest.java to accept flags or mark as flagless [v2]

[Tim Prinzing]

> Marked as flagless.

Tim Prinzing 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 four additional commits since 
the last revision:

 - use ProcessTools helper class to run test.
 - Merge branch 'master' into JDK-8319571
 - Merge branch 'master' into JDK-8319571
 - 8319571: Update jni/nullCaller/NullCallerTest.java to accept flags or mark 
as flagless
   
   Marked as flagless.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16711/files
  - new: https://git.openjdk.org/jdk/pull/16711/files/bbcbb810..b947469b

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16711&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16711&range=00-01

  Stats: 12689 lines in 337 files changed: 6298 ins; 3150 del; 3241 mod
  Patch: https://git.openjdk.org/jdk/pull/16711.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16711/head:pull/16711

PR: https://git.openjdk.org/jdk/pull/16711

Re: RFR: 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview) [v33]

[Aggelos Biboudis]

> This is the proposed patch for Primitive types in patterns, instanceof, and 
> switch (Preview).
> 
> Draft spec here: https://cr.openjdk.org/~abimpoudis/instanceof/latest/

Aggelos Biboudis has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 39 commits:

 - Merge branch 'master' into primitive-patterns
 - Merge branch 'master' into primitive-patterns
 - Merge branch 'master' into primitive-patterns
 - Merge branch 'master' into primitive-patterns
 - Small cleanup in PrimitivePatternsSwitch
 - Add test for instanceof as a pattern on with record patterns
 - Merge branch 'master' into primitive-patterns
   
   # Conflicts:
   #src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java
   #src/jdk.compiler/share/classes/com/sun/tools/javac/code/Source.java
 - Add tests for new data types in switch statements
 - Fix Bootstrap initialization error by using `explicitCastArguments` for 
integral tests
 - Fix SwitchBootstraps for pattern matching over longs
 - ... and 29 more: https://git.openjdk.org/jdk/compare/61d81d64...2b2405cc

-

Changes: https://git.openjdk.org/jdk/pull/15638/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15638&range=32
  Stats: 3199 lines in 41 files changed: 2943 ins; 111 del; 145 mod
  Patch: https://git.openjdk.org/jdk/pull/15638.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15638/head:pull/15638

PR: https://git.openjdk.org/jdk/pull/15638

Re: RFR: JDK-8319662 ForkJoinPool trims worker threads too slowly [v3]

[Doug Lea]

> This update cascades timeouts to trim subsequent workers after the first  
> keepAlive inactive period.

Doug Lea 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:

 - Merge branch 'openjdk:master' into JDK-8319662
 - tweak cascades; reinstate an @Contended; resolve JDK-8319498
 - Support cascading idle timeouts

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16725/files
  - new: https://git.openjdk.org/jdk/pull/16725/files/a3c96c6a..6d352608

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16725&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16725&range=01-02

  Stats: 6770 lines in 217 files changed: 3887 ins; 868 del; 2015 mod
  Patch: https://git.openjdk.org/jdk/pull/16725.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16725/head:pull/16725

PR: https://git.openjdk.org/jdk/pull/16725

Integrated: 8317742: ISO Standard Date Format implementation consistency on DateTimeFormatter and String.format

[Shaojin Wen]

On Tue, 3 Oct 2023 21:37:46 GMT, Shaojin Wen  wrote:

> j.u.Formatter now prints "%tF" (iso standard date) and the result is 
> incorrect when processing year < 0 or year > 

This pull request has now been integrated.

Changeset: 61d81d64
Author:Shaojin Wen 
Committer: Naoto Sato 
URL:   
https://git.openjdk.org/jdk/commit/61d81d6496a38e43a6039abc041b67626f06f5c9
Stats: 83 lines in 2 files changed: 81 ins; 0 del; 2 mod

8317742: ISO Standard Date Format implementation consistency on 
DateTimeFormatter and String.format

Reviewed-by: rriggs, naoto

-

PR: https://git.openjdk.org/jdk/pull/16033

Re: RFR: 8318776: Require supports_cx8 to always be true [v4]

[Aleksey Shipilev]

On Tue, 21 Nov 2023 06:03:38 GMT, Erik Österlund  wrote:

>> David Holmes has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove unnecessary includes of vm_version.hpp.
>>   Fix copyright years.
>
> This looks great!

> Thanks for the review @fisk ! I have to wait for a few Zero related PRs to 
> get integrated then re-merge, before I can integrate.

Zero patches were pushed, please re-merge. I checked current mainline works 
well with at least linux-arm-zero-fastdebug, and I would like to re-test it 
with this patch.

-

PR Comment: https://git.openjdk.org/jdk/pull/16625#issuecomment-1821309132

Re: RFR: 8319668: Fixup of jar filename typo in BadFactoryTest.sh

[Alan Bateman]

On Thu, 9 Nov 2023 16:49:41 GMT, Gaurav Chaudhari  wrote:

> The file test/jdk/javax/script/JDK_8196959/BadFactoryTest.sh contains a typo. 
> When running without security manager, the test references 'badfactoty.jar' 
> instead of 'badfactory.jar'. This change addresses this by correcting the jar 
> name.

Looks okay. This test is begging to be re-written in Java, maybe some day.

I assume the copyright header will be updated before this change is integrated.

-

Marked as reviewed by alanb (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16585#pullrequestreview-1742482324

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v30]

[Alan Bateman]

On Mon, 20 Nov 2023 20:45:22 GMT, Jim Laskey  wrote:

>> Address changes from JEP 445 to JEP 463.
>> 
>> - Move from a SYNTHETIC unnamed class to a MANDATED implicit class.
>> 
>> - Don't mark class on read.
>> 
>> - Remove reflection and annotation processing related to unnamed classes.
>> 
>> - Simplify main method search.
>
> Jim Laskey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update test

src/java.base/share/classes/java/lang/Class.java line 4847:

> 4845: @PreviewFeature(feature=PreviewFeature.Feature.IMPLICIT_CLASSES)
> 4846: @CallerSensitive
> 4847: public Method findMainMethod() {

Renaming it to findMainMethod separates it from the existing getXXXMethod 
methods (good) but it makes it more obvious that this is a method for 
launchers. I'm not 100% sure that java.lang.Class is the right place for this, 
it feels out of place.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16461#discussion_r1400785014

Re: RFR: 8311906: Improve robustness of String constructors with mutable array inputs [v11]

[Roger Riggs]

> Strings, after construction, are immutable but may be constructed from 
> mutable arrays of bytes, characters, or integers.
> The string constructors should guard against the effects of mutating the 
> arrays during construction that might invalidate internal invariants for the 
> correct behavior of operations on the resulting strings. In particular, a 
> number of operations have optimizations for operations on pairs of latin1 
> strings and pairs of non-latin1 strings, while operations between latin1 and 
> non-latin1 strings use a more general implementation. 
> 
> The changes include:
> 
> - Adding a warning to each constructor with an array as an argument to 
> indicate that the results are indeterminate 
>   if the input array is modified before the constructor returns. 
>   The resulting string may contain any combination of characters sampled from 
> the input array.
> 
> - Ensure that strings that are represented as non-latin1 contain at least one 
> non-latin1 character.
>   For latin1 inputs, whether the arrays contain ASCII, ISO-8859-1, UTF8, or 
> another encoding decoded to latin1 the scanning and compression is unchanged.
>   If a non-latin1 character is found, the string is represented as non-latin1 
> with the added verification that a non-latin1 character is present at the 
> same index.
>   If that character is found to be latin1, then the input array has been 
> modified and the result of the scan may be incorrect.
>   Though a ConcurrentModificationException could be thrown, the risk to an 
> existing application of an unexpected exception should be avoided.
>   Instead, the non-latin1 copy of the input is re-scanned and compressed; 
> that scan determines whether the latin1 or the non-latin1 representation is 
> returned.
> 
> - The methods that scan for non-latin1 characters and their intrinsic 
> implementations are updated to return the index of the non-latin1 character.
> 
> - String construction from StringBuilder and CharSequence must also be 
> guarded as their contents may be modified during construction.

Roger Riggs has updated the pull request incrementally with two additional 
commits since the last revision:

 - Merge pull request #4 from cl4es/8311906_x64_intr_opt
   
   Simplified and slightly optimized x86 char_array_compress
 - Simplified and slightly optimized x86 char_array_compress

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16425/files
  - new: https://git.openjdk.org/jdk/pull/16425/files/04d58779..0256b9e0

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16425&range=10
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16425&range=09-10

  Stats: 61 lines in 1 file changed: 7 ins; 20 del; 34 mod
  Patch: https://git.openjdk.org/jdk/pull/16425.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16425/head:pull/16425

PR: https://git.openjdk.org/jdk/pull/16425

Re: RFR: 8319577: x86_64 AVX2 intrinsics for Arrays.sort methods (int, float arrays) [v2]

[Dalibor Topic]

On Thu, 16 Nov 2023 07:09:31 GMT, David Holmes  wrote:

>> Srinivas Vamsi Parasa 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 11 
>> additional commits since the last revision:
>> 
>>  - Disable AVX2 sort for 64-bit types
>>  - Merge branch 'master' of https://git.openjdk.java.net/jdk into simdsort
>>  - fix jcheck failures due to windows encoding
>>  - fix carriage return and change insertion sort thresholds
>>  - fix formatting and white spaces
>>  - cleanup unused code
>>  - fix insertion sort thresholds
>>  - add insertion sort
>>  - fix headers
>>  - revert to xss-common-qsort
>>  - ... and 1 more: https://git.openjdk.org/jdk/compare/d6a8ebf0...08307b6a
>
> src/java.base/linux/native/libsimdsort/avx2-32bit-qsort.hpp line 3:
> 
>> 1: /*
>> 2:  * Copyright (c) 2021, 2023, Intel Corporation. All rights reserved.
>> 3:  * Copyright (c) 2021 Serge Sans Paille. All rights reserved.
> 
> Is this person an OpenJDK Contributor?

Not listed here: https://oca.opensource.oracle.com/?ojr=contributors

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16534#discussion_r1400753295

Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v3]

[Andrew Haley]

On Tue, 21 Nov 2023 13:24:34 GMT, Eric Liu  wrote:

>> Vector API defines zero-extend operations [1], which are going to be 
>> intrinsified and generated to `VectorUCastNode` by C2. This patch adds 
>> backend implementation for `VectorUCastNode` on AArch64.
>> 
>> The micro benchmark shows significant performance improvement. In my test 
>> machine (SVE, 256-bit), the result is shown as below:
>> 
>> 
>> 
>>   Benchmark Before After   Units   Gain
>>   VectorZeroExtend.byte2Int 3168.251   243012.399  ops/ms  75.70
>>   VectorZeroExtend.byte2Long3212.201   216291.588  ops/ms  66.33
>>   VectorZeroExtend.byte2Short   3391.968   182655.365  ops/ms  52.85
>>   VectorZeroExtend.int2Long 1012.19780448.553  ops/ms  78.48
>>   VectorZeroExtend.short2Int1812.471   153416.828  ops/ms  83.65
>>   VectorZeroExtend.short2Long   1788.382   129794.814  ops/ms  71.58
>> 
>> 
>> On other Neon systems, we can get similar performance boost as a result of 
>> intrinsification success.
>> 
>> Since `VectorUCastNode` only used in Vector API's zero extension currently, 
>> this patch also adds assertion on nodes' definitions to clarify their usages.
>> 
>> [TEST]
>> compiler/vectorapi and jdk/incubator/vector passed on NEON and SVE machines.
>> 
>> [1] 
>> https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorOperators.java#L726
>
> Eric Liu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add _sve_xunpk & remove dead code
>   
>   Change-Id: Ic19836feb8a73ea7e65443794f2a0eb1363f6e2f

src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 1412:

> 1410:   _sve_xunpk(is_unsigned, /* is_high */ false, dst, S, dst);
> 1411:   _sve_xunpk(is_unsigned, /* is_high */ false, dst, D, dst);
> 1412:   break;

Why is this change here? It doesn't do anything. Does it?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16670#discussion_r1400743499

Re: RFR: JDK-8319662 ForkJoinPool trims worker threads too slowly [v2]

[Doug Lea]

On Mon, 20 Nov 2023 14:28:15 GMT, Viktor Klang  wrote:

>> It would be better, but absolute-time Unsafe.park only operates at 
>> millisecond accuracy (and even at that may misfire early, requiring 
>> TIMEOUT_SLOP.)
>
> I was thinking more about cases where system clock moves backwards in time. 
> (currentTimeMillis() isn't spec:ed to be monotonic, right?)

Yes. But because this is a performance not correctness issue, it's OK to 
(rarely) speed up or slow down trims.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16725#discussion_r1400732882

Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v3]

[Andrew Haley]

On Tue, 21 Nov 2023 13:24:34 GMT, Eric Liu  wrote:

>> Vector API defines zero-extend operations [1], which are going to be 
>> intrinsified and generated to `VectorUCastNode` by C2. This patch adds 
>> backend implementation for `VectorUCastNode` on AArch64.
>> 
>> The micro benchmark shows significant performance improvement. In my test 
>> machine (SVE, 256-bit), the result is shown as below:
>> 
>> 
>> 
>>   Benchmark Before After   Units   Gain
>>   VectorZeroExtend.byte2Int 3168.251   243012.399  ops/ms  75.70
>>   VectorZeroExtend.byte2Long3212.201   216291.588  ops/ms  66.33
>>   VectorZeroExtend.byte2Short   3391.968   182655.365  ops/ms  52.85
>>   VectorZeroExtend.int2Long 1012.19780448.553  ops/ms  78.48
>>   VectorZeroExtend.short2Int1812.471   153416.828  ops/ms  83.65
>>   VectorZeroExtend.short2Long   1788.382   129794.814  ops/ms  71.58
>> 
>> 
>> On other Neon systems, we can get similar performance boost as a result of 
>> intrinsification success.
>> 
>> Since `VectorUCastNode` only used in Vector API's zero extension currently, 
>> this patch also adds assertion on nodes' definitions to clarify their usages.
>> 
>> [TEST]
>> compiler/vectorapi and jdk/incubator/vector passed on NEON and SVE machines.
>> 
>> [1] 
>> https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorOperators.java#L726
>
> Eric Liu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   add _sve_xunpk & remove dead code
>   
>   Change-Id: Ic19836feb8a73ea7e65443794f2a0eb1363f6e2f

src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 3872:

> 3870:   void sve_sunpklo(FloatRegister Zd, SIMD_RegVariant T, FloatRegister 
> Zn) {
> 3871: _sve_xunpk(/* is_unsigned */ false, /* is_high */ false, Zd, T, Zn);
> 3872:   }

This code expansion does not look right. You should be able to make this change 
without so much code expansion.


#define INSN(NAME, unsigned, high)
void name(FloatRegister Zd, SIMD_RegVariant T, FloatRegister Zn) { \
  _sve_xunpk(unsigned, high, T, Zn);
\
}
  INSN(sve_uunpkhi, true, true) ...

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16670#discussion_r1400716627

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v19]

[Joe Darcy]

On Tue, 21 Nov 2023 10:38:36 GMT, Pavel Rappo  wrote:

>> Look at the spec https://bugs.openjdk.org/browse/JDK-8319252 under 7.3 
>> Compilation Units.
>> 
>> - It is not abstract (8.1.1.1 ⇗).
>> - It is final (8.1.1.2 ⇗).
>> - It is a member of an unnamed package (7.4.2 ⇗) and **has package access**. 
>> Its direct superclass is Object (8.1.4 ⇗).
>> - It does not have any direct superinterfaces (8.1.5 ⇗).
>> - The body of the class contains every ClassMemberDeclaration (these are 
>> declarations of fields (8.3 ⇗), methods (8.4 ⇗), member classes (8.5 ⇗), and 
>> member interfaces (9.1.1.3 ⇗)) from the simple compilation unit. It is not 
>> possible for a simple compilation unit to declare an instance initializer 
>> (8.6 ⇗), static initializer (8.7 ⇗), or constructor (8.8 ⇗).
>> -It has an implicitly declared default constructor (8.8.9 ⇗).
>
> @JimLaskey, in my experiments for JDK-8308715 (Create a mechanism for 
> Implicitly Declared Class javadoc), I found that 
> `javax.lang.model.util.Elements.getOrigin` reports `Origin.EXPLICIT` for the 
> implicitly declared class and `Origin.MANDATED` for that class' constructor. 
> Shouldn't they both be `Origin.MANDATED` because the elements are created 
> from source, not class files, which have some limitations on `ACC_MANDATED`?

> @pavelrappo Classes can’t be MANDATED. Looking at the code I see that 
> EXPLICIT is the default return value when no other conditions apply.

While it is true that the class file format cannot capture MANDATED status for 
classes, that doesn't necessarily imply mandated-ness shouldn't be track for 
classes created from sources while inside javac.

-

PR Comment: https://git.openjdk.org/jdk/pull/16461#issuecomment-1821055805

Re: RFR: JDK-8319662 ForkJoinPool trims worker threads too slowly [v2]

[Doug Lea]

> This update cascades timeouts to trim subsequent workers after the first  
> keepAlive inactive period.

Doug Lea has updated the pull request incrementally with one additional commit 
since the last revision:

  tweak cascades; reinstate an @Contended; resolve JDK-8319498

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16725/files
  - new: https://git.openjdk.org/jdk/pull/16725/files/bd5df5ea..a3c96c6a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16725&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16725&range=00-01

  Stats: 56 lines in 1 file changed: 20 ins; 5 del; 31 mod
  Patch: https://git.openjdk.org/jdk/pull/16725.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16725/head:pull/16725

PR: https://git.openjdk.org/jdk/pull/16725

Re: RFR: 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF [v3]

[Magnus Ihse Bursie]

On Mon, 20 Nov 2023 01:47:34 GMT, Xiaohong Gong  wrote:

>> make/autoconf/lib-vmath.m4 line 92:
>> 
>>> 90: []
>>> 91: )
>>> 92: AC_MSG_RESULT([${SVE_FEATURE_SUPPORT}])
>> 
>> What is this test even for? I can't see any usage of SVE_FEATURE_SUPPORT 
>> outside this function.
>
> This is just used to print the result of `AC_MSG_CEHCKING[if ARM SVE feature 
> is supported]` in configure.

Ah, now I se what you are trying to do here. First of all, in the detection 
part, only set `SVE_FEATURE_SUPPORT`. Then you can handle the `SVE_CFLAGS` 
addition elsewhere/later. 

Secondly, you should not mix these `SVE_CFLAGS` with the spleef C flags. 
Keeping them separate will allow for LIBSLEEF_CFLAGS to be named just that.

Thirdly, I do not like at all how you just come crashing in setting `-march` 
like that. The `-march` flag is handled by `FLAGS_SETUP_ABI_PROFILE`. 

Actually, now that I think of it, this is just completely wrong! You are 
checking on features on the build machine, to determine what target machine 
code to generate, with no way to override. 

You need to break out the -march handling separately. It should be moved to 
FLAGS_SETUP_ABI_PROFILE. I'm guessing you will need to make something like a 
`aarch64-sve` profile, and possibly try to auto-select it based on the result 
of the sve test program above. But changing `OPENJDK_TARGET_ABI_PROFILE` can 
have further consequences; I do not know the full extent on the top of my head.

>> make/autoconf/libraries.m4 line 129:
>> 
>>> 127:   LIB_SETUP_LIBFFI
>>> 128:   LIB_SETUP_MISC_LIBS
>>> 129:   LIB_SETUP_VMATH
>> 
>> The function (and file) should be named after "sleef", not "vmath".
>
> Yes, it seems weird. But the library we want to built out is `libvmath.so` 
> instead of `libsleef.so`. And we not only check the sleef library, but also 
> the ARM SVE feature inside it. So using `VMATH` suffix is more reasonable to 
> me. WDYT?

As I said above, you should not mix the two together. Keep the library handling 
for libsleef. Move the march setting to where it belongs. And rename the files, 
functions and variables after this.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1400663476
PR Review Comment: https://git.openjdk.org/jdk/pull/16234#discussion_r1400665010

RFR: 8320360: ClassFile.parse: Some defect class files cause unexpected exceptions to be thrown

[Adam Sotona]

ClassFile API throws `IndexOutOfBoundsException` when classfile structure is 
corrupted so the parser attempts to read beyond the classfile bounds.
General contract is that only `IllegalArgumentException` or its subclasses is 
expected when parser fails.
This patch wraps `IndexOutOfBoundsExceptions` thrown from all 
`ClassReaderImpl.buffer` manipulations into an  
`IllegalArgumentException("Reading beyond classfile bounds", iOOBECause)`.
Relevant tests are added.

Please review.

Thanks,
Adam

-

Commit messages:
 - added bug # to the test
 - 8320360: ClassFile.parse: Some defect class files cause unexpected 
exceptions to be thrown

Changes: https://git.openjdk.org/jdk/pull/16762/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16762&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8320360
  Stats: 69 lines in 2 files changed: 47 ins; 0 del; 22 mod
  Patch: https://git.openjdk.org/jdk/pull/16762.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16762/head:pull/16762

PR: https://git.openjdk.org/jdk/pull/16762

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v5]

[Martin Doerr]

On Tue, 21 Nov 2023 13:01:50 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Update Copyright
>  - Revert lookup file in share directory.
>Add lookup file for AIX specific implementation.

src/java.base/aix/native/libsyslookup/syslookup.c line 209:

> 207: char* syslookup() {
> 208:   return "syslookup";
> 209: }

This is probably not needed, either (see Windows version).

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400629119

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v5]

[Magnus Ihse Bursie]

On Tue, 21 Nov 2023 13:01:50 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Update Copyright
>  - Revert lookup file in share directory.
>Add lookup file for AIX specific implementation.

This looks like a much saner solution than the original attempt.

src/java.base/aix/native/libsyslookup/syslookup.c line 33:

> 31: #include 
> 32: 
> 33: // Simple dummy function so this library appears as a normal library to 
> tooling.

I assume this comment should be on line 207 instead..?

-

PR Comment: https://git.openjdk.org/jdk/pull/16414#issuecomment-1820957256
PR Review Comment: https://git.openjdk.org/jdk/pull/16414#discussion_r1400626367

Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v2]

[Eric Liu]

On Mon, 20 Nov 2023 09:26:43 GMT, Eric Liu  wrote:

>> src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 1415:
>> 
>>> 1413:   break;
>>> 1414: case S:
>>> 1415:   (this->*unpklo)(dst, H, src);
>> 
>> AS above: try making` is_unsigned` a parameter.
>
> Got it. I will fix it soon. Thanks!

compiler/vectorapi and jdk/incubator/vector passed. Full test is running. I 
would report the result when it has been finished.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16670#discussion_r1400602423

Re: RFR: 8319872: AArch64: [vectorapi] Implementation of unsigned (zero extended) casts [v3]

[Eric Liu]

> Vector API defines zero-extend operations [1], which are going to be 
> intrinsified and generated to `VectorUCastNode` by C2. This patch adds 
> backend implementation for `VectorUCastNode` on AArch64.
> 
> The micro benchmark shows significant performance improvement. In my test 
> machine (SVE, 256-bit), the result is shown as below:
> 
> 
> 
>   Benchmark Before After   Units   Gain
>   VectorZeroExtend.byte2Int 3168.251   243012.399  ops/ms  75.70
>   VectorZeroExtend.byte2Long3212.201   216291.588  ops/ms  66.33
>   VectorZeroExtend.byte2Short   3391.968   182655.365  ops/ms  52.85
>   VectorZeroExtend.int2Long 1012.19780448.553  ops/ms  78.48
>   VectorZeroExtend.short2Int1812.471   153416.828  ops/ms  83.65
>   VectorZeroExtend.short2Long   1788.382   129794.814  ops/ms  71.58
> 
> 
> On other Neon systems, we can get similar performance boost as a result of 
> intrinsification success.
> 
> Since `VectorUCastNode` only used in Vector API's zero extension currently, 
> this patch also adds assertion on nodes' definitions to clarify their usages.
> 
> [TEST]
> compiler/vectorapi and jdk/incubator/vector passed on NEON and SVE machines.
> 
> [1] 
> https://github.com/openjdk/jdk/blob/master/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorOperators.java#L726

Eric Liu has updated the pull request incrementally with one additional commit 
since the last revision:

  add _sve_xunpk & remove dead code
  
  Change-Id: Ic19836feb8a73ea7e65443794f2a0eb1363f6e2f

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16670/files
  - new: https://git.openjdk.org/jdk/pull/16670/files/4fb3128f..68748e7f

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16670&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16670&range=01-02

  Stats: 85 lines in 2 files changed: 17 ins; 32 del; 36 mod
  Patch: https://git.openjdk.org/jdk/pull/16670.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16670/head:pull/16670

PR: https://git.openjdk.org/jdk/pull/16670

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v5]

[Martin Doerr]

On Tue, 21 Nov 2023 13:01:50 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Update Copyright
>  - Revert lookup file in share directory.
>Add lookup file for AIX specific implementation.

Thanks! This looks good, now.
One question: Are all the `#include`s needed?

-

Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1741972916

Re: RFR: 8318971:Better Error Handling for Jar Tool Processing of "@File" [v4]

[Ryan Wallace]

> Hi all,
> 
> Please review this fix for jar tool not producing an archive if there is a 
> missing file supplied.
> The current behaviour will recognise missing files as an error but continue 
> processing,
> creating a temporary archive and then deleting it without moving to the 
> current directory.
> The fix is to return false when a missing file is supplied and exit 
> immediately without continuing with any wasted processing.
> 
> Thanks,
> Ryan.

Ryan Wallace 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 eight additional commits since 
the last revision:

 - 8318971: Better Error Handling for Jar Tool Processing of "@File"
 - Merge branch 'master' into 8318971
 - Merge branch 'master' into 8318971
 - 8318971: jar v17 should either exit on error immediately or create archive 
as jar v1.8 did
 - 8318971: jar v17 should either exit on error immediately or create archive 
as jar v1.8 did
 - Merge branch 'master' into 8318971
 - 8318971: jar v17 should either exit on error immediately or create archive 
as jar v1.8 did
 - 8318971: jar v17 should either exit on error immediately or create archive 
as jar v1.8 did

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16423/files
  - new: https://git.openjdk.org/jdk/pull/16423/files/c7b18741..cee20934

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16423&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16423&range=02-03

  Stats: 632825 lines in 1230 files changed: 90078 ins; 478477 del; 64270 mod
  Patch: https://git.openjdk.org/jdk/pull/16423.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16423/head:pull/16423

PR: https://git.openjdk.org/jdk/pull/16423

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v5]

[suchismith1993]

> The math library in AIX specifically, is a static archive. Doing a -lm wont 
> suffice, because when the symbols are looked up using dlsym or accessing 
> native code through Java, it will lead to failures.
> Hence we had to come up with a list of symbols to allow math library symbols 
> to be accesible.
> Also, there are parts of libc library that are static too, and hence those 
> symbols also are present in this list.
> Without this change, the StdLibTest and multiple other tests which make 
> native function calls using FFI, fail with NoSuchElementException.
> 
> 
> 1. Adding required compiler flags.
> 2. Adding required symbols.
> 
> 
> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)

suchismith1993 has updated the pull request incrementally with two additional 
commits since the last revision:

 - Update Copyright
 - Revert lookup file in share directory.
   Add lookup file for AIX specific implementation.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16414/files
  - new: https://git.openjdk.org/jdk/pull/16414/files/47e5fb5a..e0b22d47

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=04
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=03-04

  Stats: 177 lines in 2 files changed: 0 ins; 176 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/16414.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16414/head:pull/16414

PR: https://git.openjdk.org/jdk/pull/16414

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v19]

[Jim Laskey]

On Tue, 21 Nov 2023 10:38:36 GMT, Pavel Rappo  wrote:

>> Look at the spec https://bugs.openjdk.org/browse/JDK-8319252 under 7.3 
>> Compilation Units.
>> 
>> - It is not abstract (8.1.1.1 ⇗).
>> - It is final (8.1.1.2 ⇗).
>> - It is a member of an unnamed package (7.4.2 ⇗) and **has package access**. 
>> Its direct superclass is Object (8.1.4 ⇗).
>> - It does not have any direct superinterfaces (8.1.5 ⇗).
>> - The body of the class contains every ClassMemberDeclaration (these are 
>> declarations of fields (8.3 ⇗), methods (8.4 ⇗), member classes (8.5 ⇗), and 
>> member interfaces (9.1.1.3 ⇗)) from the simple compilation unit. It is not 
>> possible for a simple compilation unit to declare an instance initializer 
>> (8.6 ⇗), static initializer (8.7 ⇗), or constructor (8.8 ⇗).
>> -It has an implicitly declared default constructor (8.8.9 ⇗).
>
> @JimLaskey, in my experiments for JDK-8308715 (Create a mechanism for 
> Implicitly Declared Class javadoc), I found that 
> `javax.lang.model.util.Elements.getOrigin` reports `Origin.EXPLICIT` for the 
> implicitly declared class and `Origin.MANDATED` for that class' constructor. 
> Shouldn't they both be `Origin.MANDATED` because the elements are created 
> from source, not class files, which have some limitations on `ACC_MANDATED`?

@pavelrappo Classes can’t be MANDATED. Looking at the code I see that EXPLICIT 
is the default return value when no other conditions apply.

-

PR Comment: https://git.openjdk.org/jdk/pull/16461#issuecomment-1820879428

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v4]

[suchismith1993]

> The math library in AIX specifically, is a static archive. Doing a -lm wont 
> suffice, because when the symbols are looked up using dlsym or accessing 
> native code through Java, it will lead to failures.
> Hence we had to come up with a list of symbols to allow math library symbols 
> to be accesible.
> Also, there are parts of libc library that are static too, and hence those 
> symbols also are present in this list.
> Without this change, the StdLibTest and multiple other tests which make 
> native function calls using FFI, fail with NoSuchElementException.
> 
> 
> 1. Adding required compiler flags.
> 2. Adding required symbols.
> 
> 
> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)

suchismith1993 has updated the pull request incrementally with two additional 
commits since the last revision:

 - Remove extra symbols.
 - Remove additional lines

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16414/files
  - new: https://git.openjdk.org/jdk/pull/16414/files/4899c975..47e5fb5a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=02-03

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/16414.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16414/head:pull/16414

PR: https://git.openjdk.org/jdk/pull/16414

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v3]

[Martin Doerr]

On Tue, 21 Nov 2023 11:52:23 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains four commits:
> 
>  - Remove symbols file after using inline way.
>  - Provide support for math library in inline way.
>  - Update symbols-aix-foreign
>
>Remove comments from export list, causes build failures.
>  - Symbol Resolution fix for Panama changes.
>1.Adding required compiler flags.
>2. Adding required symbols.

Nice! This solution looks better. But, please don't modify the shared version 
of syslookup.c. You can create one for AIX here: 
src/java.base/aix/native/libsyslookup

-

Changes requested by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1741796785

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v3]

[suchismith1993]

> The math library in AIX specifically, is a static archive. Doing a -lm wont 
> suffice, because when the symbols are looked up using dlsym or accessing 
> native code through Java, it will lead to failures.
> Hence we had to come up with a list of symbols to allow math library symbols 
> to be accesible.
> Also, there are parts of libc library that are static too, and hence those 
> symbols also are present in this list.
> Without this change, the StdLibTest and multiple other tests which make 
> native function calls using FFI, fail with NoSuchElementException.
> 
> 
> 1. Adding required compiler flags.
> 2. Adding required symbols.
> 
> 
> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)

suchismith1993 has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains four commits:

 - Remove symbols file after using inline way.
 - Provide support for math library in inline way.
 - Update symbols-aix-foreign
   
   Remove comments from export list, causes build failures.
 - Symbol Resolution fix for Panama changes.
   1.Adding required compiler flags.
   2. Adding required symbols.

-

Changes: https://git.openjdk.org/jdk/pull/16414/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=02
  Stats: 179 lines in 2 files changed: 177 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/16414.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16414/head:pull/16414

PR: https://git.openjdk.org/jdk/pull/16414

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v2]

[Martin Doerr]

On Tue, 21 Nov 2023 11:21:40 GMT, suchismith1993  wrote:

>> The math library in AIX specifically, is a static archive. Doing a -lm wont 
>> suffice, because when the symbols are looked up using dlsym or accessing 
>> native code through Java, it will lead to failures.
>> Hence we had to come up with a list of symbols to allow math library symbols 
>> to be accesible.
>> Also, there are parts of libc library that are static too, and hence those 
>> symbols also are present in this list.
>> Without this change, the StdLibTest and multiple other tests which make 
>> native function calls using FFI, fail with NoSuchElementException.
>> 
>> 
>> 1. Adding required compiler flags.
>> 2. Adding required symbols.
>> 
>> 
>> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)
>
> suchismith1993 has updated the pull request incrementally with 172 additional 
> commits since the last revision:
> 
>  - Provide support for math library in inline way.
>  - Symbol Resolution fix for Panama changes.
>1. Adding required compiler flags.
>2. Adding required symbols.
>  - 8320348: test/jdk/java/io/File/GetAbsolutePath.windowsDriveRelative fails 
> if working directory is not on drive C
>
>Reviewed-by: alanb, mbaesken
>  - 8320372: test/jdk/sun/security/x509/DNSName/LeadingPeriod.java validity 
> check failed
>
>Reviewed-by: alanb
>  - 8320234: Merge doclint.Env.AccessKind with tool.AccessKind
>
>Reviewed-by: jjg
>  - 8319817: Charset constructor should make defensive copy of aliases
>
>Reviewed-by: rriggs, alanb, bpb, iris, jpai
>  - 8320147: Remove DumpSharedSpaces
>
>Reviewed-by: ccheung, matsaave
>  - 8319973: AArch64: Save and restore FPCR in the call stub
>
>Reviewed-by: adinn, stuefe
>  - 8320410: Reflow markdown in building.md
>
>Reviewed-by: erikj
>  - 8319928: Exceptions thrown by cleanup actions should be handled correctly
>
>Reviewed-by: jvernee
>  - ... and 162 more: https://git.openjdk.org/jdk/compare/c340abf0...9c77fa98

It's messed up, now. Can you revert the last step?

-

Changes requested by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16414#pullrequestreview-1741741189

Re: RFR: JDK-8317799 : AIX PPC64: FFI symbol lookup doesn't find symbols [v2]

[suchismith1993]

> The math library in AIX specifically, is a static archive. Doing a -lm wont 
> suffice, because when the symbols are looked up using dlsym or accessing 
> native code through Java, it will lead to failures.
> Hence we had to come up with a list of symbols to allow math library symbols 
> to be accesible.
> Also, there are parts of libc library that are static too, and hence those 
> symbols also are present in this list.
> Without this change, the StdLibTest and multiple other tests which make 
> native function calls using FFI, fail with NoSuchElementException.
> 
> 
> 1. Adding required compiler flags.
> 2. Adding required symbols.
> 
> 
> JBS-ISSUE : [JDK-8317799](https://bugs.openjdk.org/browse/JDK-8317799)

suchismith1993 has updated the pull request incrementally with 172 additional 
commits since the last revision:

 - Provide support for math library in inline way.
 - Symbol Resolution fix for Panama changes.
   1. Adding required compiler flags.
   2. Adding required symbols.
 - 8320348: test/jdk/java/io/File/GetAbsolutePath.windowsDriveRelative fails if 
working directory is not on drive C
   
   Reviewed-by: alanb, mbaesken
 - 8320372: test/jdk/sun/security/x509/DNSName/LeadingPeriod.java validity 
check failed
   
   Reviewed-by: alanb
 - 8320234: Merge doclint.Env.AccessKind with tool.AccessKind
   
   Reviewed-by: jjg
 - 8319817: Charset constructor should make defensive copy of aliases
   
   Reviewed-by: rriggs, alanb, bpb, iris, jpai
 - 8320147: Remove DumpSharedSpaces
   
   Reviewed-by: ccheung, matsaave
 - 8319973: AArch64: Save and restore FPCR in the call stub
   
   Reviewed-by: adinn, stuefe
 - 8320410: Reflow markdown in building.md
   
   Reviewed-by: erikj
 - 8319928: Exceptions thrown by cleanup actions should be handled correctly
   
   Reviewed-by: jvernee
 - ... and 162 more: https://git.openjdk.org/jdk/compare/c340abf0...9c77fa98

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16414/files
  - new: https://git.openjdk.org/jdk/pull/16414/files/c340abf0..9c77fa98

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16414&range=00-01

  Stats: 629149 lines in 1218 files changed: 87938 ins; 478201 del; 63010 mod
  Patch: https://git.openjdk.org/jdk/pull/16414.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16414/head:pull/16414

PR: https://git.openjdk.org/jdk/pull/16414

Re: RFR: 8318776: Require supports_cx8 to always be true [v4]

[David Holmes]

On Tue, 21 Nov 2023 06:03:38 GMT, Erik Österlund  wrote:

>> David Holmes has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove unnecessary includes of vm_version.hpp.
>>   Fix copyright years.
>
> This looks great!

Thanks for the review @fisk ! I have to wait for a few Zero related PRs to get 
integrated then re-merge, before I can integrate.

-

PR Comment: https://git.openjdk.org/jdk/pull/16625#issuecomment-1820689855

Re: RFR: JDK-8315458 Implement JEP 463: Implicitly Declared Classes and Instance Main Method (Second Preview) [v19]

[Pavel Rappo]

On Wed, 15 Nov 2023 15:54:26 GMT, Jim Laskey  wrote:

>> Jim Laskey has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Filter abstract main methods and search interfaces for default main 
>> methods.
>
> Look at the spec https://bugs.openjdk.org/browse/JDK-8319252 under 7.3 
> Compilation Units.
> 
> - It is not abstract (8.1.1.1 ⇗).
> - It is final (8.1.1.2 ⇗).
> - It is a member of an unnamed package (7.4.2 ⇗) and **has package access**. 
> Its direct superclass is Object (8.1.4 ⇗).
> - It does not have any direct superinterfaces (8.1.5 ⇗).
> - The body of the class contains every ClassMemberDeclaration (these are 
> declarations of fields (8.3 ⇗), methods (8.4 ⇗), member classes (8.5 ⇗), and 
> member interfaces (9.1.1.3 ⇗)) from the simple compilation unit. It is not 
> possible for a simple compilation unit to declare an instance initializer 
> (8.6 ⇗), static initializer (8.7 ⇗), or constructor (8.8 ⇗).
> -It has an implicitly declared default constructor (8.8.9 ⇗).

@JimLaskey, in my experiments for JDK-8308715 (Create a mechanism for 
Implicitly Declared Class javadoc), I found that 
`javax.lang.model.util.Elements.getOrigin` reports `Origin.EXPLICIT` for the 
implicitly declared class and `Origin.MANDATED` for that class' constructor. 
Shouldn't they both be `Origin.MANDATED` because the elements are created from 
source, not class files, which have some limitations on `ACC_MANDATED`?

-

PR Comment: https://git.openjdk.org/jdk/pull/16461#issuecomment-1820659000

Re: RFR: 8308753: Class-File API transition to Preview [v30]

[Adam Sotona]

> Classfile API is an internal library under package `jdk.internal.classfile` 
> in JDK 21.
> This pull request turns the Classfile API into a preview feature and moves it 
> into `java.lang.classfile`.
> It repackages all uses across JDK and tests and adds lots of missing Javadoc.
> 
> This PR goes in sync with 
> [JDK-8308754](https://bugs.openjdk.org/browse/JDK-8308754): Class-File API 
> (Preview) (CSR)
> and [JDK-8280389](https://bugs.openjdk.org/browse/JDK-8280389): Class-File 
> API (Preview) (JEP).
> 
> Online javadoc is available at: 
> https://cr.openjdk.org/~asotona/JDK-8308753-preview/api/java.base/java/lang/classfile/package-summary.html
> 
> In addition to the primary transition to preview, this pull request also 
> includes:
> - All Classfile* classes ranamed to ClassFile* (based on JEP discussion).
> - A new preview feature, `CLASSFILE_API`, has been added.
> - Buildsystem tool required a little patch to support annotated modules.
> - All JDK modules using the Classfile API are newly participating in the 
> preview.
> - All tests that use the Classfile API now have preview enabled.
> - A few Javac tests not allowing preview have been partially reverted; their 
> conversion can be re-applied when the Classfile API leaves preview mode.
> 
> Despite the number of affected files, this pull request is relatively 
> straight-forward. The preview version of the Classfile API is based on the 
> internal version of the library from the JDK master branch, and there are no 
> API features added.
> 
> Please review this pull request to help the Classfile API turn into a preview.
> 
> Any comments are welcome.
> 
> Thanks,
> Adam

Adam Sotona has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 366 commits:

 - Merge branch 'master' into JDK-8308753-preview
   
   # Conflicts:
   #test/jdk/jdk/classfile/StackMapsTest.java
 - Merge branch 'master' into JDK-8308753-preview
   
   # Conflicts:
   #test/langtools/tools/javac/7199823/InnerClassCannotBeVerified.java
 - added new package-info snippets and fixed ClassFile::parse throws javadoc 
description
 - fixed lambda tests
 - Merge branch 'master' into JDK-8308753-preview
 - fixed SignaturesTest
 - fixed condy tests
 - Merge branch 'master' into JDK-8308753-preview
 - Merge branch 'master' into JDK-8308753-preview
   
   # Conflicts:
   #test/jdk/tools/lib/tests/JImageValidator.java
 - fixed jdk.jfr
 - ... and 356 more: https://git.openjdk.org/jdk/compare/e055fae1...48aa8ba8

-

Changes: https://git.openjdk.org/jdk/pull/15706/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15706&range=29
  Stats: 32342 lines in 784 files changed: 14659 ins; 14109 del; 3574 mod
  Patch: https://git.openjdk.org/jdk/pull/15706.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15706/head:pull/15706

PR: https://git.openjdk.org/jdk/pull/15706

Integrated: 8320222: Wrong bytecode accepted, and StackMap table generated

[Adam Sotona]

On Thu, 16 Nov 2023 10:00:44 GMT, Adam Sotona  wrote:

> Stack map generator in ClassFile API performs only minimal checks in favour 
> of performance.
> However it led to situations where it generates invalid stack maps for 
> corrupted code.
> This patch adds basic checks of stack when two frames are merged and throws 
> an exception in case of stack size or content mismatch. Generated or 
> transformed code with inconsistent stack will fail on stack maps generation.
> Relevant tests are added.
> 
> Please review.
> 
> Thanks,
> Adam

This pull request has now been integrated.

Changeset: c4aee66d
Author:Adam Sotona 
URL:   
https://git.openjdk.org/jdk/commit/c4aee66d742008848e5b5bc8ce3b2e3032a39bc3
Stats: 74 lines in 2 files changed: 46 ins; 0 del; 28 mod

8320222: Wrong bytecode accepted, and StackMap table generated

Reviewed-by: jlahoda

-

PR: https://git.openjdk.org/jdk/pull/16685

Re: RFR: 8319123: Implement JEP 461: Stream Gatherers (Preview) [v13]

[Viktor Klang]

> This Pull-Request implements [JEP-461](https://openjdk.org/jeps/461)

Viktor Klang has updated the pull request incrementally with one additional 
commit since the last revision:

  Javadoc clarifications for Gatherer type parameters,
  correcting descriptions of Integrator type parameters,
  and more consistently use "initializer function" instead
  of "supplier function".

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/16420/files
  - new: https://git.openjdk.org/jdk/pull/16420/files/48b7e329..ec841740

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=16420&range=12
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16420&range=11-12

  Stats: 13 lines in 1 file changed: 0 ins; 0 del; 13 mod
  Patch: https://git.openjdk.org/jdk/pull/16420.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16420/head:pull/16420

PR: https://git.openjdk.org/jdk/pull/16420

Integrated: 8304446: javap --system flag doesn't override system APIs

[Adam Sotona]

On Thu, 2 Nov 2023 13:49:17 GMT, Adam Sotona  wrote:

> Javap ignores --system option when searching for JDK classes.
> This patch prepends search over JDK system modules.
> 
> Please review.
> 
> Thanks,
> Adam

This pull request has now been integrated.

Changeset: 604d29a8
Author:Adam Sotona 
URL:   
https://git.openjdk.org/jdk/commit/604d29a8c911c1064ba0fab17f9192bb4e640709
Stats: 13 lines in 1 file changed: 13 ins; 0 del; 0 mod

8304446: javap --system flag doesn't override system APIs

Reviewed-by: jlahoda

-

PR: https://git.openjdk.org/jdk/pull/16476

Re: RFR: 8320222: Wrong bytecode accepted, and StackMap table generated [v2]

[Jan Lahoda]

On Mon, 20 Nov 2023 17:17:15 GMT, Adam Sotona  wrote:

>> Stack map generator in ClassFile API performs only minimal checks in favour 
>> of performance.
>> However it led to situations where it generates invalid stack maps for 
>> corrupted code.
>> This patch adds basic checks of stack when two frames are merged and throws 
>> an exception in case of stack size or content mismatch. Generated or 
>> transformed code with inconsistent stack will fail on stack maps generation.
>> Relevant tests are added.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   changed StackMapGenerator::generatorError to return an exception instead of 
> directly throw

Looks reasonable to me.

-

Marked as reviewed by jlahoda (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16685#pullrequestreview-1741564607

Re: RFR: 8304446: javap --system flag doesn't override system APIs [v3]

[Jan Lahoda]

On Fri, 10 Nov 2023 08:38:09 GMT, Adam Sotona  wrote:

>> Javap ignores --system option when searching for JDK classes.
>> This patch prepends search over JDK system modules.
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   applied suggested changes

Looks good to me.

-

Marked as reviewed by jlahoda (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16476#pullrequestreview-1741556690

Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref

[Viktor Klang]

On Mon, 13 Nov 2023 22:31:16 GMT, Brent Christian  wrote:

> Classes in the `java.lang.ref` package would benefit from an update to bring 
> the spec in line with how the VM already behaves. The changes would focus on 
> _happens-before_ edges at some key points during reference processing.
> 
> A couple key things we want to be able to say are:
> - `Reference.reachabilityFence(x)` _happens-before_ reference processing 
> occurs for 'x'.
> - `Cleaner.register()` _happens-before_ the Cleaner thread runs the 
> registered cleaning action.
> 
> This will bring Cleaner in line (or close) with the memory visibility 
> guarantees made for finalizers in [JLS 
> 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5):
> _"There is a happens-before edge from the end of a constructor of an object 
> to the start of a finalizer (§12.6) for that object."_

src/java.base/share/classes/java/lang/ref/Reference.java line 405:

> 403:  *  This method is invoked only by Java code; when the garbage 
> collector
> 404:  * clears references it does so directly, without invoking this 
> method. The
> 405:  * {@link #enqueue} method also clears references directly, without 
> invoking

The last sentence might make sense to have as an apiNote? 🤔

src/java.base/share/classes/java/lang/ref/Reference.java line 497:

> 495:  * or {@link ReferenceQueue#remove}.
> 496:  *
> 497:  * This method is invoked only by Java code; when the garbage 
> collector

Perhaps something along the lines of:

Suggestion:

 * This method is only invoked explicitly; when the garbage collector

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1400243561
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1400249718

Re: RFR: 8314480: Memory ordering spec updates in java.lang.ref

[Viktor Klang]

On Wed, 15 Nov 2023 22:33:03 GMT, Stuart Marks  wrote:

>> Classes in the `java.lang.ref` package would benefit from an update to bring 
>> the spec in line with how the VM already behaves. The changes would focus on 
>> _happens-before_ edges at some key points during reference processing.
>> 
>> A couple key things we want to be able to say are:
>> - `Reference.reachabilityFence(x)` _happens-before_ reference processing 
>> occurs for 'x'.
>> - `Cleaner.register()` _happens-before_ the Cleaner thread runs the 
>> registered cleaning action.
>> 
>> This will bring Cleaner in line (or close) with the memory visibility 
>> guarantees made for finalizers in [JLS 
>> 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5):
>> _"There is a happens-before edge from the end of a constructor of an object 
>> to the start of a finalizer (§12.6) for that object."_
>
> src/java.base/share/classes/java/lang/ref/Reference.java line 495:
> 
>> 493:  * > href="{@docRoot}/java.base/java/util/concurrent/package-summary.html#MemoryVisibility">happen-before
>> 494:  * the reference is removed from the queue by {@link 
>> ReferenceQueue#poll}
>> 495:  * or {@link ReferenceQueue#remove}.
> 
> Should there be an explicit statement about an unsuccessful call having no 
> memory consistency effects?

Perhaps something along the lines of "A failed {@code enqueue} has no defined 
memory consistency effects."?

> src/java.base/share/classes/java/lang/ref/Reference.java line 505:
> 
>> 503:  * to return this reference even though the referent is still 
>> strongly
>> 504:  * reachable. That is, before the referent has reached the expected
>> 505:  * reachability level.
> 
> The wording here is kind of awkward. We could try to wordsmith it, but there 
> is also the possibility that there are valid use cases for using enqueue(), 
> which makes this caution seem misplaced. An alternative would simply be to 
> omit this paragraph.

Might be better to leave out the last sencence?

> src/java.base/share/classes/java/lang/ref/Reference.java line 564:
> 
>> 562:  * {@code reachabilityFence(x)}
>> 563:  * > href="{@docRoot}/java.base/java/util/concurrent/package-summary.html#MemoryVisibility">happen-before
>> 564:  * the garbage collector clears any reference to {code x}.
> 
> This is a fairly low-level specification, so the relationship described here 
> is "reachabilityFence **hb** clearing". The relationships "clearing **hb** 
> enqueue" and "enqueue **hb** dequeue" are described elsewhere. Thus the 
> mention of Cleaner above seems misplaced.
> 
> However, the whole chain of relationships
> 
>> RF **hb** clearing **hb** enqueue **hb** dequeue **hb** cleaning-action
> 
> is important. Should this be described somewhere?

Perhaps in each of these places add a link to #Memory Consistency Properties

-

PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1400247244
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1400251257
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1400254380