This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: V6 [PATCH] x86: Add <sys/platform/x86.h>


On Wed, Oct 31, 2018 at 7:38 PM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 10/26/18 1:16 AM, Florian Weimer wrote:
> > * H. J. Lu:
> >
> >> /* CPU_FEATURE_USABLE evaluates to true if the feature is usable.  */
> >>  #define CPU_FEATURE_USABLE(name)                                  \
> >>    ((need_arch_feature_##name                                      \
> >>      && (x86_get_arch_feature (index_arch_##name##_Usable)         \
> >>    & (bit_arch_##name##_Usable)) != 0)                             \
> >>     || (!need_arch_feature_##name                               \
> >>         && (x86_get_cpuid_registers (index_cpu_##name)->reg_##name \
> >>          & (bit_cpu_##name)) != 0))
> >
> > I still think this is wrong.  This should be a const function, with a
> > single argument, returning bool, so that we do not expose the internal
> > implementation.  It will also be possible to use this function from Ada,
> > Fortran, and so on.  This is not possible with a preprocessor macro.
>
> I think this is the key sticking point for consensus.
>
> We should iterate over this as a success criteria for that discussion.
>
> This isn't the first objection to macros.
>
> In July I also objected.
>
> https://sourceware.org/ml/libc-alpha/2018-07/msg01067.html
> ~~~
> > /* HAS_* evaluates to true if we may use the feature at runtime.  */
> > #define HAS_CPU_FEATURE(name) \
> >    ((__x86_get_cpuid_registers (index_cpu_##name)->reg_##name \
> >      & (bit_cpu_##name)) != 0)
>
> I don't like these as macros.
>
> I would rather an inline function that does all the work within the function
> and returns a result of the appropriate type e.g. boolean.
> ~~~
>
> Please expand on the benefits of this being a macro.
>
> We need to see these benefits to move from a position
> where we have sustained objections to a macro API, to one where
> we might object to the macros but understand that they are perhaps
> the best compromise given your expanded description.
>
> Is there a serious performance implication?
>

We have some programs, both inside glibc and outside glibc, which
check CPUID.  I'd like to switch them to <sys/platform/x86.h>.   This
interface:

enum
  {
    COMMON_CPUID_INDEX_1 = 0,
    COMMON_CPUID_INDEX_7,
    COMMON_CPUID_INDEX_80000001,
    /* Keep the following line at the end.  */
    COMMON_CPUID_INDEX_MAX
  };

struct cpuid_registers
  {
    unsigned int eax;
    unsigned int ebx;
    unsigned int ecx;
    unsigned int edx;
  };

extern const struct cpuid_registers *x86_get_cpuid_registers
     (unsigned int) __attribute__ ((const));

extern unsigned int x86_get_arch_feature (unsigned int)
     __attribute__ ((const));

/* CPU_FEATURE_USABLE evaluates to true if the feature is usable.  */
 #define CPU_FEATURE_USABLE(name)                                  \
   ((need_arch_feature_##name                                      \
     && (x86_get_arch_feature (index_arch_##name##_Usable)         \
   & (bit_arch_##name##_Usable)) != 0)                             \
    || (!need_arch_feature_##name                            \
        && (x86_get_cpuid_registers (index_cpu_##name)->reg_##name \
    & (bit_cpu_##name)) != 0))

makes a couple things possible:

1.  Multiple features can be checked with a single function call.
2.  CPUID instruction takes EAX and optionally ECX as input,  It writes to
EAX, EBX, ECD and EDX.  Each CPU feature is determined by CPUID with
specific input values and a bit in EAX, EBX, ECX or EDX.
CPU_FEATURE_USABLE macro generate very decent code.  There are many
reserved bits in EAX, EBX, ECX and EDX, which may be changed to new CPU
features for future processors.  It is easy to add these features to x86.h
with macros without recompiling glibc.

-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]