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

[Xiaohong Gong]

On Tue, 21 Nov 2023 16:45:54 GMT, Jatin Bhateja <jbhateja 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 and Valhalla JTREG tests are now passing at various AVX level with / wo -XX:+DoptimizeALot.
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Some more fixes and cleanups
>  - Merge branch 'lworld+vector' of http://github.com/openjdk/valhalla into JDK-8319972
>  - Review comments resolutions
>  - Regression fixes
>  - 8319972: [lworld+vector] Enable intrinsification of Unsafe.finishPrivateBuffer.

src/hotspot/share/opto/library_call.cpp line 2663:

> 2661:   if (!buffer->bottom_type()->is_inlinetypeptr()) {
> 2662:      return false;
> 2663:   }

Suggestion:

  // Incoming value should be a buffer with inline type and not InlineTypeNode.
  if (buffer->is_InlineType() || !buffer->bottom_type()->is_inlinetypeptr()) {
     return false;
  }

src/hotspot/share/opto/parse1.cpp line 1782:

> 1780:           // Scalarize null in src block to be able to merge it with inline type in target block
> 1781:           assert(gvn().type(n)->is_zero_type(), "Should have been scalarized");
> 1782:           map()->set_req(j, InlineTypeNode::make_null(gvn(), t->inline_klass()));

Did you check why this assertion is hit for larval state buffer? I mean is it possible that the inline type buffer in larval state should not go to this path.

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

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