Re: [RFC PATCH 3/4] x86/asm: Make alternative macro interfaces more clear and consistent

[Josh Poimboeuf]

On Fri, Sep 15, 2017 at 07:53:46PM +0300, Andrey Ryabinin wrote:
> 
> 
> On 08/31/2017 08:25 PM, Josh Poimboeuf wrote:
> >
> > There have been a few other ideas which have *almost* worked:
> > 
> > 1) Make the 'register void *__sp asm(_ASM_SP)' a global variable instead
> >    of a local one.  This works for GCC and doesn't break clang.  However
> >    it resulted in a lot of changed code on the GCC side.  It looked like
> >    some optimizations had been disabled, even in functions which
> >    shouldn't have been affected.
> > 
> 
> (For those who wasn't following previous discussion there is the patch - http://lkml.kernel.org/r/<75850bb7-a60e-057d-d88b-afa0c79e94a0@xxxxxxxxx>
> But, Josh discovered that the patch causes regression in .text size with gcc > 7.x
> http://lkml.kernel.org/r/<20170721132452.ihpws67e3e7ym3al@treble> )
> 
> I'm not so sure that this is disabled optimization. I assume that global rsp makes
> changes something in gcc's register allocation logic, or something like that leading
> to subtle changes in generated code.
> 
> But what I recently find out, is that this "regression" sometimes is actually improvement in .text size.
> It all depends on .config, e.g:
> 
> 
> allnoconfig:
>                text    data     bss     dec     hex filename
> patched	       938094  207700 1215752 2361546  2408ca allnoconfig_p/vmlinux
> unpatched      938768  211796 1215752 2366316  241b6c allnoconfig/vmlinux                                                                                                                                                         
> 
> 
> defconfig:
>                text    data     bss     dec     hex filename
> patched	       10691469        4882856  876544 16450869         fb0535 defconfig_p/vmlinux
> unpatched      10691035        4882856  876544 16450435         fb0383 defconfig/vmlinux
> 
> allyesconfig:
>                text    data     bss     dec     hex filename
> patched	       159216239       153294411       49692672        362203322       1596c8ba        allyesconfig_p/vmlinux
> unpatched      159224951       153294387       49692672        362212010       1596eaaa        allyesconfig/vmlinux
> 
> 
> So there is regression with Josh's config and defconfig, but allnoconfig and allyesconfig shows improvement.
> Note that sometimes there is the difference in .data size too, don't ask me why.
> 
> Given that, perhaps the best thing to do here would be asking gcc devs why is generated code changed.
> And if such changes are harmless (as I suspect) we can just fix the problem with this simple patch http://lkml.kernel.org/r/<75850bb7-a60e-057d-d88b-afa0c79e94a0@xxxxxxxxx> ?
> Despite the increased .text on some configs.

Can you change all the other code sites in a similar way, and then run
the numbers again?

  $ git grep __sp |grep register |grep void
  arch/x86/include/asm/alternative.h:	register void *__sp asm(_ASM_SP);				      \
  arch/x86/include/asm/mshyperv.h:	register void *__sp asm(_ASM_SP);
  arch/x86/include/asm/mshyperv.h:	register void *__sp asm(_ASM_SP);
  arch/x86/include/asm/paravirt_types.h:	register void *__sp asm("esp")
  arch/x86/include/asm/paravirt_types.h:	register void *__sp asm("rsp")
  arch/x86/include/asm/preempt.h:	register void *__sp asm(_ASM_SP);			\
  arch/x86/include/asm/preempt.h:	register void *__sp asm(_ASM_SP);				\
  arch/x86/include/asm/processor.h:	register void *__sp asm(_ASM_SP);
  arch/x86/include/asm/rwsem.h:	register void *__sp asm(_ASM_SP);		\
  arch/x86/include/asm/uaccess.h:	register void *__sp asm(_ASM_SP);				\
  arch/x86/include/asm/xen/hypercall.h:	register void *__sp asm(_ASM_SP);
  arch/x86/kvm/emulate.c:	register void *__sp asm(_ASM_SP);
  arch/x86/kvm/vmx.c:	register void *__sp asm(_ASM_SP);
  arch/x86/mm/fault.c:		register void *__sp asm("rsp");

-- 
Josh