[vector api] Max/Min re-implementation

[Vladimir Ivanov]

> http://cr.openjdk.java.net/~vdeshpande/MaxMinVector/webrev.02/

Looks good.

src/hotspot/cpu/x86/assembler_x86.cpp:

Nice improvement! IMO splitting it improves readability:

+ assert(UseAVX > 2 && VM_Version::supports_avx512dq(), "");
+ assert(vector_len == AVX_512bit || VM_Version::supports_avx512vl(), "");

Best regards,
Vladimir Ivanov

>> -----Original Message-----
>> From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
>> Sent: Friday, March 1, 2019 4:15 AM
>> To: Bhateja, Jatin <jatin.bhateja at intel.com>; panama-dev at openjdk.java.net
>> Subject: Re: [vector api] Max/Min re-implementation
>>
>> Hi Jatin,
>>
>>> http://cr.openjdk.java.net/~kkharbas/MaxMinVector/webrev.00
>>
>> src/hotspot/cpu/x86/assembler_x86.cpp:
>>
>> +void Assembler::evpmovd2m(KRegister kdst, XMMRegister src, int
>> vector_len) {
>> +  assert(UseAVX > 2  && VM_Version::supports_avx512bw(), "");
>>
>> +void Assembler::evpmovq2m(KRegister kdst, XMMRegister src, int
>> vector_len) {
>> +  assert(UseAVX > 2  && VM_Version::supports_avx512bw(), "");
>>
>> Intel 64 Architecture Software Developer Manual [1] states that
>> vpmovd2m/vpmovq2m requires either DQ or DQ+VL (depending on
>> vector_len).
>>
>> Does BW imply DQ is available as well? Also, how hard would it be to assert
>> VL when needed?
>>
>> ================================================================
>> =======
>>
>> src/hotspot/cpu/x86/x86.ad:
>>
>> +          if (UseAVX < 1 && (bt == T_FLOAT || bt == T_DOUBLE))
>>
>> I'd prefer to see UseAVX == 0 check instead.
>>
>> ================================================================
>> =======
>>
>> -instruct min4F_reg_evex(vecX dst, vecX src1, vecX src2) %{
>> +instruct min4F_reg_evex(vecX dst, vecX a, vecX b, vecX atmp, vecX btmp)
>> +%{
>>      predicate(UseAVX > 2 && VM_Version::supports_avx512vl() && ...
>> ...
>>      ins_encode %{
>>        int vector_len = 0;
>> -    __ vminps($dst$$XMMRegister, $src1$$XMMRegister,
>> $src2$$XMMRegister, vector_len);
>> +    KRegister ktmp = k1;
>> +    KRegister mask = k0;
>> +    __ evpmovd2m(ktmp, $a$$XMMRegister, vector_len);
>>
>> Does VL always imply BW/DQ?
>>
>> ================================================================
>> =======
>>
>> +instruct min16F_reg_evex(vecZ dst, vecZ a, vecZ b, vecZ atmp, vecZ
>> +btmp) %{
>> +  predicate(UseAVX > 2 && n->as_Vector()->length() == 16 &&
>> n->bottom_type()->is_vect()->element_basic_type() == T_FLOAT);
>> ...
>> +    int vector_len = 2;
>> +    KRegister ktmp = k1;
>> +    KRegister mask = k0;
>> +    __ evpmovd2m(ktmp, $a$$XMMRegister, vector_len);
>>
>> Don't you need a check for BW/DQ here?
>>
>> (Same for min8D_reg_evex, max16F_reg_evex, max8D_reg_evex.)
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> [1] https://software.intel.com/en-us/articles/intel-sdm