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

Xiaohong Gong xgong at openjdk.org
Mon May 29 06:33:21 UTC 2023


On Mon, 29 May 2023 01:42:17 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>>> Make sense. But the condition in [line-854](https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/opto/inlinetypenode.cpp#L854) is checking the oop itself instead of the `null_free` input. So if the oop is the `NULL_PTR`, it will be catched as well? I tried to hard the oop to be `gvn.zerocon(T_PRIMITIVE_OBJECT)`, and the code goes to [line-859](https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/opto/inlinetypenode.cpp#L859). Hence the followed `load()` cannot be executed.
>>> 
>> Unbalanced PhiNode indicate creation of a scalar, you can put a FIXME over the specific code, we can then address it in subsequent patches.
>> 
>>> BTW, did you run the test again with the latest patch of this PR? Not sure whether the issue you met before is fixed, since we have changed a lot in compiler. Maybe we can add an assertion that the constant oop is not `NULL_PTR`, WDYT?
>> 
>> Thanks, did not do a test re-run with latest patch as there were several regressions in pervious one, one quick comment, kindly set can_optimize = false before following line,
>> https://github.com/openjdk/valhalla/pull/845/files#diff-e368502932198721c3ee52ace2397c5a9c6281930e83fc53a429573f6adf44fcR2584
>> 
>> I have bug fix patch ready over my pending merge patch with which I see 90% *VectorTests*.java test passing,  I shall float it once all tests are green.
>> 
>> Its ok to integrate your patch first, followed by a re-merge and then bug fixes for failing JTREG regression over it. WDYT ?
>
>> Its ok to integrate your patch first, followed by a re-merge and then bug fixes for failing JTREG regression over it. WDYT ?
> 
> Sounds good to me! Thanks!

> Thanks, did not do a test re-run with latest patch as there were several regressions in pervious one, one quick comment, kindly set can_optimize = false before following line,
https://github.com/openjdk/valhalla/pull/845/files#diff-e368502932198721c3ee52ace2397c5a9c6281930e83fc53a429573f6adf44fcR2584

It seems not right setting `can_optimize = false` before line-2584 for me. This will break the inner loop anyway, since it doesn't set `can_optimize = true` in the followed code, does it?

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

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



More information about the valhalla-dev mailing list