RFR: 8329597: C2: Intrinsify Reference.clear

Aleksey Shipilev shade at openjdk.org
Wed Jul 17 18:52:13 UTC 2024


On Fri, 12 Jul 2024 13:19:31 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>>> > The reason we did not do this before is that this is not a strong reference store. Strong reference stores with a SATB collector will keep the referent alive, which is typically the exact opposite of what a user wants when they clear a Reference.
>>> 
>>> You mean not doing this store just on the Java side? Yes, I agree, it would be awkward. In intrinsic, we are storing with the same decorators that `JVM_ReferenceClear` is using, which should be good with SATB collectors. Perhaps I am misunderstanding the comment.
>> 
>> The runtime use of the Access API knows how to resolve an unknown oop ref strength using AccessBarrierSupport::resolve_unknown_oop_ref_strength. However, we do not have support for that in the C2 backend. In fact, it does not understand non-strong oop stores at all. Because there hasn't really been a use case for it, other than clearing a Reference. That's the precise reason why we do not have a clear intrinsic; it would have to add that infrastructure.
>
>> The runtime use of the Access API knows how to resolve an unknown oop ref strength using AccessBarrierSupport::resolve_unknown_oop_ref_strength. However, we do not have support for that in the C2 backend. In fact, it does not understand non-strong oop stores at all. 
> 
> Aw, nice usability landmine. I thought C2 barrier set would assert on me if it cannot deliver. Apparently not, I see it just does pre-barriers when it is not sure what strongness the store is. Hrmpf. OK, let me see what can be done here. It might be just easier to further specialize `Reference.clear` in subclasses and carry down the actual strongness, like we do with `refersTo0` currently. This would still require C2 backend adjustments to handle `AS_NO_KEEPALIVE` on stores, but at least we would not have to guess about the strongness type in C2 intrinsic.

> I should have read what I was replying to more carefully, rather than focusing on what was further up in the thread. Looks like you (@shipilev) already spotted the refersTo stuff. But the enqueue => clear0 could have easily been missed, so perhaps not an entirely unneeded suggestion.

Yeah, thanks. The `enqueue => clear0` was indeed easy to miss.

Pushed the crude prototype that follows `refersTo` example and drills some new `AS_NO_KEEPALIVE` holes in C2 Access API to cover this intrinsic case. Super untested. IR tests are still failing, I'll take more in-depth look there. (Perhaps it would not be possible to clearly match the absence of pre-barrier in IR tests, we'll see.)

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

PR Comment: https://git.openjdk.org/jdk/pull/20139#issuecomment-2231613218


More information about the core-libs-dev mailing list