[vector api] Max/Min re-implementation

[Viswanathan, Sandhya]

Hi Jatin,

This looks good. I will push it today.

Thanks,
Sandhya


-----Original Message-----
From: Bhateja, Jatin 
Sent: Tuesday, March 05, 2019 10:10 PM
To: Vladimir Ivanov <vladimir.x.ivanov at oracle.com>; Viswanathan, Sandhya <sandhya.viswanathan at intel.com>
Cc: panama-dev at openjdk.java.net
Subject: RE: [vector api] Max/Min re-implementation

Hi Vladimir, Sandhya,

Please find below updated link to patch.

http://cr.openjdk.java.net/~sviswanathan/Jatin/vectorIntrinsics/FPMinMax/webrev.04/

Regards,
Jatin

> -----Original Message-----
> From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
> Sent: Saturday, March 2, 2019 2:13 AM
> To: Bhateja, Jatin <jatin.bhateja at intel.com>
> Cc: panama-dev at openjdk.java.net; Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com>
> Subject: Re: [vector api] Max/Min re-implementation
> 
> 
> > 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