[lworld+vector] RFR: 8314628: [lworld+vector] validation regression fixes and cleanups.

[Jatin Bhateja]

On Tue, 22 Aug 2023 06:20:31 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> Patch fixes following problem seen during validation:-
>> 
>> **CDS:-**
>> - Problem while restoring value object mirrors. These are needed to perform class level comparisons.
>> 
>> **VM:-**
>> - Remove payload offset computation logic during VM initialization. It was earlier used during object re-materialization.
>> 
>> **ci Model:-**
>> - Add a generic routine to handle synthetic ciField population for both static and non-static multifield. This enables returning fully populated ciMultiField structure to compilers during parsing.
>> 
>> **C1 compiler:-**
>> - Incorrect address displacement computation in HIR while updating multifield bundle using "withfield" bytecode in VectorPayalod constructor. This was causing  failures in Double128Mask tests.
>> 
>> **C2 compiler:-**
>> 1.  Allow vector IR creation under strict check for multifield in default_value creation routine. This was incorrectly creating vector IR for non-vector API tests for Double/Long types.
>> 2. Disable inline expansion of Unsafe.finishPrivateBuffer.  This is fixing the incorrectness problems seen with masked operations loop e.g.
>> 
>>>     vt = newPayload();
>>>     vt = Unsafe.makePrivateBuffer(vt);
>>>     for (int i = 0; i < Double128.length; i++) {
>>>          if (mask[i]) {
>>>               Unsafe.putDouble(vt, apply(src[i]));
>>>          }
>>>     }
>> 
>>    Unsafe.putDouble() only modifies the memory state by storing a new value in payload buffer but does not update any local variable in the JVM state corresponding to latch block. Thus no inductive Phi node is inserted in the loop, it also re-materializes InlineTypeNode from modified memory state. This causes incorrectness if second mask value is false as entire payload is overwritten with default value and earlier lane updates are over-written. In order to be safe, disabling intrinsification of finishPrivateBuffer if incoming argument node is of VectorPayload* type, so that it always receive updated buffer argument.
>> 
>> 3. With non-incremental in-lining (-XX:-IncrementalInline) flow user defined routines returning a vector object is incorrectly returning uninitialized VBA connected to oop input of InlineType IR. Defer returning VectorBoxAllocation node, they are expanded and initialized during box expansion and replaces all uses of box.
>> 
>> 4. Currently we try to sharpen the object type to a narrower type to optimize object type comparison against a class constant. This is achieved by inserting a CheckCastPP node which casts the object type to ...
>
> src/hotspot/share/opto/library_call.cpp line 2658:
> 
>> 2656:   }
>> 2657:   InlineTypeNode* vt = buffer->as_InlineType();
>> 2658:   if (!vt->is_allocated(&_gvn) || VectorSupport::is_vector_payload_mf(vt->inline_klass()->get_InlineKlass())) {
> 
> This will make the `finishPrivateBuffer` un-inlined in all Vector API cases, won't it? This seems not so efficient to me. But I cannot find a better way to fix the masked cases now.

You are right, but it will not affect the performance given that code is part of fallback implementation.

> src/hotspot/share/opto/vector.cpp line 336:
> 
>> 334: 
>> 335:   Node* klass_node = kit.makecon(TypeKlassPtr::make(vk));
>> 336:   Node* alloc_oop  = kit.new_instance(klass_node, NULL, NULL, /* deoptimize_on_exception */ true);
> 
> Is there any issue if adding `vector` to the alloc node's using edge here?

Yes, it causes an [assertion](https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/opto/inlinetypenode.cpp#L1312) failure in InlineTypeNode::remove_redundant_allocations pass which expect such an allocation to be remove during InlineTypeNode Idealizations.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/909#discussion_r1301446721
PR Review Comment: https://git.openjdk.org/valhalla/pull/909#discussion_r1301446609