[lworld+vector] RFR: 8307715: Integrate VectorMask/Shuffle with value/primitive classes [v6]
Jatin Bhateja
jbhateja at openjdk.org
Thu May 25 07:01:22 UTC 2023
On Thu, 25 May 2023 02:07:03 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:
>> src/hotspot/share/opto/inlinetypenode.cpp line 448:
>>
>>> 446: Matcher::match_rule_supported_vector(VectorNode::replicate_opcode(bt), vec_len, bt)) {
>>> 447: value = kit->gvn().transform(VectorNode::scalar2vector(value, vec_len, Type::get_const_type(field->type()), false));
>>> 448: }
>>
>> It may be needed for constant oops (gvn.zerocon(T_PRIMITIVE_OBJECT)) as created on following line.
>> Otherwise it may result into semantically incorrect IR which tries to load a vector from a constant.
>
> The reason that I change this here is I met another issue when the constant oops is `IOTA` which saves the index values. For such cases, the `scalar2vector` will result into wrong result. Besides, for subtype vectors, the basic type from line-443 (i.e. `BasicType bt = con_type->basic_type();`) is promoted to `int`. I cannot see the issue if loading a vector from a constant oop. Could you please give an example? Alternatively, I was thinking about generating different IRs for the `ZERO` and `IOTA` constants. But we need to do more check about the actual values, which I think is not so urgent now.
It will not be a problem with constant oops or even default values which are value_mirrors, but gvn.zerocon(T_PRIMITIVE_OBJECT) will be create a ConP node representing a NULL_PTR, any address commutation w.r.t. base is will result into illegal address.
We should keep multi-field handling in both the control paths and add an extract check for ConP (TypePtr::NULL_PTR) in if condition for Type::NULL_PTR it will handle all the cases.
-------------
PR Review Comment: https://git.openjdk.org/valhalla/pull/845#discussion_r1205077032
More information about the valhalla-dev
mailing list