[lworld+vector] RFR: 8304310: Initial compilers and runtime handling for multifield backed vectors. [v5]

Xiaohong Gong xgong at openjdk.org
Mon Apr 10 08:03:11 UTC 2023


On Mon, 10 Apr 2023 07:59:21 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Cleanup vector box expansion handling to leverage existing InlineTypeNode storage and flow merging routines.
>
> src/hotspot/share/opto/vectornode.hpp line 1689:
> 
>> 1687:     payload_value->as_InlineType()->set_field_value(0, val);
>> 1688:     payload_value = gvn.transform(payload_value);
>> 1689:     VectorBoxNode* box_node = new VectorBoxNode(vk, box, false, vk->is_empty() && vk->is_initialized());
> 
> I think we have to keep the `val` as the input of the new `VectorBoxNode`, and moving the payload creating code to the `expand_vbox_node` as before.  `VectorBoxNode` is a macro node for a vector. So it's more reasonable it only contains the vbox and vec parts. The payload creating should belong to the extension stage like before. Besides, some transformation applied to `PhiNode` may expect one of the input of  `VectorBoxNode` is a vector type?
> 
> I met a jvm crash issue with following test case on ARM NEON system:
> 
>  var av = IntVector.fromArray(I_SPECIES, ia, 0);
>  for (int i = 0; i < LENGTH; i += vl) {
>        var bv = IntVector.fromArray(I_SPECIES, ib, i);
>        av = av.and(bv);
>  }
>  av.intoArray(ic, 0);
> 
> The log shows it crashes when the type is not mismatch at a `PhiNode`.  One is `TypeVectX`, and another is `TypePtr`. I didn't dig into the `cfgnode.cpp` to see what is wrong. But as a try, I moved this code to the expanding stage, the issue is gone.

Here is the changes in `vector.cpp`:

diff --git a/src/hotspot/share/opto/vector.cpp b/src/hotspot/share/opto/vector.cpp
index d7329e72c..524791ea8 100644
--- a/src/hotspot/share/opto/vector.cpp
+++ b/src/hotspot/share/opto/vector.cpp
@@ -322,9 +322,18 @@ void PhaseVector::expand_vbox_node(VectorBoxNode* vec_box) {
     GraphKit kit(jvms);

     ciInlineKlass* vk = vec_box->inline_klass();
+    ciInlineKlass* payload = vk->declared_nonstatic_field_at(0)->type()->as_inline_klass();
+    Node* payload_value = InlineTypeNode::make_uninitialized(kit.gvn(), payload, true);
+    payload_value->as_InlineType()->set_field_value(0, vec_box->get_vec());
+    payload_value = kit.gvn().transform(payload_value);
+
+    InlineTypeNode* vector = InlineTypeNode::make_uninitialized(kit.gvn(), vk, false);
+    vector->set_field_value(0, payload_value);
+    vector = kit.gvn().transform(vector)->as_InlineType();
+
     Node* klass_node = kit.makecon(TypeKlassPtr::make(vk));
-    Node* alloc_oop  = kit.new_instance(klass_node, NULL, NULL, /* deoptimize_on_exception */ true);
-    vec_box->store(&kit, alloc_oop, alloc_oop, vk);
+    Node* alloc_oop  = kit.new_instance(klass_node, NULL, NULL, /* deoptimize_on_exception */ true, vector);
+    vector->store(&kit, alloc_oop, alloc_oop, vk);

     // Do not let stores that initialize this buffer be reordered with a subsequent
     // store that would make this buffer accessible by other threads.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1161523573



More information about the valhalla-dev mailing list