[vectorIntrinsics+compress] RFR: 8274889: Intrinsify Vector.expand/compress APIs for X86 [v3]

Joshua Zhu jzhu at openjdk.java.net
Fri Oct 8 13:16:18 UTC 2021


On Fri, 8 Oct 2021 11:33:43 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> - Common C2 compiler entry point for compress/expand APIs.
>> - Inline expansion handling and C2 graph creation.
>> - For X86 current backed support is added for AVX512 which offers direct compress/expand instructions.
>> 
>> Patch has been co-contributed by  Joshua Zhu  (@Alibaba)
>
> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8274889: Post merge IR cleanup and special case handling for allTrue mask.

Part of review comments. Will add more.

src/hotspot/cpu/x86/assembler_x86.hpp line 2552:

> 2550:   void evcompressps(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len);
> 2551:   void evcompresspd(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len);
> 2552:   void vpcompresspd(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len);

Please add prefix "e" in the above function name to align with the evex function name convention in this file.

src/hotspot/cpu/x86/assembler_x86.hpp line 2558:

> 2556:   void evpexpandd(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len);
> 2557:   void evpexpandq(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len);
> 2558:   void evpexpandps(XMMRegister dst, KRegister mask, XMMRegister src, bool merge, int vector_len);

Please use "evexpandps" instead of "evpexpandps" to match with the instruction name.

src/hotspot/cpu/x86/x86.ad line 8753:

> 8751:     int vector_len = vector_length_encoding(this);
> 8752:     BasicType bt  = Matcher::vector_element_basic_type(this);
> 8753:     __ compress_expand(opcode, $dst$$XMMRegister, $src$$XMMRegister, $mask$$KRegister, true, bt, vector_len);

Please use zeroing-masking.

src/java.base/share/classes/jdk/internal/vm/vector/VectorSupport.java line 638:

> 636:      M extends VectorMask<E>,
> 637:      E>
> 638:     V comExpOp(int opr,

I prefer boolean instead of int.

src/java.base/share/classes/jdk/internal/vm/vector/VectorSupport.java line 640:

> 638:     V comExpOp(int opr,
> 639:               Class<? extends V> vClass, Class<? extends M> mClass, Class<E> eClass,
> 640:               int length, V v1, V v2, M m,

No need to use two vectors here. Use zeroing-masking with one vector could meet requirement, right?

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

PR: https://git.openjdk.java.net/panama-vector/pull/146


More information about the panama-dev mailing list