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
