> -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: Tuesday, July 11, 2023 3:50 PM > To: Liu, Hongtao <hongtao....@intel.com> > Cc: Kirill Yukhin <kirill.yuk...@gmail.com>; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] x86: improve fast bfloat->float conversion > > On 11.07.2023 08:45, Liu, Hongtao wrote: > >> -----Original Message----- > >> From: Jan Beulich <jbeul...@suse.com> > >> Sent: Tuesday, July 11, 2023 2:08 PM > >> > >> 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. > > Thanks. > > >> --- > >> 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'm not sure I understand: It sounds like what Jakub said matches my > observation, yet then it seems unlikely that the issue wasn't fixed in over > half > a year. > > Also having the expander FAIL when HONOR_NANS (matching what I was > thinking) still doesn't clarify to me what then would happen to uses of the > builtin. Is there any (common code) fallback for such a case? I didn't think > there would be, in which case wouldn't this result in an internal compiler > error? For __bf16 -> float or target specific builtins, it should be ok since __bf16 is just an extension type. but extendbfsf2 is a standard pattern name which is also used to expand c++23 std::bfloat16_t -> float conversion which is assumed to raise exceptions for sNAN. Since vpslld won't raise any exception, we need to add HONOR_NANS in the extendbfsf2 pattern. It's my understanding, for std:bfloat16_t support, it's mentioned in [2].
https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601865.html > > Jan