[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