Re: RFR: 8352565: Add native method implementation of Reference.get() [v11]

2025-06-25 Thread Kim Barrett
On Mon, 16 Jun 2025 07:09:39 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curr

Re: RFR: 8352565: Add native method implementation of Reference.get() [v11]

2025-06-17 Thread Thomas Schatzl
On Mon, 16 Jun 2025 07:09:39 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curr

Re: RFR: 8352565: Add native method implementation of Reference.get() [v11]

2025-06-16 Thread Kim Barrett
> Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently there is intrinsic support by the interpreter, C1, C2, and

Re: RFR: 8352565: Add native method implementation of Reference.get() [v10]

2025-06-06 Thread Vladimir Ivanov
On Fri, 6 Jun 2025 06:00:48 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curre

Re: RFR: 8352565: Add native method implementation of Reference.get() [v10]

2025-06-05 Thread Kim Barrett
On Wed, 4 Jun 2025 17:39:55 GMT, Chen Liang wrote: >> Kim Barrett has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - add pseudo-native entry for Reference.get0 >> - tidy CallGenerator lookup in Compile ctor > > src/hotspot/share/opto/com

Re: RFR: 8352565: Add native method implementation of Reference.get() [v10]

2025-06-05 Thread Kim Barrett
On Fri, 6 Jun 2025 05:57:16 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curre

Re: RFR: 8352565: Add native method implementation of Reference.get() [v10]

2025-06-05 Thread Kim Barrett
> Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently there is intrinsic support by the interpreter, C1, C2, and

Re: RFR: 8352565: Add native method implementation of Reference.get() [v9]

2025-06-05 Thread Vladimir Ivanov
On Thu, 5 Jun 2025 08:42:38 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curre

Re: RFR: 8352565: Add native method implementation of Reference.get() [v9]

2025-06-05 Thread Kim Barrett
> Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently there is intrinsic support by the interpreter, C1, C2, and

Re: RFR: 8352565: Add native method implementation of Reference.get() [v8]

2025-06-04 Thread Vladimir Ivanov
On Wed, 4 Jun 2025 07:12:42 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curre

Re: RFR: 8352565: Add native method implementation of Reference.get() [v8]

2025-06-04 Thread Chen Liang
On Wed, 4 Jun 2025 07:12:42 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curre

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-06-04 Thread Kim Barrett
On Fri, 30 May 2025 19:30:50 GMT, Vladimir Ivanov wrote: >> Much of the point of this change is to let us later remove the interpreter/c1 >> intrinsics for this function. I think what you are saying is that might be >> tricky if `get()` is the intrinsic. So maybe I should just go ahead now with >

Re: RFR: 8352565: Add native method implementation of Reference.get() [v8]

2025-06-04 Thread Kim Barrett
> Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently there is intrinsic support by the interpreter, C1, C2, and

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-30 Thread Vladimir Ivanov
On Fri, 30 May 2025 11:25:01 GMT, Kim Barrett wrote: > If get0() is the intrinsic, then I think that referenced snippet from the Compile ctor can go away? Yes. - PR Review Comment: https://git.openjdk.org/jdk/pull/24315#discussion_r2116517383

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-30 Thread Kim Barrett
On Thu, 29 May 2025 18:45:20 GMT, Vladimir Ivanov wrote: >> Yes, I find it much cleaner when intrinsic is non-virtual. >> >>> Or maybe that's better done as a later cleanup? >> >> Up to you. I'm fine with the current PR if you remove the comment referring >> to C2. > > FTR the special case in

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-29 Thread Vladimir Ivanov
On Thu, 29 May 2025 18:43:31 GMT, Vladimir Ivanov wrote: >> We already have this to address that issue for the specific case of >> Reference.get: >> https://github.com/openjdk/jdk/blob/4cf729cfac57c9aec692a52c1f3f95f2403e7958/src/hotspot/share/opto/compile.cpp#L786-L792 >> I think if we made the

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-29 Thread Vladimir Ivanov
On Thu, 29 May 2025 13:08:25 GMT, Kim Barrett wrote: >> The current limitation of intrinsics support in C1/C2 is that intrinsics are >> always applied in the context of some method (as part of inlining). If a >> method is at the root of the compilation, it is never intrinsified. >> >> The prob

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-29 Thread Kim Barrett
On Thu, 29 May 2025 01:14:30 GMT, Vladimir Ivanov wrote: >> The comment is about why we have native `get0` at all, rather than just >> making >> `get` also be native. >> >>> [...] JDK-8271862 was about migrating to non-virtual intrinsic method. >> >> That's not a correct summary of JDK-8271862

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-28 Thread Vladimir Ivanov
On Sat, 24 May 2025 04:57:39 GMT, Kim Barrett wrote: >> As I understand, `JDK-8271862` was about migrating to non-virtual intrinsic >> method. >> In case of `Reference::get()`, you already have a virtual public method >> marked as `@IntrinsicCandidate`. And the patch doesn't change anything >>

Re: RFR: 8352565: Add native method implementation of Reference.get() [v7]

2025-05-27 Thread Kim Barrett
> Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently there is intrinsic support by the interpreter, C1, C2, and

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-23 Thread Kim Barrett
On Fri, 23 May 2025 23:01:02 GMT, Vladimir Ivanov wrote: >> See review thread slightly above here, specifically >> https://github.com/openjdk/jdk/pull/24315/files#r2094884157. >> I've looked at the intrinsics for Object.clone() and Object.hashCode(), but >> only enough to decide I >> understand

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-23 Thread Vladimir Ivanov
On Tue, 20 May 2025 22:14:42 GMT, Kim Barrett wrote: >> src/java.base/share/classes/java/lang/ref/Reference.java line 366: >> >>> 364: >>> 365: /* Implementation of unintrinsified get(). Making get() native >>> may lead >>> 366: * C2 to sometimes prefer the native implementation over

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-20 Thread Kim Barrett
On Tue, 20 May 2025 18:43:16 GMT, Vladimir Ivanov wrote: >> Kim Barrett 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 10 additional >> commits

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-20 Thread Vladimir Ivanov
On Fri, 9 May 2025 15:59:38 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curre

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-05-18 Thread Kim Barrett
On Fri, 11 Apr 2025 10:52:03 GMT, Aleksey Shipilev wrote: >> As far as I can tell, intrinsic selection only applies when the call target >> is >> exactly the intrinsically attributed method. (Possibly after optimizations >> that lead to a call to that specific method.) And that's obviously neces

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-05-09 Thread Kim Barrett
On Fri, 11 Apr 2025 08:14:29 GMT, Kim Barrett wrote: >> test/hotspot/jtreg/gc/TestNativeReferenceGet.java line 137: >> >>> 135: } >>> 136: checkQueue(); // One last check after refproc >>> complete. >>> 137: } catch (InterruptedException e) { >> >> Rather

Re: RFR: 8352565: Add native method implementation of Reference.get() [v6]

2025-05-09 Thread Kim Barrett
> Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently there is intrinsic support by the interpreter, C1, C2, and

Re: RFR: 8352565: Add native method implementation of Reference.get() [v5]

2025-05-07 Thread Kim Barrett
> Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently there is intrinsic support by the interpreter, C1, C2, and

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-11 Thread Aleksey Shipilev
On Fri, 11 Apr 2025 09:09:05 GMT, Kim Barrett wrote: >> On a second thought, I think we should do this shift before this PR, so that >> it is cleanly backportable. This bug messes with concurrent GC invariants, >> so it would be nice to fix it in current LTSes. > > As far as I can tell, intrins

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-11 Thread Kim Barrett
On Fri, 11 Apr 2025 08:51:56 GMT, Aleksey Shipilev wrote: >> I think it would be "easier" to shift `@IntrinsicCandidate` to a private >> `Reference.getImpl/get0` pair of methods, and intrinsify one of those >> instead. Pretty much like current `clear` and `refersTo` are doing it now. >> And it

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-11 Thread Aleksey Shipilev
On Fri, 11 Apr 2025 08:03:07 GMT, Aleksey Shipilev wrote: >> You need to use intrinsic predicates to add runtime check for receiver. See: >> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/library_call.cpp#L167 > > I think it would be "easier" to shift `@IntrinsicCandidate` to a

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-11 Thread Kim Barrett
On Wed, 2 Apr 2025 18:38:17 GMT, Kim Barrett wrote: >> Kim Barrett has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - remove timeout by using waitForReferenceProcessing >> - make ill-timed gc in non-concurrent case less likely >> - fi

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-11 Thread Aleksey Shipilev
On Thu, 10 Apr 2025 18:26:39 GMT, Vladimir Kozlov wrote: >> src/java.base/share/classes/java/lang/ref/Reference.java line 357: >> >>> 355: @IntrinsicCandidate >>> 356: public T get() { >>> 357: return get0(); >> >> I am looking at this now and wondering how current intrinsics ma

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-10 Thread Vladimir Kozlov
On Thu, 10 Apr 2025 08:08:32 GMT, Aleksey Shipilev wrote: >> Kim Barrett has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - remove timeout by using waitForReferenceProcessing >> - make ill-timed gc in non-concurrent case less likely >>

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-10 Thread Aleksey Shipilev
On Wed, 2 Apr 2025 18:33:16 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curre

Re: RFR: 8352565: Add native method implementation of Reference.get() [v3]

2025-04-05 Thread Kim Barrett
> Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently there is intrinsic support by the interpreter, C1, C2, and

Re: RFR: 8352565: Add native method implementation of Reference.get()

2025-04-05 Thread Chen Liang
On Sat, 29 Mar 2025 21:47:18 GMT, Kim Barrett wrote: > Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently the

Re: RFR: 8352565: Add native method implementation of Reference.get() [v2]

2025-04-05 Thread Kim Barrett
On Tue, 1 Apr 2025 22:01:55 GMT, Brent Christian wrote: >> Kim Barrett has updated the pull request incrementally with one additional >> commit since the last revision: >> >> parameterized return type of native get0 > > test/hotspot/jtreg/gc/TestNativeReferenceGet.java line 162: > >> 160:

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-03 Thread Leonid Mesnik
On Wed, 2 Apr 2025 18:33:16 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curre

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-02 Thread Kim Barrett
On Wed, 2 Apr 2025 18:33:16 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curre

Re: RFR: 8352565: Add native method implementation of Reference.get() [v4]

2025-04-02 Thread Kim Barrett
> Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently there is intrinsic support by the interpreter, C1, C2, and

Re: RFR: 8352565: Add native method implementation of Reference.get() [v2]

2025-04-01 Thread Brent Christian
On Tue, 1 Apr 2025 09:43:28 GMT, Kim Barrett wrote: >> Please review this change which adds a native method providing the >> implementation of Reference::get. Referece::get is an intrinsic candidate, >> so >> this native method implementation is only used when the intrinsic is not. >> >> Curre

Re: RFR: 8352565: Add native method implementation of Reference.get() [v2]

2025-04-01 Thread Kim Barrett
On Tue, 1 Apr 2025 01:54:13 GMT, Chen Liang wrote: >> Kim Barrett has updated the pull request incrementally with one additional >> commit since the last revision: >> >> parameterized return type of native get0 > > src/java.base/share/classes/java/lang/ref/Reference.java line 365: > >> 363:

Re: RFR: 8352565: Add native method implementation of Reference.get() [v2]

2025-04-01 Thread Kim Barrett
> Please review this change which adds a native method providing the > implementation of Reference::get. Referece::get is an intrinsic candidate, so > this native method implementation is only used when the intrinsic is not. > > Currently there is intrinsic support by the interpreter, C1, C2, and