On Thu, Feb 9, 2023 at 4:12 AM Jakub Jelinek <ja...@redhat.com> wrote: > > Hi! > > 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. Thanks. > 2023-02-09 Jakub Jelinek <ja...@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. > > --- gcc/common/config/i386/cpuinfo.h.jj 2023-01-16 11:52:15.910736614 +0100 > +++ gcc/common/config/i386/cpuinfo.h 2023-02-09 12:51:23.539470140 +0100 > @@ -601,8 +601,8 @@ get_intel_cpu (struct __processor_model > > static inline const char * > get_zhaoxin_cpu (struct __processor_model *cpu_model, > - struct __processor_model2 *cpu_model2, > - unsigned int *cpu_features2) > + struct __processor_model2 *cpu_model2, > + unsigned int *cpu_features2) > { > const char *cpu = NULL; > unsigned int family = cpu_model2->__cpu_family; > @@ -1016,6 +1016,10 @@ cpu_indicator_init (struct __processor_m > extended_model = (eax >> 12) & 0xf0; > extended_family = (eax >> 20) & 0xff; > > + /* Find available features. */ > + get_available_features (cpu_model, cpu_model2, cpu_features2, > + ecx, edx); > + > if (vendor == signature_INTEL_ebx) > { > /* Adjust model and family for Intel CPUS. */ > @@ -1030,9 +1034,6 @@ cpu_indicator_init (struct __processor_m > cpu_model2->__cpu_family = family; > cpu_model2->__cpu_model = model; > > - /* Find available features. */ > - get_available_features (cpu_model, cpu_model2, cpu_features2, > - ecx, edx); > /* Get CPU type. */ > get_intel_cpu (cpu_model, cpu_model2, cpu_features2); > cpu_model->__cpu_vendor = VENDOR_INTEL; > @@ -1049,9 +1050,6 @@ cpu_indicator_init (struct __processor_m > cpu_model2->__cpu_family = family; > cpu_model2->__cpu_model = model; > > - /* Find available features. */ > - get_available_features (cpu_model, cpu_model2, cpu_features2, > - ecx, edx); > /* Get CPU type. */ > get_amd_cpu (cpu_model, cpu_model2, cpu_features2); > cpu_model->__cpu_vendor = VENDOR_AMD; > @@ -1059,22 +1057,17 @@ cpu_indicator_init (struct __processor_m > else if (vendor == signature_CENTAUR_ebx && family < 0x07) > cpu_model->__cpu_vendor = VENDOR_CENTAUR; > else if (vendor == signature_SHANGHAI_ebx > - || vendor == signature_CENTAUR_ebx) > + || vendor == signature_CENTAUR_ebx) > { > /* Adjust model and family for ZHAOXIN CPUS. */ > if (family == 0x07) > - { > - model += extended_model; > - } > + model += extended_model; > > cpu_model2->__cpu_family = family; > cpu_model2->__cpu_model = model; > > - /* Find available features. */ > - get_available_features (cpu_model, cpu_model2, cpu_features2, > - ecx, edx); > /* Get CPU type. */ > - get_zhaoxin_cpu (cpu_model, cpu_model2,cpu_features2); > + get_zhaoxin_cpu (cpu_model, cpu_model2, cpu_features2); > cpu_model->__cpu_vendor = VENDOR_ZHAOXIN; > } > else if (vendor == signature_CYRIX_ebx) > > Jakub > -- H.J.