Re: V3: [PATCH] x86: Install <sys/platform/x86.h> [BZ #26124]

["H.J. Lu" <hjl.tools@gmail.com>]

On Mon, Jun 22, 2020 at 2:15 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > I changed the manual to
> >
> > @deftypefn Macro int HAS_CPU_FEATURE (@var{name})
> > This macro returns a nonzero value (true) if the processor has the feature
> > @var{name}.
> > @end deftypefn
> >
> > @deftypefn Macro int CPU_FEATURE_USABLE (@var{name})
> > This macro returns a nonzero value (true) if the processor feature
> > @var{name} is supported by the operating system.
>
> Does this mean that it's necessary to check both before using the
> feature?  This is what the description implies to me.

CPU_FEATURE_USABLE  is true only if HAS_CPU_FEATURE is true.

> If CPU_FEATURE_USABLE implies HAS_CPU_FEATURE (so it's not necessary to
> check both), then I don't see the use case for HAS_CPU_FEATURE.  To me,
> exposing both liks like a trap for programmers: they might check CPU
> support only, but not operating system support.  That's trap that we
> have fallen into with glibc itself at least once.

Since not all features need OS support, only a subset of features have both.
HAS_CPU_FEATURE is useful on its own.  For example, it can be used to
identify processors even if OS doesn't support the feature.  All the information
is readily available.  I just provide a macro with a stable ABI to access it.

>
> >> >> struct cpu_features (even in its reduced form) is fairly large.  We will
> >> >> never be able to reduce its size again if it becomes public ABI.
> >> >
> >> > Fixed by
> >> >
> >> > struct cpu_features
> >> > {
> >> >   struct cpu_features_basic basic;
> >> >   unsigned int *usable_p;
> >> >   struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
> >> > };
> >>
> >> I think the cpuid member is the fat part.  But the pointer indirection
> >> allows us to grow the *usable_p part without having to duplicate the
> >> backing storage for __x86_get_cpu_features, so it is an improvement.
> >>
> >> > __builtin_cpu_supports is equivalent to CPU_FEATURE_USABLE and it
> >> > doesn't support HAS_CPU_FEATURE which does provide useful information.
> >>
> >> I'm still puzzled as to why you aren't extending the existing function.
> >>
> >
> > I am working on it:
> >
> > https://gcc.gnu.org/pipermail/gcc-patches/2020-May/546522.html
> >
> > But it is very unlikely to support HAS_CPU_FEATURE and
> > <sys/platform/x86.h> works with all GCCs.
>
> On the other hand, it's easier for our users to upgrade GCC than to
> update glibc.

Not everyone needs/wants to upgrade GCC.

-- 
H.J.