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

[Andi Kleen]

On Thu, Oct 12, 2017 at 05:13:34PM +0200, Thomas Gleixner wrote:
> On Thu, 12 Oct 2017, Ingo Molnar wrote:
> > 
> > * Andi Kleen <andi@xxxxxxxxxxxxxx> 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