[lworld+vector] RFR: 8307715: Integrate VectorMask/Shuffle with value/primitive classes [v6]

Xiaohong Gong xgong at openjdk.org
Thu May 25 07:17:30 UTC 2023


On Thu, 25 May 2023 06:58:55 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> 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 of  TypePtr::NULL_PTR in if condition, it will handle all the cases.

Make sense to me. But if  the constant oop is `gvn.zerocon(T_PRIMITIVE_OBJECT)`, it's more reasonable that accessing the multi-fields is invalid in runtime, right? I think compiler should exclude such cases before calling `InlineTypeNode::load`.  WDYT?

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/845#discussion_r1205088977



More information about the valhalla-dev mailing list