[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