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

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

2025-03-29 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 graal, wh