All of lore.kernel.org
 help / color / mirror / Atom feed
* [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] 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 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] 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

* 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

* [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

* [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

* 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

* [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

* [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

* 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

* [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

* [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

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.