Re: [RFC PATCH 1/2] powerpc/kbuild: Use flags variables rather than overriding LD/CC/AS

[Nicholas Piggin <npiggin@gmail.com>]

On Mon, 7 May 2018 14:25:12 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Hi.
> 
> 
> 2018-04-30 10:23 GMT+09:00 Nicholas Piggin <npiggin@gmail.com>:
> > The powerpc toolchain can compile combinations of 32/64 bit and
> > big/little endian, so it's convenient to consider, e.g.,
> >
> >   CC -m64 -mbig-endian
> >
> > To be the C compiler for the purpose of invoking it to build
> > target artifacts.  
> 
> Right, but this is not possible in the new Kconfig scheme
> because CPU_BIG/LITTLE_ENDIAN can be turned on/off
> from Kconfig menus.
> 
> 
> > Rather than override, use kbuild defined variables to pass these
> > flags around. Importantly, they must be passed to things like
> > cc-option, so the usual cflags-y is not sufficient. This multitude
> > of inconsistently named variables is a mess, but it's marginally
> > better than overriding the toolchain because it matches what other
> > architectures do.
> >
> > This allows powerpc builds to work with the new kconfig macro
> > language branch. XXX: not exactly sure why it fails in the first
> > place.  
> 
> 
> Without this patch, 'scripts/kconfig/conf  --syncconfig Kconfig'
> continues eternally.
> 
> This is because the change of environment variable $(CC) will
> trigger syncconfig.
> 
> So, this patch is the right thing to do.

Thanks for looking, yes I like it better. I'm still having some
trouble with 32-bit cross compilation with 64-bit toolchain. I'm
trying to work through this, may need some help though.

> > XXX: 32-bit builds with 64-bit toolchain gain some additional options
> > like -funit-at-a-time from cc-option. Unclear why that is. Build
> > appears to be correct otherwise.
> > ---
> >  arch/powerpc/Makefile | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > index 95813df90801..046b5dde9ff5 100644
> > --- a/arch/powerpc/Makefile
> > +++ b/arch/powerpc/Makefile
> > @@ -74,13 +74,15 @@ endif
> >  endif
> >
> >  ifeq ($(CONFIG_CPU_LITTLE_ENDIAN),y)
> > -override LD    += -EL
> > +KBUILD_CPPFLAGS        += -mlittle-endian  
> 
> 
> IMHO, I personally prefer
> 
> KBUILD_CFLAGS        += -mlittle-endian
> KBUILD_AFLAGS        += -mlittle-endian
> 
> like the current arch/powerpc/Makefile add the flag
> to cflags-y / aflags-y separately.

The problem I found is that we need this option to be invoked
with 'cc-option'. I know this is probably not the right way to
go about it though, but arm64 seems to have the same hack.

Any suggestions for how to solve that? Should the cc-option
take KBUILD_CFLAGS as well?


> Only the difference would be whether -mlittle-endian
> is passed to pre-processing the linker script, though.
> 
> 
> 
> 
> > +LDFLAGS                += -EL
> >  LDEMULATION    := lppc
> >  GNUTARGET      := powerpcle
> >  MULTIPLEWORD   := -mno-multiple
> >  KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect)
> >  else
> > -override LD    += -EB
> > +KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian)
> > +LDFLAGS                += -EB
> >  LDEMULATION    := ppc
> >  GNUTARGET      := powerpc
> >  MULTIPLEWORD   := -mmultiple
> > @@ -93,8 +95,6 @@ aflags-$(CONFIG_CPU_BIG_ENDIAN)               += $(call cc-option,-mabi=elfv1)
> >  aflags-$(CONFIG_CPU_LITTLE_ENDIAN)     += -mabi=elfv2
> >  endif
> >
> > -cflags-$(CONFIG_CPU_LITTLE_ENDIAN)     += -mlittle-endian
> > -cflags-$(CONFIG_CPU_BIG_ENDIAN)                += $(call cc-option,-mbig-endian)
> >  ifneq ($(cc-name),clang)
> >    cflags-$(CONFIG_CPU_LITTLE_ENDIAN)   += -mno-strict-align
> >  endif
> > @@ -103,9 +103,9 @@ aflags-$(CONFIG_CPU_BIG_ENDIAN)             += $(call cc-option,-mbig-endian)
> >  aflags-$(CONFIG_CPU_LITTLE_ENDIAN)     += -mlittle-endian
> >
> >  ifeq ($(HAS_BIARCH),y)
> > -override AS    += -a$(BITS)
> > -override LD    += -m elf$(BITS)$(LDEMULATION)
> > -override CC    += -m$(BITS)
> > +KBUILD_CPPFLAGS        += -m$(BITS)  
> 
> 
> Do you mean this?
> 
> KBUILD_CFLAGS        += -m$(BITS)

IIRC it was the same cc-option problem.

> > +KBUILD_AFLAGS  += -m$(BITS) -Wl,-a$(BITS)  
> 
> 
> Both KBUILD_CPPFLAGS and KBUILD_AFLAGS are added
> to orig_a_flags in scripts/Makefile.lib
> 
> So, -m$(BITS) will be doubled for *.S files.

Yes, I haven't got things quite right yet.

Thanks,
Nick