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

[Eric Lin]

Hi Bin

Bin Meng <bmeng.cn at gmail.com> 於 2019年5月22日 週三 下午5:25寫道：
>
> 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.
>

It's seems the ABI issue. Because our toolchain don't support
multilib, the v5d toolchain libraries ABI is ilp64d.
If I use the v5d toolchain to build U-Boot with -mabi=lp64, it will
get link error as below:
...
riscv64-linux-ld.bfd:
/nds64le-linux-glibc-v5d/bin/../lib/gcc/riscv64-linux/7.3.0/libgcc.a(save-restore.o):
can't link double-float modules with soft-float modules
riscv64-linux-ld.bfd: failed to merge target specific data of file
/nds64le-linux-glibc-v5d/bin/../lib/gcc/riscv64-linux/7.3.0/libgcc.a(save-restore.o)
examples/standalone/Makefile:62: recipe for target
'examples/standalone/hello_world' failed
...

so we would like to override the compiler flag mabi to lp64d.

> >
> > > > 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

Thanks for your review
Eric