* [PATCH] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile @ 2017-11-02 21:26 Nick Desaulniers 2017-11-04 0:37 ` Matthias Kaehlcke 2017-11-05 3:06 ` Masahiro Yamada 0 siblings, 2 replies; 25+ messages in thread From: Nick Desaulniers @ 2017-11-02 21:26 UTC (permalink / raw) To: Masahiro Yamada Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Chris Fries, Nick Desaulniers, Michal Marek, linux-kbuild, linux-kernel From: Chris Fries <cfries@google.com> Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, so that ld-options (etc.) can work correctly. This fixes errors with clang such as ld-options trying to CC against your host architecture, but LD trying to link against your target architecture. We didn't notice this problem on Android, because we took the original LLVMLinux patch into our 4.4 kernels, which did not have this issue. We ran into this taking the proper upstream patch on newer kernel versions. The original LLVMLinux patch can be seen at: http://git.linuxfoundation.org/?p=llvmlinux/kernel.git;a=blobdiff;f=Makefile;h=389006c4ef494cda3a1ee52bf355618673ab4f31;hp=e41a3356abee83f08288362950bfceebd25ec3c2;hb=ef9126da11b18ff34eb1f01561f53c378860336c;hpb=f800c25b7a762d445ba1439a2428c8362157eba6 It seems that when the patch was re-upstreamed, a V2 was requested that moved the definition of Clang's target triple to be later in the top level Makefile than the inclusion of the arch specific Makefile, breaking macros like ld-option when cross compiling. V2 was requested at: https://lkml.org/lkml/2017/4/21/116 Signed-off-by: Chris Fries <cfries@google.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Makefile | 64 ++++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/Makefile b/Makefile index 5f91a28a3cea..72ea86157114 100644 --- a/Makefile +++ b/Makefile @@ -512,6 +512,38 @@ ifneq ($(filter install,$(MAKECMDGOALS)),) endif endif +ifeq ($(cc-name),clang) +ifneq ($(CROSS_COMPILE),) +CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) +endif +ifneq ($(GCC_TOOLCHAIN),) +CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) +endif +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) +# Quiet clang warning: comparison of unsigned expression < 0 is always false +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the +# source of a reference will be _MergedGlobals and not on of the whitelisted names. +# See modpost pattern 2 +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) +else + +# These warnings generated too much noise in a regular build. +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) +endif + ifeq ($(mixed-targets),1) # =========================================================================== # We're called with mixed targets (*config and build targets). @@ -695,38 +727,6 @@ ifdef CONFIG_CC_STACKPROTECTOR endif KBUILD_CFLAGS += $(stackp-flag) -ifeq ($(cc-name),clang) -ifneq ($(CROSS_COMPILE),) -CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) -endif -ifneq ($(GCC_TOOLCHAIN),) -CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) -endif -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) -KBUILD_CFLAGS += $(call cc-disable-warning, gnu) -KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) -# Quiet clang warning: comparison of unsigned expression < 0 is always false -KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) -# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the -# source of a reference will be _MergedGlobals and not on of the whitelisted names. -# See modpost pattern 2 -KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) -KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) -else - -# These warnings generated too much noise in a regular build. -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) -endif - ifdef CONFIG_FRAME_POINTER KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls else -- 2.15.0.403.gc27cc4dac6-goog ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-02 21:26 [PATCH] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile Nick Desaulniers @ 2017-11-04 0:37 ` Matthias Kaehlcke 2017-11-05 3:06 ` Masahiro Yamada 1 sibling, 0 replies; 25+ messages in thread From: Matthias Kaehlcke @ 2017-11-04 0:37 UTC (permalink / raw) To: Nick Desaulniers Cc: Masahiro Yamada, Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Chris Fries, Michal Marek, linux-kbuild, linux-kernel El Thu, Nov 02, 2017 at 02:26:48PM -0700 Nick Desaulniers ha dit: > From: Chris Fries <cfries@google.com> > > Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, > so that ld-options (etc.) can work correctly. > > This fixes errors with clang such as ld-options trying to CC > against your host architecture, but LD trying to link against > your target architecture. > > We didn't notice this problem on Android, because we took the original > LLVMLinux patch into our 4.4 kernels, which did not have this issue. We > ran into this taking the proper upstream patch on newer kernel versions. > The original LLVMLinux patch can be seen at: > > http://git.linuxfoundation.org/?p=llvmlinux/kernel.git;a=blobdiff;f=Makefile;h=389006c4ef494cda3a1ee52bf355618673ab4f31;hp=e41a3356abee83f08288362950bfceebd25ec3c2;hb=ef9126da11b18ff34eb1f01561f53c378860336c;hpb=f800c25b7a762d445ba1439a2428c8362157eba6 > > It seems that when the patch was re-upstreamed, a V2 was requested that > moved the definition of Clang's target triple to be later in the top > level Makefile than the inclusion of the arch specific Makefile, > breaking macros like ld-option when cross compiling. V2 was requested > at: > > https://lkml.org/lkml/2017/4/21/116 > > Signed-off-by: Chris Fries <cfries@google.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Makefile | 64 ++++++++++++++++++++++++++++++++-------------------------------- > 1 file changed, 32 insertions(+), 32 deletions(-) > > diff --git a/Makefile b/Makefile > index 5f91a28a3cea..72ea86157114 100644 > --- a/Makefile > +++ b/Makefile > @@ -512,6 +512,38 @@ ifneq ($(filter install,$(MAKECMDGOALS)),) > endif > endif > > +ifeq ($(cc-name),clang) > +ifneq ($(CROSS_COMPILE),) > +CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) > +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > +endif > +ifneq ($(GCC_TOOLCHAIN),) > +CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) > +endif > +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) > +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) > +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > +# Quiet clang warning: comparison of unsigned expression < 0 is always false > +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) > +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the > +# source of a reference will be _MergedGlobals and not on of the whitelisted names. > +# See modpost pattern 2 > +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) > +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) > +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > +else > + > +# These warnings generated too much noise in a regular build. > +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > +endif > + > ifeq ($(mixed-targets),1) > # =========================================================================== > # We're called with mixed targets (*config and build targets). > @@ -695,38 +727,6 @@ ifdef CONFIG_CC_STACKPROTECTOR > endif > KBUILD_CFLAGS += $(stackp-flag) > > -ifeq ($(cc-name),clang) > -ifneq ($(CROSS_COMPILE),) > -CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) > -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > -endif > -ifneq ($(GCC_TOOLCHAIN),) > -CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) > -endif > -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > -KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) > -KBUILD_CFLAGS += $(call cc-disable-warning, gnu) > -KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > -# Quiet clang warning: comparison of unsigned expression < 0 is always false > -KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) > -# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the > -# source of a reference will be _MergedGlobals and not on of the whitelisted names. > -# See modpost pattern 2 > -KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) > -KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) > -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > -else > - > -# These warnings generated too much noise in a regular build. > -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > -endif > - > ifdef CONFIG_FRAME_POINTER > KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls > else FWIW Tested-by: Matthias Kaehlcke <mka@chromium.org> Reviewed-by: Matthias Kaehlcke <mka@chromium.org> On the Chrome OS side we missed this because by default our kernel builds use an architecture aware wrapper instead of bare clang, which masks this issue. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-02 21:26 [PATCH] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile Nick Desaulniers 2017-11-04 0:37 ` Matthias Kaehlcke @ 2017-11-05 3:06 ` Masahiro Yamada 2017-11-07 17:37 ` Nick Desaulniers 1 sibling, 1 reply; 25+ messages in thread From: Masahiro Yamada @ 2017-11-05 3:06 UTC (permalink / raw) To: Nick Desaulniers Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List 2017-11-03 6:26 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > From: Chris Fries <cfries@google.com> > > Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, > so that ld-options (etc.) can work correctly. ld-option is only used for arch/{arm64,powerpc}/Makefile arch/arm64/Makefile: ifeq ($(call ld-option, --fix-cortex-a53-843419),) arch/powerpc/Makefile:LDFLAGS_vmlinux += $(call ld-option,--orphan-handling=warn) I think this patch makes sense when it comes along with https://patchwork.kernel.org/patch/10030581/ but, it is now being blocked by 0-day bot due to a x86 problem. > This fixes errors with clang such as ld-options trying to CC > against your host architecture, but LD trying to link against > your target architecture. > > We didn't notice this problem on Android, because we took the original > LLVMLinux patch into our 4.4 kernels, which did not have this issue. We > ran into this taking the proper upstream patch on newer kernel versions. > The original LLVMLinux patch can be seen at: > > http://git.linuxfoundation.org/?p=llvmlinux/kernel.git;a=blobdiff;f=Makefile;h=389006c4ef494cda3a1ee52bf355618673ab4f31;hp=e41a3356abee83f08288362950bfceebd25ec3c2;hb=ef9126da11b18ff34eb1f01561f53c378860336c;hpb=f800c25b7a762d445ba1439a2428c8362157eba6 > > It seems that when the patch was re-upstreamed, a V2 was requested that > moved the definition of Clang's target triple to be later in the top > level Makefile than the inclusion of the arch specific Makefile, > breaking macros like ld-option when cross compiling. V2 was requested > at: But, ld-option is defines as follows in llvm-linux tree (and mainline too): ld-option = $(call try-run,\ $(CC) -x c /dev/null -c -o "$$TMPO" ; $(LD) $(1) "$$TMPO" -o "$$TMP",$(1),$(2)) ld-option does not depend on any pre-defined flags. The location of CLANG_GCC_TC define only matters after your patch is applied, right? Did my request for v2 break anything? One more thing: this patch does not apply to kbuild tree. > https://lkml.org/lkml/2017/4/21/116 > > Signed-off-by: Chris Fries <cfries@google.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Makefile | 64 ++++++++++++++++++++++++++++++++-------------------------------- > 1 file changed, 32 insertions(+), 32 deletions(-) > > diff --git a/Makefile b/Makefile > index 5f91a28a3cea..72ea86157114 100644 > --- a/Makefile > +++ b/Makefile > @@ -512,6 +512,38 @@ ifneq ($(filter install,$(MAKECMDGOALS)),) > endif > endif > > +ifeq ($(cc-name),clang) > +ifneq ($(CROSS_COMPILE),) > +CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) > +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > +endif > +ifneq ($(GCC_TOOLCHAIN),) > +CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) > +endif > +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) > +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) > +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > +# Quiet clang warning: comparison of unsigned expression < 0 is always false > +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) > +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the > +# source of a reference will be _MergedGlobals and not on of the whitelisted names. > +# See modpost pattern 2 > +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) > +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) > +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > +else > + > +# These warnings generated too much noise in a regular build. > +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > +endif > + > ifeq ($(mixed-targets),1) > # =========================================================================== > # We're called with mixed targets (*config and build targets). > @@ -695,38 +727,6 @@ ifdef CONFIG_CC_STACKPROTECTOR > endif > KBUILD_CFLAGS += $(stackp-flag) > > -ifeq ($(cc-name),clang) > -ifneq ($(CROSS_COMPILE),) > -CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) > -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > -endif > -ifneq ($(GCC_TOOLCHAIN),) > -CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) > -endif > -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > -KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) > -KBUILD_CFLAGS += $(call cc-disable-warning, gnu) > -KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > -# Quiet clang warning: comparison of unsigned expression < 0 is always false > -KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) > -# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the > -# source of a reference will be _MergedGlobals and not on of the whitelisted names. > -# See modpost pattern 2 > -KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) > -KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) > -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > -else > - > -# These warnings generated too much noise in a regular build. > -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > -endif > - > ifdef CONFIG_FRAME_POINTER > KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls > else > -- > 2.15.0.403.gc27cc4dac6-goog > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-05 3:06 ` Masahiro Yamada @ 2017-11-07 17:37 ` Nick Desaulniers 2017-11-07 19:46 ` Nick Desaulniers 2017-11-09 4:15 ` [PATCH] " Masahiro Yamada 0 siblings, 2 replies; 25+ messages in thread From: Nick Desaulniers @ 2017-11-07 17:37 UTC (permalink / raw) To: Masahiro Yamada Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List On Sat, Nov 4, 2017 at 8:06 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > ld-option is only used for arch/{arm64,powerpc}/Makefile > > arch/arm64/Makefile: ifeq ($(call ld-option, --fix-cortex-a53-843419),) > arch/powerpc/Makefile:LDFLAGS_vmlinux += $(call > ld-option,--orphan-handling=warn) > > I think this patch makes sense when it comes along with > https://patchwork.kernel.org/patch/10030581/ Good point. > but, it is now being blocked by 0-day bot > due to a x86 problem. Looks like that is now resolved (unless 0-day bot strikes again). > The location of CLANG_GCC_TC define > only matters after your patch is applied, right? By "your patch" referring to the 0-day bot thread, yes. > Did my request for v2 break anything? Nothing immediately obvious, and no regressions. It just made this patch necessary (along with my previous one) for correctly cross compiling with clang for arm64 and powerpc as you point out. > One more thing: this patch does not apply to kbuild tree. I absolutely will rebase it on your tree and send a v2. Just to help me understand the contribution model better: none of my other patches have yet been requested against any trees other than Linus'. Is this because of where we are in the release cycle, or that a lot of kbuild code has changed, or what? ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-07 17:37 ` Nick Desaulniers @ 2017-11-07 19:46 ` Nick Desaulniers 2017-11-09 4:15 ` [PATCH] " Masahiro Yamada 1 sibling, 0 replies; 25+ messages in thread From: Nick Desaulniers @ 2017-11-07 19:46 UTC (permalink / raw) Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Chris Fries, Nick Desaulniers, Masahiro Yamada, Michal Marek, linux-kbuild, linux-kernel From: Chris Fries <cfries@google.com> Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, so that ld-options (etc.) can work correctly. This fixes errors with clang such as ld-options trying to CC against your host architecture, but LD trying to link against your target architecture. We didn't notice this problem on Android, because we took the original LLVMLinux patch into our 4.4 kernels, which did not have this issue. We ran into this taking the proper upstream patch on newer kernel versions. The original LLVMLinux patch can be seen at: http://git.linuxfoundation.org/?p=llvmlinux/kernel.git;a=blobdiff;f=Makefile;h=389006c4ef494cda3a1ee52bf355618673ab4f31;hp=e41a3356abee83f08288362950bfceebd25ec3c2;hb=ef9126da11b18ff34eb1f01561f53c378860336c;hpb=f800c25b7a762d445ba1439a2428c8362157eba6 It seems that when the patch was re-upstreamed, a V2 was requested that moved the definition of Clang's target triple to be later in the top level Makefile than the inclusion of the arch specific Makefile, breaking macros like ld-option when cross compiling. V2 was requested at: https://lkml.org/lkml/2017/4/21/116 Reviewed-by: Matthias Kaehlcke <mka@chromium.org> Signed-off-by: Chris Fries <cfries@google.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Tested-by: Matthias Kaehlcke <mka@chromium.org> --- Changes since v1: * Rebase on kbuild tree, as per Masahiro. * Add mka's RB/TB line to commit message. Makefile | 64 ++++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/Makefile b/Makefile index a7476e6934f1..d349734c7ef7 100644 --- a/Makefile +++ b/Makefile @@ -472,6 +472,38 @@ ifneq ($(KBUILD_SRC),) $(srctree) $(objtree) $(VERSION) $(PATCHLEVEL) endif +ifeq ($(cc-name),clang) +ifneq ($(CROSS_COMPILE),) +CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) +endif +ifneq ($(GCC_TOOLCHAIN),) +CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) +endif +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) +# Quiet clang warning: comparison of unsigned expression < 0 is always false +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the +# source of a reference will be _MergedGlobals and not on of the whitelisted names. +# See modpost pattern 2 +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) +else + +# These warnings generated too much noise in a regular build. +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) +endif + ifeq ($(config-targets),1) # =========================================================================== # *config targets only - make sure prerequisites are updated, and descend @@ -682,38 +714,6 @@ ifdef CONFIG_CC_STACKPROTECTOR endif KBUILD_CFLAGS += $(stackp-flag) -ifeq ($(cc-name),clang) -ifneq ($(CROSS_COMPILE),) -CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) -endif -ifneq ($(GCC_TOOLCHAIN),) -CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) -endif -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) -KBUILD_CFLAGS += $(call cc-disable-warning, gnu) -KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) -# Quiet clang warning: comparison of unsigned expression < 0 is always false -KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) -# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the -# source of a reference will be _MergedGlobals and not on of the whitelisted names. -# See modpost pattern 2 -KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) -KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) -else - -# These warnings generated too much noise in a regular build. -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) -endif - ifdef CONFIG_FRAME_POINTER KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls else -- 2.15.0.403.gc27cc4dac6-goog ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile @ 2017-11-07 19:46 ` Nick Desaulniers 0 siblings, 0 replies; 25+ messages in thread From: Nick Desaulniers @ 2017-11-07 19:46 UTC (permalink / raw) Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Chris Fries, Nick Desaulniers, Masahiro Yamada, Michal Marek, linux-kbuild, linux-kernel From: Chris Fries <cfries@google.com> Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, so that ld-options (etc.) can work correctly. This fixes errors with clang such as ld-options trying to CC against your host architecture, but LD trying to link against your target architecture. We didn't notice this problem on Android, because we took the original LLVMLinux patch into our 4.4 kernels, which did not have this issue. We ran into this taking the proper upstream patch on newer kernel versions. The original LLVMLinux patch can be seen at: http://git.linuxfoundation.org/?p=llvmlinux/kernel.git;a=blobdiff;f=Makefile;h=389006c4ef494cda3a1ee52bf355618673ab4f31;hp=e41a3356abee83f08288362950bfceebd25ec3c2;hb=ef9126da11b18ff34eb1f01561f53c378860336c;hpb=f800c25b7a762d445ba1439a2428c8362157eba6 It seems that when the patch was re-upstreamed, a V2 was requested that moved the definition of Clang's target triple to be later in the top level Makefile than the inclusion of the arch specific Makefile, breaking macros like ld-option when cross compiling. V2 was requested at: https://lkml.org/lkml/2017/4/21/116 Reviewed-by: Matthias Kaehlcke <mka@chromium.org> Signed-off-by: Chris Fries <cfries@google.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Tested-by: Matthias Kaehlcke <mka@chromium.org> --- Changes since v1: * Rebase on kbuild tree, as per Masahiro. * Add mka's RB/TB line to commit message. Makefile | 64 ++++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/Makefile b/Makefile index a7476e6934f1..d349734c7ef7 100644 --- a/Makefile +++ b/Makefile @@ -472,6 +472,38 @@ ifneq ($(KBUILD_SRC),) $(srctree) $(objtree) $(VERSION) $(PATCHLEVEL) endif +ifeq ($(cc-name),clang) +ifneq ($(CROSS_COMPILE),) +CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) +endif +ifneq ($(GCC_TOOLCHAIN),) +CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) +endif +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) +# Quiet clang warning: comparison of unsigned expression < 0 is always false +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the +# source of a reference will be _MergedGlobals and not on of the whitelisted names. +# See modpost pattern 2 +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) +else + +# These warnings generated too much noise in a regular build. +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) +endif + ifeq ($(config-targets),1) # =========================================================================== # *config targets only - make sure prerequisites are updated, and descend @@ -682,38 +714,6 @@ ifdef CONFIG_CC_STACKPROTECTOR endif KBUILD_CFLAGS += $(stackp-flag) -ifeq ($(cc-name),clang) -ifneq ($(CROSS_COMPILE),) -CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) -endif -ifneq ($(GCC_TOOLCHAIN),) -CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) -endif -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) -KBUILD_CFLAGS += $(call cc-disable-warning, gnu) -KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) -# Quiet clang warning: comparison of unsigned expression < 0 is always false -KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) -# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the -# source of a reference will be _MergedGlobals and not on of the whitelisted names. -# See modpost pattern 2 -KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) -KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) -else - -# These warnings generated too much noise in a regular build. -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) -endif - ifdef CONFIG_FRAME_POINTER KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls else -- 2.15.0.403.gc27cc4dac6-goog ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-07 19:46 ` Nick Desaulniers (?) @ 2017-11-09 4:31 ` Masahiro Yamada 2017-11-09 16:51 ` Nick Desaulniers -1 siblings, 1 reply; 25+ messages in thread From: Masahiro Yamada @ 2017-11-09 4:31 UTC (permalink / raw) To: Nick Desaulniers Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List Hi Nick 2017-11-08 4:46 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > From: Chris Fries <cfries@google.com> > > Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, > so that ld-options (etc.) can work correctly. > > This fixes errors with clang such as ld-options trying to CC > against your host architecture, but LD trying to link against > your target architecture. OK. Your previous patch was applied (and we have not received 0-day report so far), so this makes sense now. > We didn't notice this problem on Android, because we took the original > LLVMLinux patch into our 4.4 kernels, which did not have this issue. We > ran into this taking the proper upstream patch on newer kernel versions. > The original LLVMLinux patch can be seen at: > > http://git.linuxfoundation.org/?p=llvmlinux/kernel.git;a=blobdiff;f=Makefile;h=389006c4ef494cda3a1ee52bf355618673ab4f31;hp=e41a3356abee83f08288362950bfceebd25ec3c2;hb=ef9126da11b18ff34eb1f01561f53c378860336c;hpb=f800c25b7a762d445ba1439a2428c8362157eba6 > > It seems that when the patch was re-upstreamed, a V2 was requested that > moved the definition of Clang's target triple to be later in the top > level Makefile than the inclusion of the arch specific Makefile, > breaking macros like ld-option when cross compiling. V2 was requested > at: > > https://lkml.org/lkml/2017/4/21/116 IMO, this description is not helpful in any way in upstream. Moreover, 785f11aa595bc3d4e74096cbd598ada54ecc0d81 did not break anything. It sound unfair to me. > diff --git a/Makefile b/Makefile > index a7476e6934f1..d349734c7ef7 100644 > --- a/Makefile > +++ b/Makefile > @@ -472,6 +472,38 @@ ifneq ($(KBUILD_SRC),) > $(srctree) $(objtree) $(VERSION) $(PATCHLEVEL) > endif > > +ifeq ($(cc-name),clang) > +ifneq ($(CROSS_COMPILE),) > +CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) > +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > +endif > +ifneq ($(GCC_TOOLCHAIN),) > +CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) > +endif > +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) > +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) > +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > +# Quiet clang warning: comparison of unsigned expression < 0 is always false > +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) > +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the > +# source of a reference will be _MergedGlobals and not on of the whitelisted names. > +# See modpost pattern 2 > +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) > +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) > +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > +else > + > +# These warnings generated too much noise in a regular build. > +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > +endif > + Could you move this a bit later, please? # These warnings generated too much noise in a regular build. # Use make W=1 to enable them (see scripts/Makefile.extrawarn) KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) endif << INSERT HERE >> # The arch Makefile can set ARCH_{CPP,A,C}FLAGS to override the default # values of the respective KBUILD_* variables ARCH_CPPFLAGS := ARCH_AFLAGS := ARCH_CFLAGS := include arch/$(SRCARCH)/Makefile arch/$(SRCARCH)/Makefile is included for config targets for KBUILD_DEFCONFIG, but no reason to compute compiler flags. I want to cut unnecessary cc-option parsing. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-09 4:31 ` Masahiro Yamada @ 2017-11-09 16:51 ` Nick Desaulniers 2017-11-10 2:52 ` Masahiro Yamada 0 siblings, 1 reply; 25+ messages in thread From: Nick Desaulniers @ 2017-11-09 16:51 UTC (permalink / raw) To: Masahiro Yamada Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List On Wed, Nov 8, 2017 at 8:31 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2017-11-08 4:46 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: >> We didn't notice this problem on Android, because we took the original >> LLVMLinux patch into our 4.4 kernels, which did not have this issue. We >> ran into this taking the proper upstream patch on newer kernel versions. >> The original LLVMLinux patch can be seen at: >> >> http://git.linuxfoundation.org/?p=llvmlinux/kernel.git;a=blobdiff;f=Makefile;h=389006c4ef494cda3a1ee52bf355618673ab4f31;hp=e41a3356abee83f08288362950bfceebd25ec3c2;hb=ef9126da11b18ff34eb1f01561f53c378860336c;hpb=f800c25b7a762d445ba1439a2428c8362157eba6 >> >> It seems that when the patch was re-upstreamed, a V2 was requested that >> moved the definition of Clang's target triple to be later in the top >> level Makefile than the inclusion of the arch specific Makefile, >> breaking macros like ld-option when cross compiling. V2 was requested >> at: >> >> https://lkml.org/lkml/2017/4/21/116 > > IMO, this description is not helpful in any way in upstream. > > Moreover, > 785f11aa595bc3d4e74096cbd598ada54ecc0d81 > did not break anything. It sound unfair to me. Ok, I will cut that part description on v3. >> diff --git a/Makefile b/Makefile >> index a7476e6934f1..d349734c7ef7 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -472,6 +472,38 @@ ifneq ($(KBUILD_SRC),) >> $(srctree) $(objtree) $(VERSION) $(PATCHLEVEL) >> endif >> >> +ifeq ($(cc-name),clang) >> +ifneq ($(CROSS_COMPILE),) >> +CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) >> +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) >> +endif >> +ifneq ($(GCC_TOOLCHAIN),) >> +CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) >> +endif >> +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) >> +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) >> +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) >> +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) >> +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) >> +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) >> +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) >> +# Quiet clang warning: comparison of unsigned expression < 0 is always false >> +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) >> +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the >> +# source of a reference will be _MergedGlobals and not on of the whitelisted names. >> +# See modpost pattern 2 >> +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) >> +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) >> +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) >> +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) >> +else >> + >> +# These warnings generated too much noise in a regular build. >> +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) >> +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) >> +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) >> +endif >> + > > > Could you move this a bit later, please? > > > > # These warnings generated too much noise in a regular build. > # Use make W=1 to enable them (see scripts/Makefile.extrawarn) > KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > endif > > << INSERT HERE >> > > # The arch Makefile can set ARCH_{CPP,A,C}FLAGS to override the default > # values of the respective KBUILD_* variables > ARCH_CPPFLAGS := > ARCH_AFLAGS := > ARCH_CFLAGS := > include arch/$(SRCARCH)/Makefile > > > > > > > arch/$(SRCARCH)/Makefile is included for config targets > for KBUILD_DEFCONFIG, but no reason to compute compiler flags. > I want to cut unnecessary cc-option parsing. With the new try-run-cached macros, if the arch/$(SRCARCH)/Makefile gets included twice, with the compiler flags not set correctly for clang to cross compile, and the results are cached, wont they be wrong the second time the arch specific Makefile is included? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-09 16:51 ` Nick Desaulniers @ 2017-11-10 2:52 ` Masahiro Yamada 2017-11-15 20:42 ` Nick Desaulniers 0 siblings, 1 reply; 25+ messages in thread From: Masahiro Yamada @ 2017-11-10 2:52 UTC (permalink / raw) To: Nick Desaulniers Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List Hi Nick, 2017-11-10 1:51 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > On Wed, Nov 8, 2017 at 8:31 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> 2017-11-08 4:46 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: >>> We didn't notice this problem on Android, because we took the original >>> LLVMLinux patch into our 4.4 kernels, which did not have this issue. We >>> ran into this taking the proper upstream patch on newer kernel versions. >>> The original LLVMLinux patch can be seen at: >>> >>> http://git.linuxfoundation.org/?p=llvmlinux/kernel.git;a=blobdiff;f=Makefile;h=389006c4ef494cda3a1ee52bf355618673ab4f31;hp=e41a3356abee83f08288362950bfceebd25ec3c2;hb=ef9126da11b18ff34eb1f01561f53c378860336c;hpb=f800c25b7a762d445ba1439a2428c8362157eba6 >>> >>> It seems that when the patch was re-upstreamed, a V2 was requested that >>> moved the definition of Clang's target triple to be later in the top >>> level Makefile than the inclusion of the arch specific Makefile, >>> breaking macros like ld-option when cross compiling. V2 was requested >>> at: >>> >>> https://lkml.org/lkml/2017/4/21/116 >> >> IMO, this description is not helpful in any way in upstream. >> >> Moreover, >> 785f11aa595bc3d4e74096cbd598ada54ecc0d81 >> did not break anything. It sound unfair to me. > > Ok, I will cut that part description on v3. > >>> diff --git a/Makefile b/Makefile >>> index a7476e6934f1..d349734c7ef7 100644 >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -472,6 +472,38 @@ ifneq ($(KBUILD_SRC),) >>> $(srctree) $(objtree) $(VERSION) $(PATCHLEVEL) >>> endif >>> >>> +ifeq ($(cc-name),clang) >>> +ifneq ($(CROSS_COMPILE),) >>> +CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) >>> +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) >>> +endif >>> +ifneq ($(GCC_TOOLCHAIN),) >>> +CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) >>> +endif >>> +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) >>> +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) >>> +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) >>> +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) >>> +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) >>> +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) >>> +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) >>> +# Quiet clang warning: comparison of unsigned expression < 0 is always false >>> +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) >>> +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the >>> +# source of a reference will be _MergedGlobals and not on of the whitelisted names. >>> +# See modpost pattern 2 >>> +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) >>> +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) >>> +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) >>> +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) >>> +else >>> + >>> +# These warnings generated too much noise in a regular build. >>> +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) >>> +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) >>> +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) >>> +endif >>> + >> >> >> Could you move this a bit later, please? >> >> >> >> # These warnings generated too much noise in a regular build. >> # Use make W=1 to enable them (see scripts/Makefile.extrawarn) >> KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) >> KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) >> endif >> >> << INSERT HERE >> >> >> # The arch Makefile can set ARCH_{CPP,A,C}FLAGS to override the default >> # values of the respective KBUILD_* variables >> ARCH_CPPFLAGS := >> ARCH_AFLAGS := >> ARCH_CFLAGS := >> include arch/$(SRCARCH)/Makefile >> >> >> >> >> >> >> arch/$(SRCARCH)/Makefile is included for config targets >> for KBUILD_DEFCONFIG, but no reason to compute compiler flags. >> I want to cut unnecessary cc-option parsing. > > With the new try-run-cached macros, if the arch/$(SRCARCH)/Makefile > gets included twice, with the compiler flags not set correctly for > clang to cross compile, and the results are cached, wont they be wrong > the second time the arch specific Makefile is included? > -- Good point. The cached data from arch/$(SRCARCH)/Makefile for configuration is not used for the second run. That means some garbage data in the cache file, but less than 10, I think. You do not need to give CC or CROSS_COMPILE for kernel configuration in the first place. So, make ARCH=arm64 defconfig make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- should be fine. Even if we place lots of clang's cc-option earlier, users may not give the same CC for configuration and building. I think more optimized way is to skip computing cc-option for "make *config", "make clean" etc. I tried to do that. https://patchwork.kernel.org/patch/9983827/ I decided to take time for cleaner implementation, but that is what I'd like to achieve in the future. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-10 2:52 ` Masahiro Yamada @ 2017-11-15 20:42 ` Nick Desaulniers 0 siblings, 0 replies; 25+ messages in thread From: Nick Desaulniers @ 2017-11-15 20:42 UTC (permalink / raw) Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Douglas Anderson, Chris Fries, Nick Desaulniers, Masahiro Yamada, Michal Marek, linux-kbuild, linux-kernel From: Chris Fries <cfries@google.com> Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, so that ld-options (etc.) can work correctly. This fixes errors with clang such as ld-options trying to CC against your host architecture, but LD trying to link against your target architecture. Reviewed-by: Matthias Kaehlcke <mka@chromium.org> Signed-off-by: Chris Fries <cfries@google.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> Tested-by: Matthias Kaehlcke <mka@chromium.org> --- Changes since v2: * Move clang block lower in Makefile, so as not to run it when running configuration, per Masahiro. * Remove paragraphs from commit message, per Masahiro. * Add Masahiro to Suggested-by line in commit message. Makefile | 64 ++++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/Makefile b/Makefile index a7476e6934f1..b3352becc7df 100644 --- a/Makefile +++ b/Makefile @@ -608,6 +608,38 @@ CFLAGS_GCOV := -fprofile-arcs -ftest-coverage -fno-tree-loop-im $(call cc-disabl CFLAGS_KCOV := $(call cc-option,-fsanitize-coverage=trace-pc,) export CFLAGS_GCOV CFLAGS_KCOV +ifeq ($(cc-name),clang) +ifneq ($(CROSS_COMPILE),) +CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) +endif +ifneq ($(GCC_TOOLCHAIN),) +CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) +endif +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) +# Quiet clang warning: comparison of unsigned expression < 0 is always false +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the +# source of a reference will be _MergedGlobals and not on of the whitelisted names. +# See modpost pattern 2 +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) +else + +# These warnings generated too much noise in a regular build. +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) +endif + # The arch Makefile can set ARCH_{CPP,A,C}FLAGS to override the default # values of the respective KBUILD_* variables ARCH_CPPFLAGS := @@ -682,38 +714,6 @@ ifdef CONFIG_CC_STACKPROTECTOR endif KBUILD_CFLAGS += $(stackp-flag) -ifeq ($(cc-name),clang) -ifneq ($(CROSS_COMPILE),) -CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) -endif -ifneq ($(GCC_TOOLCHAIN),) -CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) -endif -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) -KBUILD_CFLAGS += $(call cc-disable-warning, gnu) -KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) -# Quiet clang warning: comparison of unsigned expression < 0 is always false -KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) -# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the -# source of a reference will be _MergedGlobals and not on of the whitelisted names. -# See modpost pattern 2 -KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) -KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) -else - -# These warnings generated too much noise in a regular build. -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) -endif - ifdef CONFIG_FRAME_POINTER KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls else -- 2.15.0.448.gf294e3d99a-goog ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile @ 2017-11-15 20:42 ` Nick Desaulniers 0 siblings, 0 replies; 25+ messages in thread From: Nick Desaulniers @ 2017-11-15 20:42 UTC (permalink / raw) Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Douglas Anderson, Chris Fries, Nick Desaulniers, Masahiro Yamada, Michal Marek, linux-kbuild, linux-kernel From: Chris Fries <cfries@google.com> Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, so that ld-options (etc.) can work correctly. This fixes errors with clang such as ld-options trying to CC against your host architecture, but LD trying to link against your target architecture. Reviewed-by: Matthias Kaehlcke <mka@chromium.org> Signed-off-by: Chris Fries <cfries@google.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> Tested-by: Matthias Kaehlcke <mka@chromium.org> --- Changes since v2: * Move clang block lower in Makefile, so as not to run it when running configuration, per Masahiro. * Remove paragraphs from commit message, per Masahiro. * Add Masahiro to Suggested-by line in commit message. Makefile | 64 ++++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/Makefile b/Makefile index a7476e6934f1..b3352becc7df 100644 --- a/Makefile +++ b/Makefile @@ -608,6 +608,38 @@ CFLAGS_GCOV := -fprofile-arcs -ftest-coverage -fno-tree-loop-im $(call cc-disabl CFLAGS_KCOV := $(call cc-option,-fsanitize-coverage=trace-pc,) export CFLAGS_GCOV CFLAGS_KCOV +ifeq ($(cc-name),clang) +ifneq ($(CROSS_COMPILE),) +CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) +endif +ifneq ($(GCC_TOOLCHAIN),) +CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) +endif +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) +# Quiet clang warning: comparison of unsigned expression < 0 is always false +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the +# source of a reference will be _MergedGlobals and not on of the whitelisted names. +# See modpost pattern 2 +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) +else + +# These warnings generated too much noise in a regular build. +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) +endif + # The arch Makefile can set ARCH_{CPP,A,C}FLAGS to override the default # values of the respective KBUILD_* variables ARCH_CPPFLAGS := @@ -682,38 +714,6 @@ ifdef CONFIG_CC_STACKPROTECTOR endif KBUILD_CFLAGS += $(stackp-flag) -ifeq ($(cc-name),clang) -ifneq ($(CROSS_COMPILE),) -CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) -endif -ifneq ($(GCC_TOOLCHAIN),) -CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) -endif -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) -KBUILD_CFLAGS += $(call cc-disable-warning, gnu) -KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) -# Quiet clang warning: comparison of unsigned expression < 0 is always false -KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) -# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the -# source of a reference will be _MergedGlobals and not on of the whitelisted names. -# See modpost pattern 2 -KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) -KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) -else - -# These warnings generated too much noise in a regular build. -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) -endif - ifdef CONFIG_FRAME_POINTER KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls else -- 2.15.0.448.gf294e3d99a-goog ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-15 20:42 ` Nick Desaulniers (?) @ 2017-11-16 2:32 ` Masahiro Yamada -1 siblings, 0 replies; 25+ messages in thread From: Masahiro Yamada @ 2017-11-16 2:32 UTC (permalink / raw) To: Nick Desaulniers Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Douglas Anderson, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List Hi Nick, 2017-11-16 5:42 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > From: Chris Fries <cfries@google.com> > > Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, > so that ld-options (etc.) can work correctly. > > This fixes errors with clang such as ld-options trying to CC > against your host architecture, but LD trying to link against > your target architecture. > > Reviewed-by: Matthias Kaehlcke <mka@chromium.org> > Signed-off-by: Chris Fries <cfries@google.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> > Tested-by: Matthias Kaehlcke <mka@chromium.org> > --- > Changes since v2: > * Move clang block lower in Makefile, so as not to run it when running > configuration, per Masahiro. > * Remove paragraphs from commit message, per Masahiro. > * Add Masahiro to Suggested-by line in commit message. > > Makefile | 64 ++++++++++++++++++++++++++++++++-------------------------------- > 1 file changed, 32 insertions(+), 32 deletions(-) > > diff --git a/Makefile b/Makefile > index a7476e6934f1..b3352becc7df 100644 > --- a/Makefile > +++ b/Makefile > @@ -608,6 +608,38 @@ CFLAGS_GCOV := -fprofile-arcs -ftest-coverage -fno-tree-loop-im $(call cc-disabl > CFLAGS_KCOV := $(call cc-option,-fsanitize-coverage=trace-pc,) > export CFLAGS_GCOV CFLAGS_KCOV > > +ifeq ($(cc-name),clang) > +ifneq ($(CROSS_COMPILE),) > +CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) > +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > +endif > +ifneq ($(GCC_TOOLCHAIN),) > +CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) > +endif > +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > +KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) > +KBUILD_CFLAGS += $(call cc-disable-warning, gnu) > +KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > +# Quiet clang warning: comparison of unsigned expression < 0 is always false > +KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) > +# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the > +# source of a reference will be _MergedGlobals and not on of the whitelisted names. > +# See modpost pattern 2 > +KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) > +KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) > +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > +else > + > +# These warnings generated too much noise in a regular build. > +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > +endif > + > # The arch Makefile can set ARCH_{CPP,A,C}FLAGS to override the default > # values of the respective KBUILD_* variables > ARCH_CPPFLAGS := > @@ -682,38 +714,6 @@ ifdef CONFIG_CC_STACKPROTECTOR > endif > KBUILD_CFLAGS += $(stackp-flag) > > -ifeq ($(cc-name),clang) > -ifneq ($(CROSS_COMPILE),) > -CLANG_TARGET := -target $(notdir $(CROSS_COMPILE:%-=%)) > -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > -endif > -ifneq ($(GCC_TOOLCHAIN),) > -CLANG_GCC_TC := -gcc-toolchain $(GCC_TOOLCHAIN) > -endif > -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > -KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier) > -KBUILD_CFLAGS += $(call cc-disable-warning, gnu) > -KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member) > -# Quiet clang warning: comparison of unsigned expression < 0 is always false > -KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare) > -# CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the > -# source of a reference will be _MergedGlobals and not on of the whitelisted names. > -# See modpost pattern 2 > -KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,) > -KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) > -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > -else > - > -# These warnings generated too much noise in a regular build. > -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > -endif > - > ifdef CONFIG_FRAME_POINTER > KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls > else > -- > 2.15.0.448.gf294e3d99a-goog > BTW, I notice another issue. If we move clang settings before including arch Makefile, "ifneq ($(CROSS_COMPILE),)" comes early. Some arch Makefiles (arch/mips/Makefile, arch/blackfin/Makefile, etc.) set CROSS_COMPILE there if CROSS_COMPILE is not given. Then, we have a conflict between two requirements among arch. [1] arm64, powerpc use ld-option in their Makefile. So, clang flags must be set before inc. arch Makefile. [2] mips, blackfin, etc. may set CROSS_COMPILE in their Makefile. So, we want to reference CROSS_COMPILE only after inc. arch Makefile I have no idea how to solve it. At this moment, I guess clang is intended to support only limited architectures. It might be OK to be compromised here. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-15 20:42 ` Nick Desaulniers (?) (?) @ 2017-11-18 4:09 ` Masahiro Yamada 2017-11-23 4:24 ` Masahiro Yamada -1 siblings, 1 reply; 25+ messages in thread From: Masahiro Yamada @ 2017-11-18 4:09 UTC (permalink / raw) To: Nick Desaulniers Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Douglas Anderson, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List 2017-11-16 5:42 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > From: Chris Fries <cfries@google.com> > > Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, > so that ld-options (etc.) can work correctly. > > This fixes errors with clang such as ld-options trying to CC > against your host architecture, but LD trying to link against > your target architecture. > > Reviewed-by: Matthias Kaehlcke <mka@chromium.org> > Signed-off-by: Chris Fries <cfries@google.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> > Tested-by: Matthias Kaehlcke <mka@chromium.org> > --- Applied to linux-kbuild/kbuild. Thanks! -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-18 4:09 ` Masahiro Yamada @ 2017-11-23 4:24 ` Masahiro Yamada 2017-11-28 18:18 ` Nick Desaulniers 0 siblings, 1 reply; 25+ messages in thread From: Masahiro Yamada @ 2017-11-23 4:24 UTC (permalink / raw) To: Nick Desaulniers Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Douglas Anderson, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List Hi. 2017-11-18 13:09 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>: > 2017-11-16 5:42 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: >> From: Chris Fries <cfries@google.com> >> >> Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, >> so that ld-options (etc.) can work correctly. >> >> This fixes errors with clang such as ld-options trying to CC >> against your host architecture, but LD trying to link against >> your target architecture. >> >> Reviewed-by: Matthias Kaehlcke <mka@chromium.org> >> Signed-off-by: Chris Fries <cfries@google.com> >> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> >> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> >> Tested-by: Matthias Kaehlcke <mka@chromium.org> >> --- > > Applied to linux-kbuild/kbuild. Thanks! > > -- > Best Regards > Masahiro Yamada After more thought, I picked up your v2. (clang variables before kernel configuration) Linus suggests to move compiler flag testing to Kconfig. To do it, we need to feed target CC information to Kconfig. Sorry for spending your time. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-23 4:24 ` Masahiro Yamada 2017-11-28 18:18 ` Nick Desaulniers @ 2017-11-28 18:18 ` Nick Desaulniers 0 siblings, 0 replies; 25+ messages in thread From: Nick Desaulniers @ 2017-11-28 18:18 UTC (permalink / raw) To: Masahiro Yamada Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Douglas Anderson, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, linux-mips, linux-arm-kernel, linux-hexagon, openrisc Hi Masahiro, Thanks for merging Chris' patch, and sorry for taking so long to respond. On Wed, Nov 22, 2017 at 8:24 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > Linus suggests to move compiler flag testing to Kconfig. Do you have an LKML link for context? On Wed, Nov 15, 2017 at 6:32 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > BTW, I notice another issue. > > If we move clang settings before including arch Makefile, > "ifneq ($(CROSS_COMPILE),)" comes early. > > Some arch Makefiles (arch/mips/Makefile, arch/blackfin/Makefile, etc.) > set CROSS_COMPILE there if CROSS_COMPILE is not given. > > Then, we have a conflict between two requirements among arch. > > [1] arm64, powerpc use ld-option in their Makefile. > So, clang flags must be set before inc. arch Makefile. > [2] mips, blackfin, etc. may set CROSS_COMPILE in their Makefile. > So, we want to reference CROSS_COMPILE only after inc. arch Makefile > > I have no idea how to solve it. > > At this moment, I guess clang is intended to support > only limited architectures. > > It might be OK to be compromised here. I definitely find it curious that certain arch's define CROSS_COMPILE themselves. The benefit is one less argument to supply at compile time, but it assumes that the toolchain always has a certain prefix. This makes sense to me when cross compiling, but seems odd when compiling natively on that arch as the host and target. Maybe those arch's use that convention, or simply are always cross compiled for? Taking a survey of all arch's currently in the kernel via `cd arch; ag CROSS_COMPILE` and quickly eyeballing the result: m68k if not set arc if not set openrisc for some configs (openrisc/configs/or1ksim_defconfig, openrisc/configs/simple_smp_defconfig) blackfin if not set hexagon for some configs (hexagon/configs/comet_defconfig) parisc if not set sh if not set xtensa if not set score always arm for some configs (arm/configs/lpc18xx_defconfig) h8300 if not set mips if not set (and explicitly emptied for some configs, mips/configs/nlm_xlr_defconfig ) unicore32 if not set tile if not set The * if not set (or not being on the list) seems correct, as the top level Makefile will handle this correctly. Setting it for some configs seems curious (not necessarily wrong?), emptying it/always setting it via config sounds wrong to me, but maybe those hosts don't have toolchains and must always be cross compiled for? For reference, this file in LLVM source defines the supported backend targets: https://llvm.org/doxygen/Triple_8h_source.html Either way, it sounds like we're all set here, I guess I'm just curious about the LKML link/context and why some configs set CROSS_COMPILE themselves? ^ permalink raw reply [flat|nested] 25+ messages in thread
* [OpenRISC] [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile @ 2017-11-28 18:18 ` Nick Desaulniers 0 siblings, 0 replies; 25+ messages in thread From: Nick Desaulniers @ 2017-11-28 18:18 UTC (permalink / raw) To: openrisc Hi Masahiro, Thanks for merging Chris' patch, and sorry for taking so long to respond. On Wed, Nov 22, 2017 at 8:24 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > Linus suggests to move compiler flag testing to Kconfig. Do you have an LKML link for context? On Wed, Nov 15, 2017 at 6:32 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > BTW, I notice another issue. > > If we move clang settings before including arch Makefile, > "ifneq ($(CROSS_COMPILE),)" comes early. > > Some arch Makefiles (arch/mips/Makefile, arch/blackfin/Makefile, etc.) > set CROSS_COMPILE there if CROSS_COMPILE is not given. > > Then, we have a conflict between two requirements among arch. > > [1] arm64, powerpc use ld-option in their Makefile. > So, clang flags must be set before inc. arch Makefile. > [2] mips, blackfin, etc. may set CROSS_COMPILE in their Makefile. > So, we want to reference CROSS_COMPILE only after inc. arch Makefile > > I have no idea how to solve it. > > At this moment, I guess clang is intended to support > only limited architectures. > > It might be OK to be compromised here. I definitely find it curious that certain arch's define CROSS_COMPILE themselves. The benefit is one less argument to supply at compile time, but it assumes that the toolchain always has a certain prefix. This makes sense to me when cross compiling, but seems odd when compiling natively on that arch as the host and target. Maybe those arch's use that convention, or simply are always cross compiled for? Taking a survey of all arch's currently in the kernel via `cd arch; ag CROSS_COMPILE` and quickly eyeballing the result: m68k if not set arc if not set openrisc for some configs (openrisc/configs/or1ksim_defconfig, openrisc/configs/simple_smp_defconfig) blackfin if not set hexagon for some configs (hexagon/configs/comet_defconfig) parisc if not set sh if not set xtensa if not set score always arm for some configs (arm/configs/lpc18xx_defconfig) h8300 if not set mips if not set (and explicitly emptied for some configs, mips/configs/nlm_xlr_defconfig ) unicore32 if not set tile if not set The * if not set (or not being on the list) seems correct, as the top level Makefile will handle this correctly. Setting it for some configs seems curious (not necessarily wrong?), emptying it/always setting it via config sounds wrong to me, but maybe those hosts don't have toolchains and must always be cross compiled for? For reference, this file in LLVM source defines the supported backend targets: https://llvm.org/doxygen/Triple_8h_source.html Either way, it sounds like we're all set here, I guess I'm just curious about the LKML link/context and why some configs set CROSS_COMPILE themselves? ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile @ 2017-11-28 18:18 ` Nick Desaulniers 0 siblings, 0 replies; 25+ messages in thread From: Nick Desaulniers @ 2017-11-28 18:18 UTC (permalink / raw) To: linux-arm-kernel Hi Masahiro, Thanks for merging Chris' patch, and sorry for taking so long to respond. On Wed, Nov 22, 2017 at 8:24 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > Linus suggests to move compiler flag testing to Kconfig. Do you have an LKML link for context? On Wed, Nov 15, 2017 at 6:32 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > BTW, I notice another issue. > > If we move clang settings before including arch Makefile, > "ifneq ($(CROSS_COMPILE),)" comes early. > > Some arch Makefiles (arch/mips/Makefile, arch/blackfin/Makefile, etc.) > set CROSS_COMPILE there if CROSS_COMPILE is not given. > > Then, we have a conflict between two requirements among arch. > > [1] arm64, powerpc use ld-option in their Makefile. > So, clang flags must be set before inc. arch Makefile. > [2] mips, blackfin, etc. may set CROSS_COMPILE in their Makefile. > So, we want to reference CROSS_COMPILE only after inc. arch Makefile > > I have no idea how to solve it. > > At this moment, I guess clang is intended to support > only limited architectures. > > It might be OK to be compromised here. I definitely find it curious that certain arch's define CROSS_COMPILE themselves. The benefit is one less argument to supply at compile time, but it assumes that the toolchain always has a certain prefix. This makes sense to me when cross compiling, but seems odd when compiling natively on that arch as the host and target. Maybe those arch's use that convention, or simply are always cross compiled for? Taking a survey of all arch's currently in the kernel via `cd arch; ag CROSS_COMPILE` and quickly eyeballing the result: m68k if not set arc if not set openrisc for some configs (openrisc/configs/or1ksim_defconfig, openrisc/configs/simple_smp_defconfig) blackfin if not set hexagon for some configs (hexagon/configs/comet_defconfig) parisc if not set sh if not set xtensa if not set score always arm for some configs (arm/configs/lpc18xx_defconfig) h8300 if not set mips if not set (and explicitly emptied for some configs, mips/configs/nlm_xlr_defconfig ) unicore32 if not set tile if not set The * if not set (or not being on the list) seems correct, as the top level Makefile will handle this correctly. Setting it for some configs seems curious (not necessarily wrong?), emptying it/always setting it via config sounds wrong to me, but maybe those hosts don't have toolchains and must always be cross compiled for? For reference, this file in LLVM source defines the supported backend targets: https://llvm.org/doxygen/Triple_8h_source.html Either way, it sounds like we're all set here, I guess I'm just curious about the LKML link/context and why some configs set CROSS_COMPILE themselves? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-28 18:18 ` Nick Desaulniers (?) @ 2017-11-28 19:27 ` Geert Uytterhoeven -1 siblings, 0 replies; 25+ messages in thread From: Geert Uytterhoeven @ 2017-11-28 19:27 UTC (permalink / raw) To: Nick Desaulniers Cc: Masahiro Yamada, Linux MIPS Mailing List, Linux Kbuild mailing list, open list:QUALCOMM HEXAGON..., Douglas Anderson, Greg Hackmann, Linux Kernel Mailing List, Openrisc, Michal Marek, Matthias Kaehlcke, Jan-Simon Möller, Chris Fries, Mark Charlebois, linux-arm-kernel Hi Nick, On Tue, Nov 28, 2017 at 7:18 PM, Nick Desaulniers <ndesaulniers@google.com> wrote: > Thanks for merging Chris' patch, and sorry for taking so long to respond. > > On Wed, Nov 22, 2017 at 8:24 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> Linus suggests to move compiler flag testing to Kconfig. > > Do you have an LKML link for context? > > On Wed, Nov 15, 2017 at 6:32 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> BTW, I notice another issue. >> >> If we move clang settings before including arch Makefile, >> "ifneq ($(CROSS_COMPILE),)" comes early. >> >> Some arch Makefiles (arch/mips/Makefile, arch/blackfin/Makefile, etc.) >> set CROSS_COMPILE there if CROSS_COMPILE is not given. >> >> Then, we have a conflict between two requirements among arch. >> >> [1] arm64, powerpc use ld-option in their Makefile. >> So, clang flags must be set before inc. arch Makefile. >> [2] mips, blackfin, etc. may set CROSS_COMPILE in their Makefile. >> So, we want to reference CROSS_COMPILE only after inc. arch Makefile >> >> I have no idea how to solve it. >> >> At this moment, I guess clang is intended to support >> only limited architectures. >> >> It might be OK to be compromised here. > > I definitely find it curious that certain arch's define CROSS_COMPILE > themselves. The benefit is one less argument to supply at compile > time, but it assumes that the toolchain always has a certain prefix. > This makes sense to me when cross compiling, but seems odd when > compiling natively on that arch as the host and target. Maybe those > arch's use that convention, or simply are always cross compiled for? > > Taking a survey of all arch's currently in the kernel via `cd arch; ag > CROSS_COMPILE` and quickly eyeballing the result: > > m68k if not set And inside "ifneq ($(SUBARCH),$(ARCH))", so the prefix is not used for native compilation. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 25+ messages in thread
* [OpenRISC] [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile @ 2017-11-28 19:27 ` Geert Uytterhoeven 0 siblings, 0 replies; 25+ messages in thread From: Geert Uytterhoeven @ 2017-11-28 19:27 UTC (permalink / raw) To: openrisc Hi Nick, On Tue, Nov 28, 2017 at 7:18 PM, Nick Desaulniers <ndesaulniers@google.com> wrote: > Thanks for merging Chris' patch, and sorry for taking so long to respond. > > On Wed, Nov 22, 2017 at 8:24 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> Linus suggests to move compiler flag testing to Kconfig. > > Do you have an LKML link for context? > > On Wed, Nov 15, 2017 at 6:32 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> BTW, I notice another issue. >> >> If we move clang settings before including arch Makefile, >> "ifneq ($(CROSS_COMPILE),)" comes early. >> >> Some arch Makefiles (arch/mips/Makefile, arch/blackfin/Makefile, etc.) >> set CROSS_COMPILE there if CROSS_COMPILE is not given. >> >> Then, we have a conflict between two requirements among arch. >> >> [1] arm64, powerpc use ld-option in their Makefile. >> So, clang flags must be set before inc. arch Makefile. >> [2] mips, blackfin, etc. may set CROSS_COMPILE in their Makefile. >> So, we want to reference CROSS_COMPILE only after inc. arch Makefile >> >> I have no idea how to solve it. >> >> At this moment, I guess clang is intended to support >> only limited architectures. >> >> It might be OK to be compromised here. > > I definitely find it curious that certain arch's define CROSS_COMPILE > themselves. The benefit is one less argument to supply at compile > time, but it assumes that the toolchain always has a certain prefix. > This makes sense to me when cross compiling, but seems odd when > compiling natively on that arch as the host and target. Maybe those > arch's use that convention, or simply are always cross compiled for? > > Taking a survey of all arch's currently in the kernel via `cd arch; ag > CROSS_COMPILE` and quickly eyeballing the result: > > m68k if not set And inside "ifneq ($(SUBARCH),$(ARCH))", so the prefix is not used for native compilation. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile @ 2017-11-28 19:27 ` Geert Uytterhoeven 0 siblings, 0 replies; 25+ messages in thread From: Geert Uytterhoeven @ 2017-11-28 19:27 UTC (permalink / raw) To: linux-arm-kernel Hi Nick, On Tue, Nov 28, 2017 at 7:18 PM, Nick Desaulniers <ndesaulniers@google.com> wrote: > Thanks for merging Chris' patch, and sorry for taking so long to respond. > > On Wed, Nov 22, 2017 at 8:24 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> Linus suggests to move compiler flag testing to Kconfig. > > Do you have an LKML link for context? > > On Wed, Nov 15, 2017 at 6:32 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> BTW, I notice another issue. >> >> If we move clang settings before including arch Makefile, >> "ifneq ($(CROSS_COMPILE),)" comes early. >> >> Some arch Makefiles (arch/mips/Makefile, arch/blackfin/Makefile, etc.) >> set CROSS_COMPILE there if CROSS_COMPILE is not given. >> >> Then, we have a conflict between two requirements among arch. >> >> [1] arm64, powerpc use ld-option in their Makefile. >> So, clang flags must be set before inc. arch Makefile. >> [2] mips, blackfin, etc. may set CROSS_COMPILE in their Makefile. >> So, we want to reference CROSS_COMPILE only after inc. arch Makefile >> >> I have no idea how to solve it. >> >> At this moment, I guess clang is intended to support >> only limited architectures. >> >> It might be OK to be compromised here. > > I definitely find it curious that certain arch's define CROSS_COMPILE > themselves. The benefit is one less argument to supply at compile > time, but it assumes that the toolchain always has a certain prefix. > This makes sense to me when cross compiling, but seems odd when > compiling natively on that arch as the host and target. Maybe those > arch's use that convention, or simply are always cross compiled for? > > Taking a survey of all arch's currently in the kernel via `cd arch; ag > CROSS_COMPILE` and quickly eyeballing the result: > > m68k if not set And inside "ifneq ($(SUBARCH),$(ARCH))", so the prefix is not used for native compilation. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-28 18:18 ` Nick Desaulniers (?) @ 2017-11-29 2:39 ` Masahiro Yamada -1 siblings, 0 replies; 25+ messages in thread From: Masahiro Yamada @ 2017-11-29 2:39 UTC (permalink / raw) To: Nick Desaulniers Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Douglas Anderson, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List, Linux-MIPS, linux-arm-kernel, linux-hexagon, openrisc Hi Nick, 2017-11-29 3:18 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > Hi Masahiro, > > Thanks for merging Chris' patch, and sorry for taking so long to respond. > > On Wed, Nov 22, 2017 at 8:24 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> Linus suggests to move compiler flag testing to Kconfig. > > Do you have an LKML link for context? https://lkml.org/lkml/2017/11/19/291 > On Wed, Nov 15, 2017 at 6:32 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> BTW, I notice another issue. >> >> If we move clang settings before including arch Makefile, >> "ifneq ($(CROSS_COMPILE),)" comes early. >> >> Some arch Makefiles (arch/mips/Makefile, arch/blackfin/Makefile, etc.) >> set CROSS_COMPILE there if CROSS_COMPILE is not given. >> >> Then, we have a conflict between two requirements among arch. >> >> [1] arm64, powerpc use ld-option in their Makefile. >> So, clang flags must be set before inc. arch Makefile. >> [2] mips, blackfin, etc. may set CROSS_COMPILE in their Makefile. >> So, we want to reference CROSS_COMPILE only after inc. arch Makefile >> >> I have no idea how to solve it. >> >> At this moment, I guess clang is intended to support >> only limited architectures. >> >> It might be OK to be compromised here. > > I definitely find it curious that certain arch's define CROSS_COMPILE > themselves. The benefit is one less argument to supply at compile > time, but it assumes that the toolchain always has a certain prefix. > This makes sense to me when cross compiling, but seems odd when > compiling natively on that arch as the host and target. Maybe those > arch's use that convention, or simply are always cross compiled for? > > Taking a survey of all arch's currently in the kernel via `cd arch; ag > CROSS_COMPILE` and quickly eyeballing the result: > > m68k if not set > arc if not set > openrisc for some configs (openrisc/configs/or1ksim_defconfig, > openrisc/configs/simple_smp_defconfig) > blackfin if not set > hexagon for some configs (hexagon/configs/comet_defconfig) > parisc if not set > sh if not set > xtensa if not set > score always > arm for some configs (arm/configs/lpc18xx_defconfig) > h8300 if not set > mips if not set (and explicitly emptied for some configs, > mips/configs/nlm_xlr_defconfig ) > unicore32 if not set > tile if not set > > The * if not set (or not being on the list) seems correct, as the top > level Makefile will handle this correctly. Setting it for some > configs seems curious (not necessarily wrong?), Perhaps, the maintainer of those platforms may want to save one command-line argument at compile time. > emptying it/always > setting it via config sounds wrong to me, arch/mips/configs/nlm_xlr_defconfig explicitly empties it. It is just redundant because CONFIG_CROSS_COMPILE="" is the default in Kconfig. Not necessarily wrong, I think. > but maybe those hosts don't > have toolchains and must always be cross compiled for? > > For reference, this file in LLVM source defines the supported backend > targets: https://llvm.org/doxygen/Triple_8h_source.html > > Either way, it sounds like we're all set here, I guess I'm just > curious about the LKML link/context and why some configs set > CROSS_COMPILE themselves? > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 25+ messages in thread
* [OpenRISC] [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile @ 2017-11-29 2:39 ` Masahiro Yamada 0 siblings, 0 replies; 25+ messages in thread From: Masahiro Yamada @ 2017-11-29 2:39 UTC (permalink / raw) To: openrisc Hi Nick, 2017-11-29 3:18 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > Hi Masahiro, > > Thanks for merging Chris' patch, and sorry for taking so long to respond. > > On Wed, Nov 22, 2017 at 8:24 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> Linus suggests to move compiler flag testing to Kconfig. > > Do you have an LKML link for context? https://lkml.org/lkml/2017/11/19/291 > On Wed, Nov 15, 2017 at 6:32 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> BTW, I notice another issue. >> >> If we move clang settings before including arch Makefile, >> "ifneq ($(CROSS_COMPILE),)" comes early. >> >> Some arch Makefiles (arch/mips/Makefile, arch/blackfin/Makefile, etc.) >> set CROSS_COMPILE there if CROSS_COMPILE is not given. >> >> Then, we have a conflict between two requirements among arch. >> >> [1] arm64, powerpc use ld-option in their Makefile. >> So, clang flags must be set before inc. arch Makefile. >> [2] mips, blackfin, etc. may set CROSS_COMPILE in their Makefile. >> So, we want to reference CROSS_COMPILE only after inc. arch Makefile >> >> I have no idea how to solve it. >> >> At this moment, I guess clang is intended to support >> only limited architectures. >> >> It might be OK to be compromised here. > > I definitely find it curious that certain arch's define CROSS_COMPILE > themselves. The benefit is one less argument to supply at compile > time, but it assumes that the toolchain always has a certain prefix. > This makes sense to me when cross compiling, but seems odd when > compiling natively on that arch as the host and target. Maybe those > arch's use that convention, or simply are always cross compiled for? > > Taking a survey of all arch's currently in the kernel via `cd arch; ag > CROSS_COMPILE` and quickly eyeballing the result: > > m68k if not set > arc if not set > openrisc for some configs (openrisc/configs/or1ksim_defconfig, > openrisc/configs/simple_smp_defconfig) > blackfin if not set > hexagon for some configs (hexagon/configs/comet_defconfig) > parisc if not set > sh if not set > xtensa if not set > score always > arm for some configs (arm/configs/lpc18xx_defconfig) > h8300 if not set > mips if not set (and explicitly emptied for some configs, > mips/configs/nlm_xlr_defconfig ) > unicore32 if not set > tile if not set > > The * if not set (or not being on the list) seems correct, as the top > level Makefile will handle this correctly. Setting it for some > configs seems curious (not necessarily wrong?), Perhaps, the maintainer of those platforms may want to save one command-line argument at compile time. > emptying it/always > setting it via config sounds wrong to me, arch/mips/configs/nlm_xlr_defconfig explicitly empties it. It is just redundant because CONFIG_CROSS_COMPILE="" is the default in Kconfig. Not necessarily wrong, I think. > but maybe those hosts don't > have toolchains and must always be cross compiled for? > > For reference, this file in LLVM source defines the supported backend > targets: https://llvm.org/doxygen/Triple_8h_source.html > > Either way, it sounds like we're all set here, I guess I'm just > curious about the LKML link/context and why some configs set > CROSS_COMPILE themselves? > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile @ 2017-11-29 2:39 ` Masahiro Yamada 0 siblings, 0 replies; 25+ messages in thread From: Masahiro Yamada @ 2017-11-29 2:39 UTC (permalink / raw) To: linux-arm-kernel Hi Nick, 2017-11-29 3:18 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > Hi Masahiro, > > Thanks for merging Chris' patch, and sorry for taking so long to respond. > > On Wed, Nov 22, 2017 at 8:24 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> Linus suggests to move compiler flag testing to Kconfig. > > Do you have an LKML link for context? https://lkml.org/lkml/2017/11/19/291 > On Wed, Nov 15, 2017 at 6:32 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> BTW, I notice another issue. >> >> If we move clang settings before including arch Makefile, >> "ifneq ($(CROSS_COMPILE),)" comes early. >> >> Some arch Makefiles (arch/mips/Makefile, arch/blackfin/Makefile, etc.) >> set CROSS_COMPILE there if CROSS_COMPILE is not given. >> >> Then, we have a conflict between two requirements among arch. >> >> [1] arm64, powerpc use ld-option in their Makefile. >> So, clang flags must be set before inc. arch Makefile. >> [2] mips, blackfin, etc. may set CROSS_COMPILE in their Makefile. >> So, we want to reference CROSS_COMPILE only after inc. arch Makefile >> >> I have no idea how to solve it. >> >> At this moment, I guess clang is intended to support >> only limited architectures. >> >> It might be OK to be compromised here. > > I definitely find it curious that certain arch's define CROSS_COMPILE > themselves. The benefit is one less argument to supply at compile > time, but it assumes that the toolchain always has a certain prefix. > This makes sense to me when cross compiling, but seems odd when > compiling natively on that arch as the host and target. Maybe those > arch's use that convention, or simply are always cross compiled for? > > Taking a survey of all arch's currently in the kernel via `cd arch; ag > CROSS_COMPILE` and quickly eyeballing the result: > > m68k if not set > arc if not set > openrisc for some configs (openrisc/configs/or1ksim_defconfig, > openrisc/configs/simple_smp_defconfig) > blackfin if not set > hexagon for some configs (hexagon/configs/comet_defconfig) > parisc if not set > sh if not set > xtensa if not set > score always > arm for some configs (arm/configs/lpc18xx_defconfig) > h8300 if not set > mips if not set (and explicitly emptied for some configs, > mips/configs/nlm_xlr_defconfig ) > unicore32 if not set > tile if not set > > The * if not set (or not being on the list) seems correct, as the top > level Makefile will handle this correctly. Setting it for some > configs seems curious (not necessarily wrong?), Perhaps, the maintainer of those platforms may want to save one command-line argument at compile time. > emptying it/always > setting it via config sounds wrong to me, arch/mips/configs/nlm_xlr_defconfig explicitly empties it. It is just redundant because CONFIG_CROSS_COMPILE="" is the default in Kconfig. Not necessarily wrong, I think. > but maybe those hosts don't > have toolchains and must always be cross compiled for? > > For reference, this file in LLVM source defines the supported backend > targets: https://llvm.org/doxygen/Triple_8h_source.html > > Either way, it sounds like we're all set here, I guess I'm just > curious about the LKML link/context and why some configs set > CROSS_COMPILE themselves? > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-07 17:37 ` Nick Desaulniers 2017-11-07 19:46 ` Nick Desaulniers @ 2017-11-09 4:15 ` Masahiro Yamada 2017-11-09 16:58 ` Nick Desaulniers 1 sibling, 1 reply; 25+ messages in thread From: Masahiro Yamada @ 2017-11-09 4:15 UTC (permalink / raw) To: Nick Desaulniers Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List 2017-11-08 2:37 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > On Sat, Nov 4, 2017 at 8:06 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> ld-option is only used for arch/{arm64,powerpc}/Makefile >> >> arch/arm64/Makefile: ifeq ($(call ld-option, --fix-cortex-a53-843419),) >> arch/powerpc/Makefile:LDFLAGS_vmlinux += $(call >> ld-option,--orphan-handling=warn) >> >> I think this patch makes sense when it comes along with >> https://patchwork.kernel.org/patch/10030581/ > > Good point. > >> but, it is now being blocked by 0-day bot >> due to a x86 problem. > > Looks like that is now resolved (unless 0-day bot strikes again). > >> The location of CLANG_GCC_TC define >> only matters after your patch is applied, right? > > By "your patch" referring to the 0-day bot thread, yes. > >> Did my request for v2 break anything? > > Nothing immediately obvious, and no regressions. It just made this > patch necessary (along with my previous one) for correctly cross > compiling with clang for arm64 and powerpc as you point out. > >> One more thing: this patch does not apply to kbuild tree. > > I absolutely will rebase it on your tree and send a v2. Just to help > me understand the contribution model better: none of my other patches > have yet been requested against any trees other than Linus'. Is this > because of where we are in the release cycle, or that a lot of kbuild > code has changed, or what? Generally speaking, a preferred way is to base patches on the subsystem tree. Kernel developers are supposed to do their development on linux-next, but, in reality, many people work on Linus' tree since it is more stable and git history is fast-forward. In many cases, patches based on Linus' tree can apply to sub-systems as well. I am happy to fix-up a conflict locally as long as it is trivial, and there is no other reason for re-spin. Unfortunately, Kbuild tree changed the top-level Makefile a lot in this development cycle. If your patch does not apply cleanly, I do not know which context you are moving the code to. Also, I found suspicious description in the commit log. That's why. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile 2017-11-09 4:15 ` [PATCH] " Masahiro Yamada @ 2017-11-09 16:58 ` Nick Desaulniers 0 siblings, 0 replies; 25+ messages in thread From: Nick Desaulniers @ 2017-11-09 16:58 UTC (permalink / raw) To: Masahiro Yamada Cc: Behan Webster, Jan-Simon Möller, Mark Charlebois, Greg Hackmann, Matthias Kaehlcke, Chris Fries, Michal Marek, Linux Kbuild mailing list, Linux Kernel Mailing List On Wed, Nov 8, 2017 at 8:15 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2017-11-08 2:37 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: >> On Sat, Nov 4, 2017 at 8:06 PM, Masahiro Yamada >> <yamada.masahiro@socionext.com> wrote: >>> ld-option is only used for arch/{arm64,powerpc}/Makefile >>> >>> arch/arm64/Makefile: ifeq ($(call ld-option, --fix-cortex-a53-843419),) >>> arch/powerpc/Makefile:LDFLAGS_vmlinux += $(call >>> ld-option,--orphan-handling=warn) >>> >>> I think this patch makes sense when it comes along with >>> https://patchwork.kernel.org/patch/10030581/ >> >> Good point. >> >>> but, it is now being blocked by 0-day bot >>> due to a x86 problem. >> >> Looks like that is now resolved (unless 0-day bot strikes again). >> >>> The location of CLANG_GCC_TC define >>> only matters after your patch is applied, right? >> >> By "your patch" referring to the 0-day bot thread, yes. >> >>> Did my request for v2 break anything? >> >> Nothing immediately obvious, and no regressions. It just made this >> patch necessary (along with my previous one) for correctly cross >> compiling with clang for arm64 and powerpc as you point out. >> >>> One more thing: this patch does not apply to kbuild tree. >> >> I absolutely will rebase it on your tree and send a v2. Just to help >> me understand the contribution model better: none of my other patches >> have yet been requested against any trees other than Linus'. Is this >> because of where we are in the release cycle, or that a lot of kbuild >> code has changed, or what? > > > Generally speaking, > a preferred way is to base patches on the subsystem tree. > > Kernel developers are supposed to do their development on linux-next, > but, in reality, many people work on Linus' tree since it is more stable and > git history is fast-forward. > > In many cases, patches based on Linus' tree can apply to sub-systems as well. > > I am happy to fix-up a conflict locally > as long as it is trivial, and there is no other reason for re-spin. > > Unfortunately, Kbuild tree changed the top-level Makefile a lot in > this development cycle. > > If your patch does not apply cleanly, I do not know which context you > are moving the code to. > Also, I found suspicious description in the commit log. > > That's why. > > > -- > Best Regards > Masahiro Yamada Great, thanks for taking time to explain that, I appreciate it. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2017-11-29 2:40 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-02 21:26 [PATCH] kbuild: Set KBUILD_CFLAGS before incl. arch Makefile Nick Desaulniers 2017-11-04 0:37 ` Matthias Kaehlcke 2017-11-05 3:06 ` Masahiro Yamada 2017-11-07 17:37 ` Nick Desaulniers 2017-11-07 19:46 ` [PATCH v2] " Nick Desaulniers 2017-11-07 19:46 ` Nick Desaulniers 2017-11-09 4:31 ` Masahiro Yamada 2017-11-09 16:51 ` Nick Desaulniers 2017-11-10 2:52 ` Masahiro Yamada 2017-11-15 20:42 ` [PATCH v3] " Nick Desaulniers 2017-11-15 20:42 ` Nick Desaulniers 2017-11-16 2:32 ` Masahiro Yamada 2017-11-18 4:09 ` Masahiro Yamada 2017-11-23 4:24 ` Masahiro Yamada 2017-11-28 18:18 ` Nick Desaulniers 2017-11-28 18:18 ` [OpenRISC] " Nick Desaulniers 2017-11-28 18:18 ` Nick Desaulniers 2017-11-28 19:27 ` Geert Uytterhoeven 2017-11-28 19:27 ` [OpenRISC] " Geert Uytterhoeven 2017-11-28 19:27 ` Geert Uytterhoeven 2017-11-29 2:39 ` Masahiro Yamada 2017-11-29 2:39 ` [OpenRISC] " Masahiro Yamada 2017-11-29 2:39 ` Masahiro Yamada 2017-11-09 4:15 ` [PATCH] " Masahiro Yamada 2017-11-09 16:58 ` Nick Desaulniers
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.