Re: [PATCH 13/15] x86/cpu: Make CONFIG_X86_CX8 unconditional

[Ingo Molnar <mingo@kernel.org>]

* Arnd Bergmann <arnd@kernel.org> wrote:

> >   M486
> >   M486SX
> >   M586
> >   M586MMX         # NEW
> >   M586TSC         # NEW
> >   MCYRIXIII       # NEW
> >   MELAN
> >   MGEODEGX1       # NEW
> >   MK6             # NEW
> >   MWINCHIP3D
> >   MWINCHIPC6
> 
> This would also mean dropping support for the pre-2015 Intel Quark
> and DM&P Vortex86DX/DX2/MX/EX that never had a custom CONFIG_Mxxxx
> option but are still relevant to some degree.
> I think that would be a mistake. 

Yeah, agreed, and especially with the <asm/bitops.h> CMOV complication 
removed per Linus's patch, we could actually remove CONFIG_X86_CMOV, as 
nothing uses it anymore:

  starship:~/mingo.tip.git> git grep X86_CMOV
  arch/x86/Kconfig.cpu:config X86_CMOV
  arch/x86/Kconfig.cpufeatures:   depends on X86_CMOV

The CMOV dependency comes in through compiler options only: 

  arch/x86/Makefile_32.cpu:cflags-$(CONFIG_M586TSC)	+= -march=i586
  arch/x86/Makefile_32.cpu:cflags-$(CONFIG_M586MMX)	+= -march=pentium-mmx
  arch/x86/Makefile_32.cpu:cflags-$(CONFIG_MCYRIXIII)	+= $(call cc-option,-march=c3,-march=i486) $(align)
  arch/x86/Makefile_32.cpu:cflags-$(CONFIG_MGEODEGX1)	+= -march=pentium-mmx
  arch/x86/Makefile_32.cpu:cflags-$(CONFIG_MGEODE_LX)	+= $(call cc-option,-march=geode,-march=pentium-mmx)

These build options will, indirectly, not include CMOV in the kernel 
image, while i686 and higher march options will.

(BTW., we should probably remove the -march=i486 fallback for 
MCYRIXIII, our minimum CC version is beyond that already I believe.)

Anyway, the current plan is to not drop common-i586, only the removal 
of what's in the -v1 series:

   M486
   M486SX
   M586
   MELAN
   MWINCHIP3D
   MWINCHIPC6

> Here are some alternatives I like better than dropping i586:
> 
> a) keep my patch with an new bool option to pick between
>    i586 and i686 targets, by any name.
> 
> b) always build with -march=i586 and leave only the -mtune
>    flags; see if anyone cares enough to even benchmark
>    and pick one of the other options if they can show
>    a meaningful regression over -march=i686 -mtune=

That's actually a good idea IMO. I looked at the code generation with 
current compilers and it turns out that M686 is *substantially* worse 
in code generation than M586, as apparently the extra CMOV instructions 
bloat up the generated code:

      text	   data	    bss	    dec	    hex	filename
  15427023	7601010	1744896	24772929	17a0141	vmlinux.M586
  16578295	7598826	1744896	25922017	18b89e1	vmlinux.M686

 - +7.5% increase in text size (5.6% according to bloatometer),
 - +2% increase in instruction count,
 - while number of branches increases by +1.3%.

But it's not about CMOV: I checked about a dozen functions that end up 
using CMOV, and the 'conditional' part of CMOV does seem to reduce 
branches for those functions by a minor degree and ends up reducing 
their size as well. So CMOV helps, a bit.

The substantial code bloat comes from some other aspect of GCC's 
march=i686 flag ... I bet it's primarily inlining: there's a 0.7% 
reduction in number of calls done.

I have a hard time believing that this kind of bloat and complexity 
helps performance to *any* degree.

I really didn't remember how bad it was, until I re-measured it.

CMOV is likely a drop in the ocean compared to this kind of text bloat. 
And yeah, it doesn't really matter that i686 class CPUs have larger 
caches, the kernel is dominantly cache-cold code execution, inlining 
driven bloat almost never helps performance.

> c) keep the outcome of your v1 series, dropping only
>    pre-i586 support, and leave my patch out. No change here,
>    so at least no regression potential.

Yeah, so this is roughly the current plan, with perhaps light touchups 
on top to make it easier to configure, and to remove residual legacies.

Thanks,

	Ingo