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

Jatin Bhateja jbhateja at openjdk.org
Thu May 25 10:35:25 UTC 2023


On Thu, 25 May 2023 09:14:46 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 ?

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

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



More information about the valhalla-dev mailing list