Re: [PATCH 1/1] riscv: support building double-float modules

[Leo Liang <ycliang@andestech.com>]

Hi Heinrich,

On Sat, Oct 08, 2022 at 11:17:57AM +0200, Heinrich Schuchardt wrote:
> The riscv32 toolchain for GCC-12 provided by kernel.org contains libgcc.a
> compiled for double-float. To link to it we have to adjust how we build
> U-Boot.
> 
> As U-Boot actually does not use floating point at all this should not
> make a significant difference for the produced binaries.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  arch/riscv/Kconfig  | 14 ++++++++++++++
>  arch/riscv/Makefile | 17 ++++++++++++++---
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 32a90b83b5..8add95c8ef 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -152,6 +152,20 @@ config RISCV_ISA_C
>  	  when building U-Boot, which results in compressed instructions in the
>  	  U-Boot binary.
>  
> +config RISCV_ISA_F
> +	bool "Standard extension for Single-Precision Floating Point"
> +	default y
> +	help
> +	  Adds "F" to the ISA string passed to the compiler.
> +
> +config RISCV_ISA_D
> +	bool "Standard extension forr Double-Precision Floating Point"

typo "forr"

> +	depends on RISCV_ISA_F
> +	default y
> +	help
> +	  Adds "D" to the ISA string passed to the compiler and changes the
> +	  riscv32 ABI from ilp32 to ipl32d.

typo "ipl32d"

I think we do not need to specify that the option is created for rv32.

> +
>  config RISCV_ISA_A
>  	def_bool y
>  
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 53d1194ffb..d1b6e86dd8 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -5,15 +5,24 @@
>  
>  ifeq ($(CONFIG_ARCH_RV64I),y)
>  	ARCH_BASE = rv64im
> -	ABI = lp64
> +	ABI_BASE = lp64
>  endif
>  ifeq ($(CONFIG_ARCH_RV32I),y)
>  	ARCH_BASE = rv32im
> -	ABI = ilp32
> +	ABI_BASE = ilp32
>  endif
>  ifeq ($(CONFIG_RISCV_ISA_A),y)
>  	ARCH_A = a
>  endif
> +ifeq ($(CONFIG_RISCV_ISA_F),y)
> +	ARCH_F = f
> +endif
> +ifeq ($(CONFIG_RISCV_ISA_D),y)
> +	ARCH_D = d
> +ifeq ($(CONFIG_ARCH_RV32I),y)

I think we do not need this if statement as Rick stated.
The default libgcc.a of both rv32 and rv64 toolchains from kernel.org are built with double float abi,
so this CONFIG_RISCV_ISA_D could be applied to rv64 setup as well.

Other than the above,
Looks good to me.

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

> +	ABI_D = d
> +endif
> +endif
>  ifeq ($(CONFIG_RISCV_ISA_C),y)
>  	ARCH_C = c
>  endif
> @@ -24,7 +33,9 @@ ifeq ($(CONFIG_CMODEL_MEDANY),y)
>  	CMODEL = medany
>  endif
>  
> -RISCV_MARCH = $(ARCH_BASE)$(ARCH_A)$(ARCH_C)
> +
> +RISCV_MARCH = $(ARCH_BASE)$(ARCH_A)$(ARCH_F)$(ARCH_D)$(ARCH_C)
> +ABI = $(ABI_BASE)$(ABI_D)
>  
>  # Newer binutils versions default to ISA spec version 20191213 which moves some
>  # instructions from the I extension to the Zicsr and Zifencei extensions.
> -- 
> 2.37.2
> 

Thanks for catching rv32 compilation issue.

Best regards,
Leo