On Wed, 2 Apr 2025 18:33:16 GMT, Kim Barrett <kbarr...@openjdk.org> 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 there is intrinsic support by the interpreter, C1, C2, and graal,
>> which are always used. With this change we can later remove all the
>> per-platform interpreter intrinsic implementations, and might also remove the
>> C1 intrinsic implementation.
>>
>> Testing:
>> (1) mach5 tier1-6 normal (so using all the existing intrinsics).
>> (2) mach5 tier1-6 with interpreter and C1 Reference::get intrinsics disabled.
>
> 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
> - fix test package use
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 matchers work in
case of virtual calls.
For example, when type information/profile tells us the receiver is generic
`Reference`, but in reality it is a `PhantomReference` subclass, would the call
to `PhantomReference.get()` match accidentally to `Reference.get` intrinsic,
and thus enter Access API wit `ON_WEAK_REF`? Looks pre-existing, and I would
have expected native code to assert, but I also think at least C2 intrinsics do
not check the reference type.
It seems both `clear` and `refersTo` side-step all this by: a) not
intrinsifying the virtual methods; b) doing `AS_NO_KEEPALIVE` -- so they are
not as exposed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24315#discussion_r2036768100
