RFR: 8299032: Interface IN_NATIVE oop stores for C2

[Kim Barrett]

On Tue, 10 Jan 2023 09:37:01 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

> > Not a review, since I'm not sufficiently knowledgeable about C2 to really do justice to it. Rather, a couple questions/comments.
> > We made a concerted effort to reduce/eliminate oops in native storage, instead using OopStorage entries. This kind of feels like backsliding on that effort. Why?
> 
> It isn't a backsliding on that effort. This code is indeed using OopStorage. An OopHandle::replace uses IN_NATIVE stores. This is essentially an intrinsic that replaces the contents of an OopHandle, so similarly it should use an IN_NATIVE store for that. Having said that, perhaps it would have been more clear if there was a high level function of some sort with a name like oop_handle_replace containing the IN_NATIVE store?

Oh, I see now.  The `make_load` is smashing through the OopHandle abstraction.  That's of course not new to
this change.

>> src/hotspot/share/opto/library_call.cpp line 3353:
>> 
>>> 3351:   thread_obj_handle = _gvn.transform(thread_obj_handle);
>>> 3352:   const TypePtr *adr_type = _gvn.type(thread_obj_handle)->isa_ptr();
>>> 3353:   access_store_at(NULL, thread_obj_handle, adr_type, arr, _gvn.type(arr), T_OBJECT, IN_NATIVE | MO_UNORDERED);
>> 
>> Old code has a `control()` constraint on the store.  Is it okay to drop that?
>
> The first parameter to access_store_at, is the containing object, not the control (which might be different to the address). As for IN_NATIVE stores, there is no containing object, which is what the NULL denotes. In the backend, it does indeed attach the current control inside of BarrierSetC2::store_at_resolved.

Thanks for the explanation.

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

PR: https://git.openjdk.org/jdk/pull/11777