Re: [PATCH] x86_64: Exclude SSE, AVX and FMA4 variants in libm multiarch

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Tue, Feb 20, 2024 at 8:03 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 20/02/24 16:56, Adhemerval Zanella Netto wrote:
> >
> >
> > On 20/02/24 16:10, H.J. Lu wrote:
> >> On Tue, Feb 20, 2024 at 11:02 AM Adhemerval Zanella Netto
> >> <adhemerval.zanella@linaro.org> wrote:
> >>>
> >>>
> >>>
> >>> On 20/02/24 15:54, H.J. Lu wrote:
> >>>> On Tue, Feb 20, 2024 at 10:48 AM Adhemerval Zanella Netto
> >>>> <adhemerval.zanella@linaro.org> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>> On 20/02/24 15:36, H.J. Lu wrote:
> >>>>>> On Tue, Feb 20, 2024 at 10:32 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >>>>>>>
> >>>>>>> On Tue, Feb 20, 2024 at 6:28 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>>>>>>>
> >>>>>>>> On Tue, Feb 20, 2024 at 10:19 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >>>>>>>>>
> >>>>>>>>> On Tue, Feb 20, 2024 at 6:14 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> On Tue, Feb 20, 2024 at 10:07 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> On Tue, Feb 20, 2024 at 6:05 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> On Tue, Feb 20, 2024 at 9:56 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On Tue, Feb 20, 2024 at 5:51 PM Sunil Pandey <skpgkp2@gmail.com> wrote:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> On Tue, Feb 20, 2024 at 9:34 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> On Tue, Feb 20, 2024 at 4:58 PM Sunil K Pandey <skpgkp2@gmail.com> wrote:
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> When glibc is built with FMA and AVX2 enabled by default, the resulting
> >>>>>>>>>>>>>>>> glibc binaries won't run on SSE or FMA4 processors.  Exclude SSE, AVX and
> >>>>>>>>>>>>>>>> FMA4 variants in libm multiarch when both FMA and AVX2 are enabled by
> >>>>>>>>>>>>>>>> default.  Disallow glibc build with only AVX2 or FMA enabled as all AVX2
> >>>>>>>>>>>>>>>> processors, including VMs, should also support FMA and vice versa.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> When glibc is built with SSE4.1 enabled by default, only keep SSE4.1
> >>>>>>>>>>>>>>>> variant.
> >>>>>>>>>>>>>>> Not avx2 + FMA as well?
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Correct. Logic is as follows
> >>>>>>>>>>>>>> If (build with AVX2+FMA): Keep AVX2+FMA variants only.
> >>>>>>>>>>>>>> else if (build with SSE4.1): Keep SSE4.1 variants only.
> >>>>>>>>>>>>> What if someone builds with sse4.1 as a minimum but then
> >>>>>>>>>>>>> runs on avx2+ machines?
> >>>>>>>>>>>>
> >>>>>>>>>>>> Only SSE4.1 variant will be used in this case.   Both SSE4.1
> >>>>>>>>>>>> and AVX versions only have a single instruction.  This matches
> >>>>>>>>>>>> the compiler builtin function of SS4.1 and AVX.
> >>>>>>>>>>>
> >>>>>>>>>>> if they are all the same, whats the rationale for having an
> >>>>>>>>>>> avx version at all?
> >>>>>>>>>>
> >>>>>>>>>> They aren't the same.  For ceil, it is
> >>>>>>>>>>
> >>>>>>>>>> roundsd $10, %xmm0, %xmm0
> >>>>>>>>>> ret
> >>>>>>>>>>
> >>>>>>>>>> vs
> >>>>>>>>>>
> >>>>>>>>>> vroundsd $10, %xmm0, %xmm0, %xmm0
> >>>>>>>>>> ret
> >>>>>>>>>>
> >>>>>>>>>> You get the same things with
> >>>>>>>>>>
> >>>>>>>>>> return __builtin_ceil (x);
> >>>>>>>>>
> >>>>>>>>> I mean if they are equal quality sse4.1 / avx,
> >>>>>>>>> why not just remove the avx impls are using sse4.1 impls
> >>>>>>>>> on avx targets?
> >>>>>>>>
> >>>>>>>> If glibc is compiled with AVX, we should use the AVX version if
> >>>>>>>> appropriate.   Since the minimum GCC for glibc build can't inline
> >>>>>>>>  __builtin_ceil, we inline  __builtin_ceil by hand.
> >>>>>>> if compiled with avx, but for generic target do we need to hold
> >>>>>>> onto avx versions for any reason?
> >>>>>>
> >>>>>> I don't understand what you were asking.   This patch leads to the same
> >>>>>> assembly code generated from
> >>>>>>
> >>>>>> double
> >>>>>> __ceil (double x)
> >>>>>> {
> >>>>>>   return __builtin_ceil (x);
> >>>>>> }
> >>>>>
> >>>>> Wouldn't make sense to follow the already define x86_64 ABI versions and
> >>>>> provided the ifunc variants based on the ABI uses?
> >>>>
> >>>> There are no conflicts here.  For these math functions, ISA level 2 == SSE4.1
> >>>> and ISA level 3 == AVX2 + FMA.   If glibc is built with ISA level N, this patch
> >>>> will exclude ISA level N-1 or older variants in IFUNC selection.
> >>>>
> >>>
> >>> I mean, why not use the MINIMUM_X86_ISA_LEVEL to define whether to provide/build
> >>> the variants instead of adding two new configure checks?
> >>
> >> One issue is that the minimum GCC (GCC 6?) doesn't support -march=x86-64-vN.
> >> Another reason is that these math functions don't need the full ISA
> >> level instructions.
> >>
> >
> > But afaiu gcc support for -march=x86_64-vN does not really matter, isa-level.h
> > will define MINIMUM_X86_ISA_LEVEL based compiler preprocessor that should be
> > compatible with GCC 6.
> >
> > So instead of checking for HAVE_X86_AVX2_FMA, it would be for
> > MINIMUM_X86_ISA_LEVEL >= 3. It does not work for C files that explicit uses
> > -march, but I don't that is the case here.
>
> I understand that math support is not really tied to x86_64-vX, but I also think
> that there is not strong reason to also use the same logic libc.so is already
> using on ifunc variant to provide the ifunc variants on libm.so.
>
> It simplifies the testing and minimize the build permutation, since I know that
> checking for -march=x86_64-v{1,2,3,4} should be suffice instead of adding
> -mfma, etc.
>
+1
> So I also think that instead of libc_cv_have_x86_avx2_fma/libc_cv_have_x86_sse4_1,
> it should check for libc_cv_x86_64_vN (similar to what isa-level.h does).  Also,
> I don't see much point in adding support for 32 bits.