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

[Xiaohong Gong]

On Mon, 21 Aug 2023 07:36:19 GMT, Jatin Bhateja <jbhateja 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 a higher speculative type, if cast type is an inlinetyp...

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

> 2342:     if (is_store) {
> 2343:       ciInlineKlass* vk = vt->type()->inline_klass();
> 2344:       if (!vt->is_allocated(&_gvn)) {

`vk` is unused in this block, so it can be removed?

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

> 2398:       }
> 2399:     } else if ((is_vector_value && skip_scalarization) ||
> 2400:                (value->Opcode() != Op_VectorBox && value->is_InlineType())) {

`is_vector_value` can be removed in the `else-if` condition since `skip_scalarization = is_vector_value && Compile::current()->inlining_incrementally();` , right?

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?

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/909#discussion_r1300845435
PR Review Comment: https://git.openjdk.org/valhalla/pull/909#discussion_r1300850192
PR Review Comment: https://git.openjdk.org/valhalla/pull/909#discussion_r1300850651