On Fri, 12 Jul 2024 13:19:31 GMT, Aleksey Shipilev <sh...@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
