RE: [PATCH] x86: improve fast bfloat->float conversion

["Liu, Hongtao" <hongtao.liu@intel.com>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, July 11, 2023 2:08 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Liu, Hongtao <hongtao.liu@intel.com>; Kirill Yukhin
> <kirill.yukhin@gmail.com>
> Subject: [PATCH] x86: improve fast bfloat->float conversion
> 
> There's nothing AVX512BW-ish in here, so no reason to use Yw as the
> constraints for the AVX alternative. Furthermore by using the 512-bit form of
> VPSSLD (in a new alternative) all 32 registers can be used directly by the insn
> without AVX512VL needing to be enabled.
Yes, the instruction vpslld doesn't need AVX512BW, the patch LGTM.
> 
> Also adjust the originally last alternative's "prefix" attribute to maybe_evex.
> 
> gcc/
> 
> 	* config/i386/i386.md (extendbfsf2_1): Add new AVX512F
> 	alternative. Adjust original last alternative's "prefix"
> 	attribute to maybe_evex.
> ---
> The corresponding expander, "extendbfsf2", looks to have been dead since
> its introduction in a1ecc5600464 ("Fix incorrect _mm_cvtsbh_ss"): The builtin
> references the insn (extendbfsf2_1), not the expander. Can't the expander
> be deleted and the name of the insn then pruned of the _1 suffix? If so, that
> further raises the question of the significance of the "!HONOR_NANS
> (BFmode)" that the expander has, but the insn doesn't have. Which may
> instead suggest the builtin was meant to reference the expander. Yet then I
> can't see what would the builtin would expand to when HONOR_NANS
> (BFmode) it true.

Quote from what Jakub said in [1].
-------
This is not correct.
While using such code for _mm_cvtsbh_ss is fine if it is documented not to
raise exceptions and turn a sNaN into a qNaN, it is not fine for HONOR_NANS
(i.e. when -ffast-math is not on), because a __bf16 -> float conversion
on sNaN should raise invalid exception and turn it into a qNaN.
We could have extendbfsf2 expander that would FAIL; if HONOR_NANS and
emit extendbfsf2_1 otherwise. 
-------
[1] https://gcc.gnu.org/pipermail/gcc-patches/2022-November/607108.html
> 
> I further wonder whether the nearby "extendhfdf2" expander is really
> needed. It doesn't look to specify anything that the corresponding insn
> doesn't also specify.
> 
> --- a/gcc/config/i386/i386.md
> +++ b/gcc/config/i386/i386.md
> @@ -5181,21 +5181,27 @@
>  ;; Don't use float_extend since psrlld doesn't raise  ;; exceptions and turn a
> sNaN into a qNaN.
>  (define_insn "extendbfsf2_1"
> -  [(set (match_operand:SF 0 "register_operand"   "=x,Yw")
> +  [(set (match_operand:SF 0 "register_operand"   "=x,Yv,v")
>  	(unspec:SF
> -	  [(match_operand:BF 1 "register_operand" " 0,Yw")]
> +	  [(match_operand:BF 1 "register_operand" " 0,Yv,v")]
>  	  UNSPEC_CVTBFSF))]
>   "TARGET_SSE2"
>   "@
>    pslld\t{$16, %0|%0, 16}
> -  vpslld\t{$16, %1, %0|%0, %1, 16}"
> -  [(set_attr "isa" "noavx,avx")
> +  vpslld\t{$16, %1, %0|%0, %1, 16}
> +  vpslld\t{$16, %g1, %g0|%g0, %g1, 16}"
> +  [(set_attr "isa" "noavx,avx,*")
>     (set_attr "type" "sseishft1")
>     (set_attr "length_immediate" "1")
> -   (set_attr "prefix_data16" "1,*")
> -   (set_attr "prefix" "orig,vex")
> -   (set_attr "mode" "TI")
> -   (set_attr "memory" "none")])
> +   (set_attr "prefix_data16" "1,*,*")
> +   (set_attr "prefix" "orig,maybe_evex,evex")
> +   (set_attr "mode" "TI,TI,XI")
> +   (set_attr "memory" "none")
> +   (set (attr "enabled")
> +     (if_then_else (eq_attr "alternative" "2")
> +       (symbol_ref "TARGET_AVX512F && !TARGET_AVX512VL
> +		    && !TARGET_PREFER_AVX256")
> +       (const_string "*")))])
> 
>  (define_expand "extend<mode>xf2"
>    [(set (match_operand:XF 0 "nonimmediate_operand")