On Wed, 22 Mar 2023 07:59:40 GMT, Xiaohong Gong <[email protected]> wrote:
>> Quan Anh Mai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> reviews
>
> src/hotspot/share/opto/vectorIntrinsics.cpp line 2059:
>
>> 2057: if (need_load_shuffle) {
>> 2058: shuffle = gvn().transform(new VectorLoadShuffleNode(shuffle, vt));
>> 2059: }
>
> How about generating `VectorLoadShuffleNode` for all platforms that support
> Vector API, and remove the helper method `vector_needs_load_shuffle()` ? For
> those platforms that do not need this shuffle preparation, we can emit
> nothing in codegen.
I think not emitting `VectorLoadShuffleNode` is more common so it is better to
emit them only when needed, as it will simplify the graph and may allow better
inspections of the indices in the future. Additionally, a do-nothing node does
not alias with its input and therefore kills the input, which leads to an
additional spill if they both need to live.
> src/hotspot/share/opto/vectorIntrinsics.cpp line 2426:
>
>> 2424: if (is_vector_shuffle(vbox_klass_from)) {
>> 2425: return false; // vector shuffles aren't supported
>> 2426: }
>
> Is it better to change this as an "assertion" or print the log details?
The change indifferentiates a vector shuffle from a normal vector in C2, so
this should be removed, as vector shuffles are converted to/from normal vector
using this routine
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13093#discussion_r1144748489
PR Review Comment: https://git.openjdk.org/jdk/pull/13093#discussion_r1144744663
