On Tue, 4 Apr 2023 13:46:12 GMT, Quan Anh Mai <qa...@openjdk.org> wrote:
>> `Vector::slice` is a method at the top-level class of the Vector API that
>> concatenates the 2 inputs into an intermediate composite and extracts a
>> window equal to the size of the inputs into the result. It is used in vector
>> conversion methods where the part number is not 0 to slice the parts to the
>> correct positions. Slicing is also used in text processing such as utf8 and
>> utf16 validation. x86 starting from SSSE3 has `palignr` which does vector
>> slicing very efficiently. As a result, I think it is beneficial to add a C2
>> node for this operation as well as intrinsify `Vector::slice` method.
>>
>> A slice is currently implemented as
>> `v2.rearrange(iota).blend(v1.rearrange(iota), blendMask)` which requires
>> preparation of the index vector and the blending mask. Even with the
>> preparations being hoisted out of the loops, microbenchmarks show
>> improvement using the slice instrinsics. Some have tremendous increases in
>> throughput due to the limitation that a mask of length 2 cannot currently be
>> intrinsified, leading to falling back to the Java implementations.
>>
>> Please take a look and have some reviews. Thank you very much.
>
> Quan Anh Mai has updated the pull request incrementally with one additional
> commit since the last revision:
>
> style
src/hotspot/share/opto/vectorIntrinsics.cpp line 1953:
> 1951: Node* v1 = unbox_vector(argument(3), vbox_type, elem_bt, num_elem);
> 1952: Node* v2 = unbox_vector(argument(4), vbox_type, elem_bt, num_elem);
> 1953: if (v1 == NULL || v2 == NULL) {
nullptr is more common.
src/hotspot/share/opto/vectornode.cpp line 1999:
> 1997: // (VectorSlice X Y 0) => X
> 1998: // (VectorSlice X Y VLENGTH) => Y
> 1999: if (origin->is_con(0)) {
is_con(0) is pre defined as TypeInt::ZERO.
src/hotspot/share/opto/vectornode.cpp line 2001:
> 1999: if (origin->is_con(0)) {
> 2000: return in(1);
> 2001: } else if (origin->is_con(Matcher::vector_length(this))) {
If they were the same, length() looks simple.
Suggestion:
} else if (origin->is_con(length())) {
src/java.base/share/classes/jdk/internal/vm/vector/VectorSupport.java line 635:
> 633: }
> 634:
> 635: @ForceInline
May I ask why `forceInline` here is necessary?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12909#discussion_r1164311234
PR Review Comment: https://git.openjdk.org/jdk/pull/12909#discussion_r1164328793
PR Review Comment: https://git.openjdk.org/jdk/pull/12909#discussion_r1164346771
PR Review Comment: https://git.openjdk.org/jdk/pull/12909#discussion_r1164348507
