Re: [PATCH v1 2/2] RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only

From: Nathan Chancellor <nathan@kernel.org>
To: Conor Dooley <conor@kernel.org>
Cc: palmer@dabbelt.com, Conor Dooley <conor.dooley@microchip.com>,
	naresh.kamboju@linaro.org, linux-riscv@lists.infradead.org,
	llvm@lists.linux.dev, stable@vger.kernel.org
Subject: Re: [PATCH v1 2/2] RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only
Date: Fri, 24 Feb 2023 09:32:27 -0700	[thread overview]
Message-ID: <Y/jmm0Q7ypzkUaXw@dev-arch.thelio-3990X> (raw)
In-Reply-To: <20230223220546.52879-3-conor@kernel.org>

Hi Conor,

On Thu, Feb 23, 2023 at 10:05:46PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Quoting the llvm docs:
> > Between versions 2.0 and 2.1 of the base I specification, a backwards
> > incompatible change was made to remove selected instructions and CSRs
> > from the base ISA. These instructions were grouped into a set of new
> > extensions, but were no longer required by the base ISA. (snip) LLVM
> > currently implements version 2.0 of the base specification. Thus,
> > instructions from these extensions are accepted as part of the base
> > ISA.
> 
> There is therefore no need (at present!) to carry out a $cc-option
> check, and instead just gate presence of zicsr and zifencei in march
> on the version of binutils that commit 6df2a016c0c8 ("riscv: fix build
> with binutils 2.38") highlights as the introduction of the requirement.
> 
> In fact, the status quo creates some issues with mixed llvm/binutils
> builds, specifically building with llvm-17 and ld from binutils-2.35.
> Odd combo you may think, but this is what tuxsuite's debian stable uses
> while testing 5.10 stable kernels as doesn't support LLD.
> 
> CC: stable@vger.kernel.org # needs RISC-V: move zicsr/zifencei spec version check to Kconfi

I think it would be better to drop this comment and just mark patch 1
for stable directly. However, if it remains, 'Kconfi' -> 'Kconfig' :)

> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Link: https://lore.kernel.org/all/CA+G9fYt9T=ELCLaB9byxaLW2Qf4pZcDO=huCA0D8ug2V2+irJQ@mail.gmail.com/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

Thanks for helping getting down to the bottom of this!

> ---
>  arch/riscv/Kconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 4eb0ef8314b3..c6902f4c5650 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -291,8 +291,7 @@ endchoice
>  config TOOLCHAIN_NEEDS_SPEC_20191213
>  	bool
>  	default y
> -	depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zicsr_zifencei)
> -	depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zicsr_zifencei)
> +	depends on AS_IS_GNU && AS_VERSION >= 23800
>  	help
>  	  Newer binutils versions default to ISA spec version 20191213 which
>  	  moves some instructions from the I extension to the Zicsr and Zifencei
> -- 
> 2.39.1
> 