On Fri, 23 May 2025 23:01:02 GMT, Vladimir Ivanov <vliva...@openjdk.org> 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 the approach being taken here better than what's in those.
>
> 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 there. 
>  So, unless I miss something, I believe the comment is misleading.

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. That change was about migrating
away from having a method that was all 3 of (1) intrinsic (2) native, and (3)
virtual. It split that method into two parts, and allocated the properties to
them in such a way that neither had all 3. There are several possible
allocations of the properties to the methods that would have worked. In this
case we already had all the machinary in place for `refersTo0` to be intrinsic
and native, so the allocation that didn't require a bunch of renaming for
consistency was to introduce `refersToImpl` to provide the virtual part.

For this PR we have a method that is already intrinsic and virtual. Of course,
we don't want to make it native too. Since we already have all the machinary
in place for `get` to be intrinsic and virtual, the allocation that didn't
require a bunch of renaming for consistency was to introduce native `get0` for
use as the non-intrinsic implementation of `get`. See also here:
https://github.com/openjdk/jdk/pull/24315#discussion_r2039137923

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24315#discussion_r2105706304

Reply via email to