Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221118003057.3223394-1-nathan@kernel.org>
In-Reply-To: <20221118003057.3223394-1-nathan@kernel.org>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Fri, 18 Nov 2022 09:23:34 +0100
Message-ID: <CAMj1kXHgAZeG9ntSQnuV7Ta+-b9__=Z2y2x0+u3LnSRbJR_FNw@mail.gmail.com>
Subject: Re: [PATCH v2] ARM: Drop '-mthumb' from AFLAGS_ISA
To:     Nathan Chancellor <nathan@kernel.org>
Cc:     Russell King <linux@armlinux.org.uk>,
        Arnd Bergmann <arnd@arndb.de>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        llvm@lists.linux.dev, patches@lists.linux.dev,
        "kernelci.org bot" <bot@kernelci.org>,
        Nick Desaulniers <ndesaulniers@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 18 Nov 2022 at 01:34, Nathan Chancellor <nathan@kernel.org> wrote:
>
> When building with CONFIG_THUMB2_KERNEL=y + a version of clang from
> Debian using CROSS_COMPILE=arm-linux-gnueabihf-, the following warning
> occurs frequently:
>
>   <built-in>:383:9: warning: '__thumb2__' macro redefined [-Wmacro-redefined]
>   #define __thumb2__ 2
>           ^
>   <built-in>:353:9: note: previous definition is here
>   #define __thumb2__ 1
>           ^
>   1 warning generated.
>
> Debian carries a downstream patch that changes the default CPU of the
> arm-linux-gnueabihf target from 'arm1176jzf-s' (v6) to 'cortex-a7' (v7).
> As a result, '-mthumb' defines both '__thumb__' and '__thumb2__'. The
> define of '__thumb2__' via the command line was purposefully added to
> catch a situation like this.
>
> In a similar vein as commit 26b12e084bce ("ARM: 9264/1: only use
> -mtp=cp15 for the compiler"), do not add '-mthumb' to AFLAGS_ISA, as it
> is already passed to the assembler via '-Wa,-mthumb' and '__thumb2__' is
> already defined for preprocessing.
>
> Fixes: 1d2e9b67b001 ("ARM: 9265/1: pass -march= only to compiler")
> Link: https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/raw/622dbcbd40b316ed3905a2d25d9623544a06e6b1/debian/patches/930008-arm.diff
> Reported-by: "kernelci.org bot" <bot@kernelci.org>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

> ---
> v2:
>   * Add Nick's review and test tags.
>   * Call out explicitly that CROSS_COMPILE=arm-linux-gnueabihf- is
>     needed to reproduce this issue in commit message (Nick).
>   * Fix dead link in commit message (Nick).
>   * Use the snapshot branch for the patch link, as the diff is more up
>     to date against upstream LLVM (Nick).
> v1: https://lore.kernel.org/20221114225719.1657174-1-nathan@kernel.org/
>  arch/arm/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 357f0d9b8607..d1ebb746ff40 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -131,8 +131,9 @@ endif
>  AFLAGS_NOWARN  :=$(call as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W)
>
>  ifeq ($(CONFIG_THUMB2_KERNEL),y)
> -CFLAGS_ISA     :=-mthumb -Wa,-mimplicit-it=always $(AFLAGS_NOWARN)
> +CFLAGS_ISA     :=-Wa,-mimplicit-it=always $(AFLAGS_NOWARN)
>  AFLAGS_ISA     :=$(CFLAGS_ISA) -Wa$(comma)-mthumb -D__thumb2__=2
> +CFLAGS_ISA     +=-mthumb
>  else
>  CFLAGS_ISA     :=$(call cc-option,-marm,) $(AFLAGS_NOWARN)
>  AFLAGS_ISA     :=$(CFLAGS_ISA)
>
> base-commit: 0c52591d22e99759da3793f19249bbf45ad742bd
> --
> 2.38.1
>