On Mon, 19 Aug 2024 07:19:30 GMT, Jatin Bhateja <jbhat...@openjdk.org> wrote:

>> Hi All,
>> 
>> As per the discussion on panama-dev mailing list[1], patch adds the support 
>> following new vector operators.
>> 
>> 
>>      . SUADD   : Saturating unsigned addition.
>>      . SADD    : Saturating signed addition. 
>>      . SUSUB   : Saturating unsigned subtraction.
>>      . SSUB    : Saturating signed subtraction.
>>      . UMAX    : Unsigned max
>>      . UMIN    : Unsigned min.
>>      
>> 
>> New vector operators are applicable to only integral types since their 
>> values wraparound in over/underflowing scenarios after setting appropriate 
>> status flags. For floating point types, as per IEEE 754 specs there are 
>> multiple schemes to handler underflow, one of them is gradual underflow 
>> which transitions the value to subnormal range. Similarly, overflow 
>> implicitly saturates the floating-point value to an Infinite value.
>> 
>> As the name suggests, these are saturating operations, i.e. the result of 
>> the computation is strictly capped by lower and upper bounds of the result 
>> type and is not wrapped around in underflowing or overflowing scenarios.
>> 
>> Summary of changes:
>> - Java side implementation of new vector operators.
>> - Add new scalar saturating APIs for each of the above saturating vector 
>> operator in corresponding primitive box classes, fallback implementation of 
>> vector operators is based over it.
>> - C2 compiler IR and inline expander changes.
>> - Optimized x86 backend implementation for new vector operators and their 
>> predicated counterparts.
>> - Extends existing VectorAPI Jtreg test suite to cover new operations.
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> PS: Intrinsification and auto-vectorization of new core-lib API will be 
>> addressed separately in a follow-up patch.
>> 
>> [1] https://mail.openjdk.org/pipermail/panama-dev/2024-May/020408.html
>
> Jatin Bhateja has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Review comments resolutions.

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

> 1771:         return false;
> 1772:       }
> 1773:       if (bt == T_LONG && !VM_Version::supports_avx512vl()) {

we should be able to support bt == T_LONG for 512 bit irrespective of avx512vl.

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

> 1951:        if (UseAVX < 1 || size_in_bits < 128 || (size_in_bits == 512 && 
> !VM_Version::supports_avx512bw())) {
> 1952:          return false;
> 1953:        }

UseAVX < 1 could be written as UseAVX == 0. Could we not do register version 
for size_in_bit < 128?

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

> 1960:         return false; // Implementation limitation
> 1961:       }
> 1962:       break;

Could we not do register version for size_in_bit < 128?

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

> 2141:       if (is_subword_type(bt) && !VM_Version::supports_avx512bw()) {
> 2142:         return false; // Implementation limitation
> 2143:       }

UMinV and UMaxV are supported on AVX1, AVX2 platform.

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

> 2153:         return false; // Implementation limitation
> 2154:       }
> 2155:       return true;

Byte/Short saturating vector add is supported for AVX1, AVX2 platforms. Could 
we not do register version for size_in_bit < 128?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20507#discussion_r1733330892
PR Review Comment: https://git.openjdk.org/jdk/pull/20507#discussion_r1733333203
PR Review Comment: https://git.openjdk.org/jdk/pull/20507#discussion_r1733333608
PR Review Comment: https://git.openjdk.org/jdk/pull/20507#discussion_r1733336005
PR Review Comment: https://git.openjdk.org/jdk/pull/20507#discussion_r1733338300

Reply via email to