Re: [PATCH] i386: Call get_available_features for all CPUs with max_level >= 1 [PR100758]

[Uros Bizjak <ubizjak@gmail.com>]

On Thu, Feb 9, 2023 at 4:43 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Feb 09, 2023 at 07:30:52AM -0800, H.J. Lu wrote:
> > On Thu, Feb 9, 2023 at 4:12 AM Jakub Jelinek <jakub@redhat.com> wrote:
> > > get_available_features doesn't depend on cpu_model2->__cpu_{family,model}
> > > and just sets stuff up based on CPUID leaf 1, or some extended ones,
> > > so I wonder why are we calling it separately for Intel, AMD and Zhaoxin
> > > and not for all other CPUs too?  I think various programs in the wild
> > > which aren't using __builtin_cpu_{is,supports} just check the various CPUID
> > > leafs and query bits in there, without blacklisting unknown CPU vendors,
> > > so I think even __builtin_cpu_supports ("sse2") etc. should be reliable
> > > if those VENDOR_{CENTAUR,CYRIX,NSC,OTHER} CPUs set those bits in CPUID leaf
> > > 1 or some extended ones.  Calling it for all CPUs also means it can be
> > > inlined because there will be just a single caller.
> > >
> > > I will test on Intel but can't test it on non-Intel (or with some extra
> > > effort on AMD; for both of those arches it should be really no change in
> > > behavior).
> > >
> > > Thoughts on this?
> >
> > No objection here.   It just isn't easy to verify CPUID behavior on
> > other processors.
>
> Sure, worst case it can be reverted or exceptions could be added if some
> CPUs misbehave but then we'd hopefully have detailed into on how exactly it
> behaves.
>
> FYI, I've successfully bootstrapped/regtested this on Intel i9-7960X
> and Martin Liska has regtested it with just i386.exp tests on AMD.
>
> Uros, is this ok now?

OK. Let's go forward with the patch.

Thanks,
Uros.

>
> > > 2023-02-09  Jakub Jelinek  <jakub@redhat.com>
> > >
> > >         PR target/100758
> > >         * common/config/i386/cpuinfo.h (get_zhaoxin_cpu): Formatting fixes.
> > >         (cpu_indicator_init): Call get_available_features for all CPUs with
> > >         max_level >= 1, rather than just Intel, AMD or Zhaoxin.  Formatting
> > >         fixes.
>
>         Jakub
>