[vector api] Max/Min re-implementation

[Bhateja, Jatin]

Hi Vladimir,

Please find updated patch incorporating your comments.

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


Thanks,
Jatin

> -----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