[U-Boot] [PATCH] riscv: add Kconfig entries for the F and D ISA extensions support

[Bin Meng]

Hi Eric,

On Wed, May 22, 2019 at 4:23 PM <tesheng at andestech.com> wrote:
>
> Hi Bin,
>
> > -----Original Message-----
> > From: Bin Meng [mailto:bmeng.cn at gmail.com]
> > Sent: Tuesday, May 21, 2019 3:56 PM
> > To: Eric Te-Sheng Lin(林德晟)
> > Cc: U-Boot Mailing List; Lukas Auer; Anup Patel; Rick Jian-Zhi Chen(陳建志);
> > Greentime Ying-Han Hu(胡英漢); dslin1010 at gmail.com
> > Subject: Re: [PATCH] riscv: add Kconfig entries for the F and D ISA extensions
> > support
> >
> > Hi Eric,
> >
> > On Tue, May 21, 2019 at 3:18 PM Eric Lin <tesheng at andestech.com> wrote:
> > >
> > > This patch add Kconfig entries for the F (Single-Precision)
> >
> > adds
> >
>
> OK I'll correct it as adds
>
> > > and D (Double-Precision) floating point instruction-set extensions.
> > >
> >
> > Could you please provide reason that why U-Boot has to be compiled using F/D
> > extension?
> >
>
> Cause on AE350 platform, we have two different kinds of toolchain v5d (support I/M/A/C/F/D ISA) and
> v5 (support I/M/A/C ISA). If we use the v5d toolchain to build U-Boot it will build fail, so we would like to add F/D extension on U-Boot.

I don't understand. What difference do these two toochains have? Isn't
the v5d toolchain's default -march string be pre-configured to imafd?
But even if the toolchain is pre-configured to generate fd
instruction, I think it can be override by the compiler flags. Can you
please share the details of the toolchain you used? I suspect you have
to fix your toolchain, not U-Boot.

>
> > > Signed-off-by: Eric Lin <tesheng at andestech.com>
> > > ---
> > >  arch/riscv/Kconfig  |  8 ++++++++
> > >  arch/riscv/Makefile | 12 ++++++++----
> > >  2 files changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index
> > > 362f3cdc65..a8031fa230 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -91,6 +91,14 @@ config RISCV_ISA_C
> > >           when building U-Boot, which results in compressed instructions
> > in the
> > >           U-Boot binary.
> > >
> > > +config RISCV_ISA_F
> > > +       bool "Emit Floating-point instructions"
> > > +       default n
> >
> > this can be dropped as default is n
>
> OK, I'll drop it
>
> > > +       help
> > > +         Adds "F" to the ISA subsets that the toolchain is allowed to emit
> > > +         when building U-Boot, which results in Single and
> > > + Double-precision instructions
> >
> > This does not match what the config name says. The config name is for F and it
> > cannot indicate both F and D here.
> >
>
> OK, I'll correct it as follows to cover F and D:
> config RISCV_ISA_FD
>     bool "Emit Floating-point instructions"
>     help
>       Adds "F" and "D" to the ISA subsets that the toolchain is allowed to emit
>       when building U-Boot, which results in Single and Double-precision instructions
>       in the U-Boot binary.
>
>
> > > +         in the U-Boot binary.
> > > +
> > >  config RISCV_ISA_A
> > >         def_bool y
> > >
> > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index
> > > 0b80eb8d86..87ec0ea4b5 100644
> > > --- a/arch/riscv/Makefile
> > > +++ b/arch/riscv/Makefile
> > > @@ -5,15 +5,19 @@
> > >
> > >  ifeq ($(CONFIG_ARCH_RV64I),y)
> > >         ARCH_BASE = rv64im
> > > -       ABI = lp64
> > > +       ABI := lp64
> > >  endif
> > >  ifeq ($(CONFIG_ARCH_RV32I),y)
> > >         ARCH_BASE = rv32im
> > > -       ABI = ilp32
> > > +       ABI := ilp32
> > >  endif
> > >  ifeq ($(CONFIG_RISCV_ISA_A),y)
> > >         ARCH_A = a
> > >  endif
> > > +ifeq ($(CONFIG_RISCV_ISA_F),y)
> > > +       ARCH_F = fd
>
> ifeq ($(CONFIG_RISCV_ISA_FD),y)
>                   ARCH_FD = fd
>
> > > +       ABI := $(ABI)d
> > > +endif
> > >  ifeq ($(CONFIG_RISCV_ISA_C),y)
> > >         ARCH_C = c
> > >  endif
> > > @@ -24,8 +28,8 @@ ifeq ($(CONFIG_CMODEL_MEDANY),y)
> > >         CMODEL = medany
> > >  endif
> > >
> > > -ARCH_FLAGS = -march=$(ARCH_BASE)$(ARCH_A)$(ARCH_C) -mabi=$(ABI) \
> > > -            -mcmodel=$(CMODEL)
> > > +ARCH_FLAGS = -march=$(ARCH_BASE)$(ARCH_A)$(ARCH_F)$(ARCH_C)
>
> ARCH_FLAGS = -march=$(ARCH_BASE)$(ARCH_A)$(ARCH_FD)$(ARCH_C)
>
> > -mabi=$(ABI) \
> > > +                                -mcmodel=$(CMODEL)
> > >
> > >  PLATFORM_CPPFLAGS      += $(ARCH_FLAGS)
> > >  CFLAGS_EFI             += $(ARCH_FLAGS)


Regards,
Bin