From: Nathan Chancellor <nathan@kernel.org> To: Nick Desaulniers <ndesaulniers@google.com>, Masahiro Yamada <masahiroy@kernel.org>, Miguel Ojeda <ojeda@kernel.org> Cc: Arnd Bergmann <arnd@kernel.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Fangrui Song <maskray@google.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, clang-built-linux <clang-built-linux@googlegroups.com> Subject: Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1 Date: Wed, 7 Jul 2021 12:08:24 -0700 [thread overview] Message-ID: <546f3d11-8580-7f23-875d-ef856913cb6d@kernel.org> (raw) In-Reply-To: <CAKwvOdnNWKDGOEqCg5g0GX=zPJce9gBoCLcYs8nayLA7ud2XPQ@mail.gmail.com> 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
WARNING: multiple messages have this Message-ID (diff)
From: Nathan Chancellor <nathan@kernel.org> To: Nick Desaulniers <ndesaulniers@google.com>, Masahiro Yamada <masahiroy@kernel.org>, Miguel Ojeda <ojeda@kernel.org> Cc: Arnd Bergmann <arnd@kernel.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Fangrui Song <maskray@google.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, clang-built-linux <clang-built-linux@googlegroups.com> Subject: Re: [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1 Date: Wed, 7 Jul 2021 12:08:24 -0700 [thread overview] Message-ID: <546f3d11-8580-7f23-875d-ef856913cb6d@kernel.org> (raw) In-Reply-To: <CAKwvOdnNWKDGOEqCg5g0GX=zPJce9gBoCLcYs8nayLA7ud2XPQ@mail.gmail.com> 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-07-07 19:08 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-07-01 23:55 [PATCH] arm64: drop CROSS_COMPILE for LLVM=1 LLVM_IAS=1 Nick Desaulniers 2021-07-01 23:55 ` Nick Desaulniers 2021-07-02 1:05 ` Tom Stellard 2021-07-02 1:05 ` Tom Stellard 2021-07-02 17:37 ` Nick Desaulniers 2021-07-02 17:37 ` Nick Desaulniers 2021-07-02 11:22 ` Will Deacon 2021-07-02 11:22 ` Will Deacon 2021-07-02 17:50 ` Nick Desaulniers 2021-07-02 17:50 ` Nick Desaulniers 2021-07-02 11:59 ` Arnd Bergmann 2021-07-02 11:59 ` Arnd Bergmann 2021-07-02 18:29 ` Nick Desaulniers 2021-07-02 18:29 ` Nick Desaulniers 2021-07-04 0:47 ` Nathan Chancellor 2021-07-04 0:47 ` Nathan Chancellor 2021-07-07 19:04 ` Nick Desaulniers 2021-07-07 19:04 ` Nick Desaulniers 2021-07-07 19:08 ` Nathan Chancellor [this message] 2021-07-07 19:08 ` Nathan Chancellor 2021-07-07 22:44 ` Nick Desaulniers 2021-07-07 22:44 ` Nick Desaulniers
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=546f3d11-8580-7f23-875d-ef856913cb6d@kernel.org \ --to=nathan@kernel.org \ --cc=arnd@kernel.org \ --cc=catalin.marinas@arm.com \ --cc=clang-built-linux@googlegroups.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=masahiroy@kernel.org \ --cc=maskray@google.com \ --cc=ndesaulniers@google.com \ --cc=ojeda@kernel.org \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.