[PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Nick Desaulniers]

We get constant feedback that the command line invocation of make is too
long. CROSS_COMPILE is helpful when a toolchain has a prefix of the
target triple, or is an absolute path outside of $PATH, but it's mostly
redundant for a given ARCH.

If CROSS_COMPILE is not set, simply set --target=aarch64-linux for
CLANG_FLAGS, KBUILD_CFLAGS, and KBUILD_AFLAGS.

Previously, we'd cross compile via:
$ ARCH=arm64 CROSS_COMPILE=aarch64-linxu-gnu make LLVM=1 LLVM_IAS=1
Now:
$ ARCH=arm64 make LLVM=1 LLVM_IAS=1

We can drop gnu from the triple, but dropping linux from the triple
produces different .config files for the above invocations for the
defconfig target.

Link: https://github.com/ClangBuiltLinux/linux/issues/1399
Suggested-by: Arnd Bergmann <arnd@kernel.org>
Suggested-by: Fangrui Song <maskray@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/arm64/Makefile | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 7bc37d0a1b68..016873fddcc3 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -34,6 +34,17 @@ $(warning LSE atomics not supported by binutils)
   endif
 endif
 
+ifneq ($(LLVM),)
+ifneq ($(LLVM_IAS),)
+ifeq ($(CROSS_COMPILE),)
+CLANG_TARGET	:=--target=aarch64-linux
+CLANG_FLAGS	+= $(CLANG_TARGET)
+KBUILD_CFLAGS	+= $(CLANG_TARGET)
+KBUILD_AFLAGS	+= $(CLANG_TARGET)
+endif
+endif
+endif
+
 cc_has_k_constraint := $(call try-run,echo				\
 	'int main(void) {						\
 		asm volatile("and w0, w0, %w0" :: "K" (4294967295));	\
-- 
2.32.0.93.g670b81a890-goog

Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Tom Stellard]

On 7/1/21 4:55 PM, 'Nick Desaulniers' via Clang Built Linux wrote:
> We get constant feedback that the command line invocation of make is too
> long. CROSS_COMPILE is helpful when a toolchain has a prefix of the
> target triple, or is an absolute path outside of $PATH, but it's mostly
> redundant for a given ARCH.
> 
> If CROSS_COMPILE is not set, simply set --target=aarch64-linux for
> CLANG_FLAGS, KBUILD_CFLAGS, and KBUILD_AFLAGS.
> 
> Previously, we'd cross compile via:
> $ ARCH=arm64 CROSS_COMPILE=aarch64-linxu-gnu make LLVM=1 LLVM_IAS=1
> Now:
> $ ARCH=arm64 make LLVM=1 LLVM_IAS=1
> 
> We can drop gnu from the triple, but dropping linux from the triple
> produces different .config files for the above invocations for the
> defconfig target.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1399
> Suggested-by: Arnd Bergmann <arnd@kernel.org>
> Suggested-by: Fangrui Song <maskray@google.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>   arch/arm64/Makefile | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 7bc37d0a1b68..016873fddcc3 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile

Are there plans to do this for other architectures?

-Tom

> @@ -34,6 +34,17 @@ $(warning LSE atomics not supported by binutils)
>     endif
>   endif
>   
> +ifneq ($(LLVM),)
> +ifneq ($(LLVM_IAS),)
> +ifeq ($(CROSS_COMPILE),)
> +CLANG_TARGET	:=--target=aarch64-linux
> +CLANG_FLAGS	+= $(CLANG_TARGET)
> +KBUILD_CFLAGS	+= $(CLANG_TARGET)
> +KBUILD_AFLAGS	+= $(CLANG_TARGET)
> +endif
> +endif
> +endif
> +
>   cc_has_k_constraint := $(call try-run,echo				\
>   	'int main(void) {						\
>   		asm volatile("and w0, w0, %w0" :: "K" (4294967295));	\
> 

Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Nick Desaulniers]

On Thu, Jul 1, 2021 at 6:05 PM Tom Stellard <tstellar@redhat.com> wrote:
>
> On 7/1/21 4:55 PM, 'Nick Desaulniers' via Clang Built Linux wrote:
> > We get constant feedback that the command line invocation of make is too
> > long. CROSS_COMPILE is helpful when a toolchain has a prefix of the
> > target triple, or is an absolute path outside of $PATH, but it's mostly
> > redundant for a given ARCH.
> >
> > If CROSS_COMPILE is not set, simply set --target=aarch64-linux for
> > CLANG_FLAGS, KBUILD_CFLAGS, and KBUILD_AFLAGS.
>
> Are there plans to do this for other architectures?

Yep, just starting small to collect feedback on the idea before
blasting maintainers with more patches. The goal is to handle this in
a per arch/ manner, rather than the top level Makefile.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Will Deacon]

On Thu, Jul 01, 2021 at 04:55:05PM -0700, Nick Desaulniers wrote:
> We get constant feedback that the command line invocation of make is too
> long. CROSS_COMPILE is helpful when a toolchain has a prefix of the
> target triple, or is an absolute path outside of $PATH, but it's mostly
> redundant for a given ARCH.
> 
> If CROSS_COMPILE is not set, simply set --target=aarch64-linux for
> CLANG_FLAGS, KBUILD_CFLAGS, and KBUILD_AFLAGS.
> 
> Previously, we'd cross compile via:
> $ ARCH=arm64 CROSS_COMPILE=aarch64-linxu-gnu make LLVM=1 LLVM_IAS=1
> Now:
> $ ARCH=arm64 make LLVM=1 LLVM_IAS=1
> 
> We can drop gnu from the triple, but dropping linux from the triple
> produces different .config files for the above invocations for the
> defconfig target.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1399
> Suggested-by: Arnd Bergmann <arnd@kernel.org>
> Suggested-by: Fangrui Song <maskray@google.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  arch/arm64/Makefile | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 7bc37d0a1b68..016873fddcc3 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -34,6 +34,17 @@ $(warning LSE atomics not supported by binutils)
>    endif
>  endif
>  
> +ifneq ($(LLVM),)
> +ifneq ($(LLVM_IAS),)
> +ifeq ($(CROSS_COMPILE),)
> +CLANG_TARGET	:=--target=aarch64-linux
> +CLANG_FLAGS	+= $(CLANG_TARGET)
> +KBUILD_CFLAGS	+= $(CLANG_TARGET)
> +KBUILD_AFLAGS	+= $(CLANG_TARGET)

Do we need to do anything extra for the linker here? I can't see how we
avoid picking up the host copy.

> +endif
> +endif
> +endif

Have you tested the compat vDSO with this change? I think we'll just end
up passing two --target options, which is hopefully ok, but thought I'd
better check.

Will

Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Nick Desaulniers]

On Fri, Jul 2, 2021 at 4:22 AM Will Deacon <will@kernel.org> wrote:
>
> On Thu, Jul 01, 2021 at 04:55:05PM -0700, Nick Desaulniers wrote:
> > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> > index 7bc37d0a1b68..016873fddcc3 100644
> > --- a/arch/arm64/Makefile
> > +++ b/arch/arm64/Makefile
> > @@ -34,6 +34,17 @@ $(warning LSE atomics not supported by binutils)
> >    endif
> >  endif
> >
> > +ifneq ($(LLVM),)
> > +ifneq ($(LLVM_IAS),)
> > +ifeq ($(CROSS_COMPILE),)
> > +CLANG_TARGET :=--target=aarch64-linux
> > +CLANG_FLAGS  += $(CLANG_TARGET)
> > +KBUILD_CFLAGS        += $(CLANG_TARGET)
> > +KBUILD_AFLAGS        += $(CLANG_TARGET)
>
> Do we need to do anything extra for the linker here? I can't see how we
> avoid picking up the host copy.

That's handled by the top level Makefile when LLVM=1 is set.

There is $KBUILD_LDFLAGS, but we don't do anything with it at the
moment in terms of which linker we select; $LD controls which linker
we use.

LLD can figure out the target based on the object files it's given as
input, so it doesn't need any `--target=` flag. When clang is invoked
as the compiler or assembler, it does need --target.

> Have you tested the compat vDSO with this change? I think we'll just end
> up passing two --target options, which is hopefully ok, but thought I'd
> better check.

Good catch.  We don't reuse KBUILD_CFLAGS or KBUILD_AFLAGS for the
compat vdso for this very reason. In arch/arm64/kernel/vdso32/Makefile
you'll see no references to KBUILD_CFLAGS or KBUILD_AFLAGS; instead we
use VDSO_CFLAGS and VDSO_AFLAGS in their stead.

But, we could (and should) make this same change for the compat vdso,
and drop the need for CROSS_COMPILE_COMPAT for LLVM.

Let me play around with the changes Arnd suggested and see if I can
get that working.  I'm a bit nervous about making this depend on
something from the top level Makefile on initial glance; these changes
start to become tree wide rather than isolated per arch/, but let's
see.  Maybe at that point we carry a series in the kbuild tree with
acks for the arch/ specific changes from the respective maintainers?

Either way, I'll send a v2 that nixes CROSS_COMPILE_COMPAT for LLVM.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Arnd Bergmann]

On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> +ifneq ($(LLVM),)
> +ifneq ($(LLVM_IAS),)
> +ifeq ($(CROSS_COMPILE),)
> +CLANG_TARGET   :=--target=aarch64-linux
> +CLANG_FLAGS    += $(CLANG_TARGET)
> +KBUILD_CFLAGS  += $(CLANG_TARGET)
> +KBUILD_AFLAGS  += $(CLANG_TARGET)
> +endif
> +endif
> +endif

I think only the "CLANG_TARGET   :=--target=aarch64-linux" line should
go into the
per-architecture Makefile. It doesn't hurt to just set that
unconditionally here,
and then change the CLANG_FLAGS logic in the top-level Makefile to use this
in place of $(notdir $(CROSS_COMPILE:%-=%)).

       Arnd

Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Nick Desaulniers]

On Fri, Jul 2, 2021 at 4:59 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> >
> > +ifneq ($(LLVM),)
> > +ifneq ($(LLVM_IAS),)
> > +ifeq ($(CROSS_COMPILE),)
> > +CLANG_TARGET   :=--target=aarch64-linux
> > +CLANG_FLAGS    += $(CLANG_TARGET)
> > +KBUILD_CFLAGS  += $(CLANG_TARGET)
> > +KBUILD_AFLAGS  += $(CLANG_TARGET)
> > +endif
> > +endif
> > +endif
>
> I think only the "CLANG_TARGET   :=--target=aarch64-linux" line should
> go into the
> per-architecture Makefile. It doesn't hurt to just set that
> unconditionally here,
> and then change the CLANG_FLAGS logic in the top-level Makefile to use this
> in place of $(notdir $(CROSS_COMPILE:%-=%)).

I don't think we can do that. Based on the order the arch/ specific
Makefiles are included, if we don't eagerly add --target to the
KBUILD_{C|A}FLAGS, then cc-option, as-option, and as-instr macros
(defined in scripts/Makefile.compiler) checks in per arch/ Makefiles
may fail erroneously because --target was not set for
KBUILD_{C|A}FLAGS yet.

Another issue is the order of operations between the top level
Makefile and the per arch/ Makefiles.  The `notdir` block you
reference occurs earlier than the per-arch includes:

 609 TENTATIVE_CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
...
 648 include $(srctree)/arch/$(SRCARCH)/Makefile

We would need the opposite order to do what you describe. Reordering
these would effectively be a revert of
commit ae6b289a3789 ("kbuild: Set KBUILD_CFLAGS before incl. arch Makefile")
which I'm not sure we want to do.  But maybe there's another way I'm
not seeing yet?
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Nathan Chancellor]

On Fri, Jul 02, 2021 at 11:29:31AM -0700, Nick Desaulniers wrote:
> On Fri, Jul 2, 2021 at 4:59 AM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
> > Linux <clang-built-linux@googlegroups.com> wrote:
> > >
> > > +ifneq ($(LLVM),)
> > > +ifneq ($(LLVM_IAS),)
> > > +ifeq ($(CROSS_COMPILE),)
> > > +CLANG_TARGET   :=--target=aarch64-linux
> > > +CLANG_FLAGS    += $(CLANG_TARGET)
> > > +KBUILD_CFLAGS  += $(CLANG_TARGET)
> > > +KBUILD_AFLAGS  += $(CLANG_TARGET)
> > > +endif
> > > +endif
> > > +endif
> >
> > I think only the "CLANG_TARGET   :=--target=aarch64-linux" line should
> > go into the
> > per-architecture Makefile. It doesn't hurt to just set that
> > unconditionally here,
> > and then change the CLANG_FLAGS logic in the top-level Makefile to use this
> > in place of $(notdir $(CROSS_COMPILE:%-=%)).
> 
> I don't think we can do that. Based on the order the arch/ specific
> Makefiles are included, if we don't eagerly add --target to the
> KBUILD_{C|A}FLAGS, then cc-option, as-option, and as-instr macros
> (defined in scripts/Makefile.compiler) checks in per arch/ Makefiles
> may fail erroneously because --target was not set for
> KBUILD_{C|A}FLAGS yet.
> 
> Another issue is the order of operations between the top level
> Makefile and the per arch/ Makefiles.  The `notdir` block you
> reference occurs earlier than the per-arch includes:
> 
>  609 TENTATIVE_CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> ...
>  648 include $(srctree)/arch/$(SRCARCH)/Makefile
> 
> We would need the opposite order to do what you describe. Reordering
> these would effectively be a revert of
> commit ae6b289a3789 ("kbuild: Set KBUILD_CFLAGS before incl. arch Makefile")
> which I'm not sure we want to do.  But maybe there's another way I'm
> not seeing yet?

Is there any reason we cannot just add this sort of logic to the main
Makefile?

Such as (indentation to emphasis diff):

ifeq ($(CROSS_COMPILE),)
ifneq ($(LLVM),)
ifeq ($(LLVM_IAS),1)
	ifeq ($(ARCH),arm64)
		TENTATIVE_CLANG_FLAGS	+= --target=aarch64-linux
	else ifeq ($(ARCH),s390)
		TENTATIVE_CLANG_FLAGS	+= --target=s390x-linux
	else ifeq ($(ARCH),x86_64)
		TENTATIVE_CLANG_FLAGS	+= --target=x86_64-linux
	else
		$(error Specify CROSS_COMPILE or add '--target=' option to Makefile)
	endif
endif
endif
else
TENTATIVE_CLANG_FLAGS	+= --target=$(notdir $(CROSS_COMPILE:%-=%))
ifeq ($(LLVM_IAS),1)
TENTATIVE_CLANG_FLAGS	+= -integrated-as
else
TENTATIVE_CLANG_FLAGS	+= -no-integrated-as
GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
TENTATIVE_CLANG_FLAGS	+= --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
endif
endif

I know this looks a little cumbersome but it does help us avoid
duplication across architecture Makefiles and ordering dependencies.

Cheers,
Nathan

Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Nick Desaulniers]

On Sat, Jul 3, 2021 at 5:47 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Fri, Jul 02, 2021 at 11:29:31AM -0700, Nick Desaulniers wrote:
> > On Fri, Jul 2, 2021 at 4:59 AM Arnd Bergmann <arnd@kernel.org> wrote:
> > >
> > > On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
> > > Linux <clang-built-linux@googlegroups.com> wrote:
> > > >
> > > > +ifneq ($(LLVM),)
> > > > +ifneq ($(LLVM_IAS),)
> > > > +ifeq ($(CROSS_COMPILE),)
> > > > +CLANG_TARGET   :=--target=aarch64-linux
> > > > +CLANG_FLAGS    += $(CLANG_TARGET)
> > > > +KBUILD_CFLAGS  += $(CLANG_TARGET)
> > > > +KBUILD_AFLAGS  += $(CLANG_TARGET)
> > > > +endif
> > > > +endif
> > > > +endif
> > >
> > > I think only the "CLANG_TARGET   :=--target=aarch64-linux" line should
> > > go into the
> > > per-architecture Makefile. It doesn't hurt to just set that
> > > unconditionally here,
> > > and then change the CLANG_FLAGS logic in the top-level Makefile to use this
> > > in place of $(notdir $(CROSS_COMPILE:%-=%)).
> >
> > I don't think we can do that. Based on the order the arch/ specific
> > Makefiles are included, if we don't eagerly add --target to the
> > KBUILD_{C|A}FLAGS, then cc-option, as-option, and as-instr macros
> > (defined in scripts/Makefile.compiler) checks in per arch/ Makefiles
> > may fail erroneously because --target was not set for
> > KBUILD_{C|A}FLAGS yet.
> >
> > Another issue is the order of operations between the top level
> > Makefile and the per arch/ Makefiles.  The `notdir` block you
> > reference occurs earlier than the per-arch includes:
> >
> >  609 TENTATIVE_CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> > ...
> >  648 include $(srctree)/arch/$(SRCARCH)/Makefile
> >
> > We would need the opposite order to do what you describe. Reordering
> > these would effectively be a revert of
> > commit ae6b289a3789 ("kbuild: Set KBUILD_CFLAGS before incl. arch Makefile")
> > which I'm not sure we want to do.  But maybe there's another way I'm
> > not seeing yet?
>
> Is there any reason we cannot just add this sort of logic to the main
> Makefile?
>
> Such as (indentation to emphasis diff):
>
> ifeq ($(CROSS_COMPILE),)
> ifneq ($(LLVM),)
> ifeq ($(LLVM_IAS),1)
>         ifeq ($(ARCH),arm64)
>                 TENTATIVE_CLANG_FLAGS   += --target=aarch64-linux
>         else ifeq ($(ARCH),s390)
>                 TENTATIVE_CLANG_FLAGS   += --target=s390x-linux
>         else ifeq ($(ARCH),x86_64)
>                 TENTATIVE_CLANG_FLAGS   += --target=x86_64-linux
>         else
>                 $(error Specify CROSS_COMPILE or add '--target=' option to Makefile)
>         endif
> endif
> endif
> else
> TENTATIVE_CLANG_FLAGS   += --target=$(notdir $(CROSS_COMPILE:%-=%))
> ifeq ($(LLVM_IAS),1)
> TENTATIVE_CLANG_FLAGS   += -integrated-as
> else
> TENTATIVE_CLANG_FLAGS   += -no-integrated-as
> GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> TENTATIVE_CLANG_FLAGS   += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> endif
> endif
>
> I know this looks a little cumbersome but it does help us avoid
> duplication across architecture Makefiles and ordering dependencies.

Yeah, ok.

I like the use of `include` to compartmentalize the top level Makefile
further.  We can move this whole block of LLVM related flag handling
into something under scripts, then add this block and it doesn't look
too bad IMO.  Masahiro, are you ok with that?  If so, I'd break this
into 2 patches:
1. moving this block of existing code into a new file.
2. adding the CROSS_COMPILE functionality.

See https://groups.google.com/g/clang-built-linux/c/s-voh6WQFxM for
the gist of what I was thinking (though not broken into 2 patches yet,
just testing that it works; it does).

This approach will collide with Miguel's series in -next.  Should I
base the patches on mainline, or linux-kbuild, then have Miguel rebase
his patches on that or what?
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Nathan Chancellor]

On 7/7/2021 12:04 PM, Nick Desaulniers wrote:
> On Sat, Jul 3, 2021 at 5:47 PM Nathan Chancellor <nathan@kernel.org> wrote:
>>
>> On Fri, Jul 02, 2021 at 11:29:31AM -0700, Nick Desaulniers wrote:
>>> On Fri, Jul 2, 2021 at 4:59 AM Arnd Bergmann <arnd@kernel.org> wrote:
>>>>
>>>> On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
>>>> Linux <clang-built-linux@googlegroups.com> wrote:
>>>>>
>>>>> +ifneq ($(LLVM),)
>>>>> +ifneq ($(LLVM_IAS),)
>>>>> +ifeq ($(CROSS_COMPILE),)
>>>>> +CLANG_TARGET   :=--target=aarch64-linux
>>>>> +CLANG_FLAGS    += $(CLANG_TARGET)
>>>>> +KBUILD_CFLAGS  += $(CLANG_TARGET)
>>>>> +KBUILD_AFLAGS  += $(CLANG_TARGET)
>>>>> +endif
>>>>> +endif
>>>>> +endif
>>>>
>>>> I think only the "CLANG_TARGET   :=--target=aarch64-linux" line should
>>>> go into the
>>>> per-architecture Makefile. It doesn't hurt to just set that
>>>> unconditionally here,
>>>> and then change the CLANG_FLAGS logic in the top-level Makefile to use this
>>>> in place of $(notdir $(CROSS_COMPILE:%-=%)).
>>>
>>> I don't think we can do that. Based on the order the arch/ specific
>>> Makefiles are included, if we don't eagerly add --target to the
>>> KBUILD_{C|A}FLAGS, then cc-option, as-option, and as-instr macros
>>> (defined in scripts/Makefile.compiler) checks in per arch/ Makefiles
>>> may fail erroneously because --target was not set for
>>> KBUILD_{C|A}FLAGS yet.
>>>
>>> Another issue is the order of operations between the top level
>>> Makefile and the per arch/ Makefiles.  The `notdir` block you
>>> reference occurs earlier than the per-arch includes:
>>>
>>>   609 TENTATIVE_CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
>>> ...
>>>   648 include $(srctree)/arch/$(SRCARCH)/Makefile
>>>
>>> We would need the opposite order to do what you describe. Reordering
>>> these would effectively be a revert of
>>> commit ae6b289a3789 ("kbuild: Set KBUILD_CFLAGS before incl. arch Makefile")
>>> which I'm not sure we want to do.  But maybe there's another way I'm
>>> not seeing yet?
>>
>> Is there any reason we cannot just add this sort of logic to the main
>> Makefile?
>>
>> Such as (indentation to emphasis diff):
>>
>> ifeq ($(CROSS_COMPILE),)
>> ifneq ($(LLVM),)
>> ifeq ($(LLVM_IAS),1)
>>          ifeq ($(ARCH),arm64)
>>                  TENTATIVE_CLANG_FLAGS   += --target=aarch64-linux
>>          else ifeq ($(ARCH),s390)
>>                  TENTATIVE_CLANG_FLAGS   += --target=s390x-linux
>>          else ifeq ($(ARCH),x86_64)
>>                  TENTATIVE_CLANG_FLAGS   += --target=x86_64-linux
>>          else
>>                  $(error Specify CROSS_COMPILE or add '--target=' option to Makefile)
>>          endif
>> endif
>> endif
>> else
>> TENTATIVE_CLANG_FLAGS   += --target=$(notdir $(CROSS_COMPILE:%-=%))
>> ifeq ($(LLVM_IAS),1)
>> TENTATIVE_CLANG_FLAGS   += -integrated-as
>> else
>> TENTATIVE_CLANG_FLAGS   += -no-integrated-as
>> GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
>> TENTATIVE_CLANG_FLAGS   += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
>> endif
>> endif
>>
>> I know this looks a little cumbersome but it does help us avoid
>> duplication across architecture Makefiles and ordering dependencies.
> 
> Yeah, ok.
> 
> I like the use of `include` to compartmentalize the top level Makefile
> further.  We can move this whole block of LLVM related flag handling
> into something under scripts, then add this block and it doesn't look
> too bad IMO.  Masahiro, are you ok with that?  If so, I'd break this
> into 2 patches:
> 1. moving this block of existing code into a new file.
> 2. adding the CROSS_COMPILE functionality.
> 
> See https://groups.google.com/g/clang-built-linux/c/s-voh6WQFxM for
> the gist of what I was thinking (though not broken into 2 patches yet,
> just testing that it works; it does).

Yeah, I think that looks okay. Not sure how I feel about the name since 
it is handling more than just the target triple but that is a bikeshed 
for another time :)

> This approach will collide with Miguel's series in -next.  Should I
> base the patches on mainline, or linux-kbuild, then have Miguel rebase
> his patches on that or what?

Yes, the patches should be based on mainline or linux-kbuild then Miguel 
will have to solve the conflicts and let Stephen Rothwell know about 
them so that -next keeps working.

Cheers,
Nathan

Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1

[Nick Desaulniers]

On Wed, Jul 7, 2021 at 12:08 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On 7/7/2021 12:04 PM, Nick Desaulniers wrote:
> > On Sat, Jul 3, 2021 at 5:47 PM Nathan Chancellor <nathan@kernel.org> wrote:
> >>
> >> On Fri, Jul 02, 2021 at 11:29:31AM -0700, Nick Desaulniers wrote:
> >>> On Fri, Jul 2, 2021 at 4:59 AM Arnd Bergmann <arnd@kernel.org> wrote:
> >>>>
> >>>> On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
> >>>> Linux <clang-built-linux@googlegroups.com> wrote:
> >>>>>
> >>>>> +ifneq ($(LLVM),)
> >>>>> +ifneq ($(LLVM_IAS),)
> >>>>> +ifeq ($(CROSS_COMPILE),)
> >>>>> +CLANG_TARGET   :=--target=aarch64-linux
> >>>>> +CLANG_FLAGS    += $(CLANG_TARGET)
> >>>>> +KBUILD_CFLAGS  += $(CLANG_TARGET)
> >>>>> +KBUILD_AFLAGS  += $(CLANG_TARGET)
> >>>>> +endif
> >>>>> +endif
> >>>>> +endif
> >>>>
> >>>> I think only the "CLANG_TARGET   :=--target=aarch64-linux" line should
> >>>> go into the
> >>>> per-architecture Makefile. It doesn't hurt to just set that
> >>>> unconditionally here,
> >>>> and then change the CLANG_FLAGS logic in the top-level Makefile to use this
> >>>> in place of $(notdir $(CROSS_COMPILE:%-=%)).
> >>>
> >>> I don't think we can do that. Based on the order the arch/ specific
> >>> Makefiles are included, if we don't eagerly add --target to the
> >>> KBUILD_{C|A}FLAGS, then cc-option, as-option, and as-instr macros
> >>> (defined in scripts/Makefile.compiler) checks in per arch/ Makefiles
> >>> may fail erroneously because --target was not set for
> >>> KBUILD_{C|A}FLAGS yet.
> >>>
> >>> Another issue is the order of operations between the top level
> >>> Makefile and the per arch/ Makefiles.  The `notdir` block you
> >>> reference occurs earlier than the per-arch includes:
> >>>
> >>>   609 TENTATIVE_CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> >>> ...
> >>>   648 include $(srctree)/arch/$(SRCARCH)/Makefile
> >>>
> >>> We would need the opposite order to do what you describe. Reordering
> >>> these would effectively be a revert of
> >>> commit ae6b289a3789 ("kbuild: Set KBUILD_CFLAGS before incl. arch Makefile")
> >>> which I'm not sure we want to do.  But maybe there's another way I'm
> >>> not seeing yet?
> >>
> >> Is there any reason we cannot just add this sort of logic to the main
> >> Makefile?
> >>
> >> Such as (indentation to emphasis diff):
> >>
> >> ifeq ($(CROSS_COMPILE),)
> >> ifneq ($(LLVM),)
> >> ifeq ($(LLVM_IAS),1)
> >>          ifeq ($(ARCH),arm64)
> >>                  TENTATIVE_CLANG_FLAGS   += --target=aarch64-linux
> >>          else ifeq ($(ARCH),s390)
> >>                  TENTATIVE_CLANG_FLAGS   += --target=s390x-linux
> >>          else ifeq ($(ARCH),x86_64)
> >>                  TENTATIVE_CLANG_FLAGS   += --target=x86_64-linux
> >>          else
> >>                  $(error Specify CROSS_COMPILE or add '--target=' option to Makefile)
> >>          endif
> >> endif
> >> endif
> >> else
> >> TENTATIVE_CLANG_FLAGS   += --target=$(notdir $(CROSS_COMPILE:%-=%))
> >> ifeq ($(LLVM_IAS),1)
> >> TENTATIVE_CLANG_FLAGS   += -integrated-as
> >> else
> >> TENTATIVE_CLANG_FLAGS   += -no-integrated-as
> >> GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> >> TENTATIVE_CLANG_FLAGS   += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> >> endif
> >> endif
> >>
> >> I know this looks a little cumbersome but it does help us avoid
> >> duplication across architecture Makefiles and ordering dependencies.
> >
> > Yeah, ok.
> >
> > I like the use of `include` to compartmentalize the top level Makefile
> > further.  We can move this whole block of LLVM related flag handling
> > into something under scripts, then add this block and it doesn't look
> > too bad IMO.  Masahiro, are you ok with that?  If so, I'd break this
> > into 2 patches:
> > 1. moving this block of existing code into a new file.
> > 2. adding the CROSS_COMPILE functionality.
> >
> > See https://groups.google.com/g/clang-built-linux/c/s-voh6WQFxM for
> > the gist of what I was thinking (though not broken into 2 patches yet,
> > just testing that it works; it does).
>
> Yeah, I think that looks okay. Not sure how I feel about the name since
> it is handling more than just the target triple but that is a bikeshed
> for another time :)
>
> > This approach will collide with Miguel's series in -next.  Should I
> > base the patches on mainline, or linux-kbuild, then have Miguel rebase
> > his patches on that or what?
>
> Yes, the patches should be based on mainline or linux-kbuild then Miguel
> will have to solve the conflicts and let Stephen Rothwell know about
> them so that -next keeps working.

Folks can find the new thread for v1:
https://lore.kernel.org/lkml/20210707224310.1403944-1-ndesaulniers@google.com/
if interested.

-- 
Thanks,
~Nick Desaulniers