RFR: 8352565: Add native method implementation of Reference.get() [v6]
Kim Barrett
kbarrett at openjdk.org
Thu May 29 13:10:52 UTC 2025
On Thu, 29 May 2025 01:14:30 GMT, Vladimir Ivanov <vlivanov at openjdk.org> 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. 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
>
> 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 problem with virtual intrinsic methods is that when devirtualization fails, the call site ends up permanently calling into non-intrinsified version. That's what `refersTo0` initially suffered (fixed by JDK-8271862). `Reference::refersTo0` was virtual intrinsic method (no intrinsic when no inlining, hence call into native) and once it was untangled (virtual `refersToImpl` calls non-virtual intrinsic method `refersTo0`) intrinsification happens reliably in the context of `refersToImpl`.
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 non-virtual native method be the intrinsic then we
could remove that special handling? Maybe that's a good enough reason to cover
the cost of renaming churn. Or maybe that's better done as a later cleanup?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24315#discussion_r2113909448
More information about the hotspot-dev
mailing list