[lworld+vector] RFR: 8319972: [lworld+vector] Enable intrinsification of Unsafe.finishPrivateBuffer.

[Jatin Bhateja]

On Wed, 15 Nov 2023 08:57:34 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> Re-enabling intrinsification of Unsafe.finishPrivateBuffer to fix performance degradation seen in Vector API JMH micro benchmarks. It's intrinsifation was disabled to fix incorrectness caused due to missing inductive InlineType Phi node in loops with Unsafe.put* operations. Since Unsafe.put* APIs returns a void hence JVM state is not altered and ciTypeFlow dataflow analysis does not encounter a inductive definition in the loop, due to this C2 parser misses creating an inductive phi node on
>> encountering [loop header block](https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/opto/parse1.cpp#L696). 
>> 
>> In order to maintain IR compliance with ciTypeFlow analysis C2 should prevent scalarizing value object b/w make and finish private buffer calls.
>> 
>> New behavior of unsafe inline expanders
>> 1) makePrivate: Receive InlineTypeNode input and return initialized buffer in larval state.
>> 2) finishPrivateBuffer: Receive value object buffer input and return rematerialize InlineTypeNode in non-larval state.
>> 
>> This may result into creation of unbalanced phi node at control flow merges where one of the phi input may InlineTypeNode and other is a buffer of compatible value type, but the IR still remains valid.
>> 
>> In addition compiler is now preventing elimination of allocation in larval state.
>> 
>> Validation Status:-
>>   - All the VectorAPI JTREG tests are now passing at various AVX level with / wo -XX:+DoptimizeALot.
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>
> src/hotspot/share/opto/inlinetypenode.cpp line 1039:
> 
>> 1037:   kit->insert_mem_bar(Op_MemBarStoreStore, alloc->proj_out_or_null(AllocateNode::RawAddress));
>> 1038: 
>> 1039:   return InlineTypeNode::make_from_oop(kit, obj, vk->inline_klass(), !vk->maybe_null());
> 
> I think this is the root cause of incorrectness in https://bugs.openjdk.org/browse/JDK-8317659. The field values of the `InlineTypeNode` are not updated from the changed oop buffer.
> 
> So can we just make `Unsafe.finishPrivateBuffer()` accept `InlineTypeNode` like before, checking its oop is in larval state, and return `make_from_oop()` here?

Following is the API sequence to modify a vector lane.


obj =  Unsafe.makePrivateBuffer(Value.default);
Unsafe.putDouble(obj, OFFSET, value);
obj = Unsafe.finishPrivateBuffer(obj);


In case putDouble is not in-line expanded, compiler makes a call to Unsafe_MakePrivateBuffer native implimentation. There is no way for compiler to detect any JVM state changes since put* returns a void. Thus finishPrivateBuffer will directly use default value and this will cause incorrectness, preventing scalarization ie. creating an InlineTypeNode b/w  make and finish private buffer will fix this problem.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/952#discussion_r1395260160