Re: [PATCH v9 2/5] x86/cpuid: Add generic table for cpuid dependencies

Andi Kleen
2017-10-12T22:01:38Z

On Thu, Oct 12, 2017 at 05:13:34PM +0200, Thomas Gleixner wrote:
> On Thu, 12 Oct 2017, Ingo Molnar wrote:
> >
> > * Andi Kleen <andi@firstfloor.org> wrote:
> >
> > > --- /dev/null
> > > +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> > > @@ -0,0 +1,109 @@
> > > +/* Declare dependencies between CPUIDs */
> > > +#include <linux/kernel.h>
> > > +#include <linux/init.h>
> > > +#include <linux/module.h>
> > > +#include <asm/cpufeature.h>
> > > +
> > > +struct cpuid_dep {
> > > + unsigned short feature;
> > > + unsigned short depends;
> > > +};
> >
> > Why are these 16-bit fields? 16-bit data types should be avoided as much as
> > possible, as they generate suboptimal code.
>
> I was looking at that as well and decided that we preferrably have a
> compressed data structure. The code which walks the table is hardly
> performance critical and the difference in text size is marginal.

Right it was an attempt to save a tiny bit of text space.
On modern x86 CPUs there is no penalty for 16bit except for slightly
larger code. The table is far bigger than the few additional 16bit prefixes
in the code

text data bss dec hex filename
436 0 0 436 1b4 arch/x86/kernel/cpu/cpuid-deps.o-short
572 0 0 572 23c arch/x86/kernel/cpu/cpuid-deps.o-int

But can convert to 4 bytes in the next version.

-Andi