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 <[email protected]>
Suggested-by: Fangrui Song <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
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
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 <[email protected]>
> Suggested-by: Fangrui Song <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> 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)); \
>
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 <[email protected]>
> Suggested-by: Fangrui Song <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> 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
On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
Linux <[email protected]> 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
On Thu, Jul 1, 2021 at 6:05 PM Tom Stellard <[email protected]> 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
On Fri, Jul 2, 2021 at 4:22 AM Will Deacon <[email protected]> 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
On Fri, Jul 2, 2021 at 4:59 AM Arnd Bergmann <[email protected]> wrote:
>
> On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
> Linux <[email protected]> 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
On Fri, Jul 02, 2021 at 11:29:31AM -0700, Nick Desaulniers wrote:
> On Fri, Jul 2, 2021 at 4:59 AM Arnd Bergmann <[email protected]> wrote:
> >
> > On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
> > Linux <[email protected]> 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
On 7/7/2021 12:04 PM, Nick Desaulniers wrote:
> On Sat, Jul 3, 2021 at 5:47 PM Nathan Chancellor <[email protected]> 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 <[email protected]> wrote:
>>>>
>>>> On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
>>>> Linux <[email protected]> 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
On Sat, Jul 3, 2021 at 5:47 PM Nathan Chancellor <[email protected]> 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 <[email protected]> wrote:
> > >
> > > On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
> > > Linux <[email protected]> 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
On Wed, Jul 7, 2021 at 12:08 PM Nathan Chancellor <[email protected]> wrote:
>
> On 7/7/2021 12:04 PM, Nick Desaulniers wrote:
> > On Sat, Jul 3, 2021 at 5:47 PM Nathan Chancellor <[email protected]> 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 <[email protected]> wrote:
> >>>>
> >>>> On Fri, Jul 2, 2021 at 1:55 AM 'Nick Desaulniers' via Clang Built
> >>>> Linux <[email protected]> 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/[email protected]/
if interested.
--
Thanks,
~Nick Desaulniers