Hi Bin,
On Mon, 2018-10-22 at 15:21 +0800, Bin Meng wrote:
> Hi Lukas,
>
> On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer
> <lukas.a...@aisec.fraunhofer.de> wrote:
> >
> > Use the new Kconfig entries to construct the ISA string for the
> > -march
> > compiler flag. The -mabi compiler flag is selected based on the
> > base
> > integer instruction set.
> >
> > With this change, the C (compressed instructions) ISA extension is
> > now
> > enabled for all boards with CONFIG_RISCV_ISA_C set. Buildman
> > reports a
> > decrease in binary size of 71590 bytes.
> >
> > Signed-off-by: Lukas Auer <lukas.a...@aisec.fraunhofer.de>
> > ---
> >
> > arch/riscv/Makefile | 13 +++++++++++++
> > arch/riscv/config.mk | 4 ----
> > 2 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index 8fb6a889d8..6fb292d0b4 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -3,6 +3,19 @@
> > # Copyright (C) 2017 Andes Technology Corporation.
> > # Rick Chen, Andes Technology Corporation <r...@andestech.com>
> >
> > +riscv-march-$(CONFIG_ARCH_RV32I) := rv32im
> > +riscv-march-$(CONFIG_ARCH_RV64I) := rv64im
> > +riscv-march-$(CONFIG_RISCV_ISA_A) := $(riscv-march-y)a
> > +riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
> > +
> > +riscv-mabi-$(CONFIG_ARCH_RV64I) := lp64
> > +riscv-mabi-$(CONFIG_ARCH_RV32I) := ilp32
> > +
> > +arch-y := -march=$(riscv-march-y) -mabi=$(riscv-mabi-y)
> > +
> > +PLATFORM_CPPFLAGS += $(arch-y)
> > +CFLAGS_EFI += $(arch-y)
> > +
>
> The concept of this patch is good. However the usage of := is a bit
> odd, since it makes people think the latter will override the former
> one, however it is not.
>
> Can we get rid of these riscv-mach-xxx, instead using something like
> this:
>
> ifeq ($(CONFIG_RISCV_ISA_A),y)
> ARCH_A = a
> endif
> ifeq ($(CONFIG_RISCV_ISA_C),y)
> ARCH_C = c
> endif
>
> ifeq ($(CONFIG_ARCH_RV32I),y)
> BITS = 32
> ABI_I = i
> endif
> ifeq ($(CONFIG_ARCH_RV64I),y)
> BITS = 64
> endif
>
> PLATFORM_CPPFLAGS += -march=rv$(BITS)im$(ARCH_A)$(ARCH_C)
> -mabi=$(ABI_I)lp$(BITS)
>
That makes sense. Yes I can change it to something like that.
One small change I would do is to try and keep any constant ISA / ABI
strings (so rv and im) out of PLATFORM_CPPFLAGS to keep the
configuration more in one place. So something like this. What do you
think?
ifeq ($(CONFIG_ARCH_RV32I),y)
ARCH_BASE = rv32im
ABI = ilp32
endif
ifeq ($(CONFIG_ARCH_RV64I),y)
ARCH_BASE = rv64im
ABI = lp64
endif
PLATFORM_CPPFLAGS += -march=$(ARCH_BASE)$(ARCH_A)$(ARCH_C)
-mabi=$(ABI)
Thanks,
Lukas
> > head-y := arch/riscv/cpu/start.o
> >
> > libs-y += arch/riscv/cpu/
> > diff --git a/arch/riscv/config.mk b/arch/riscv/config.mk
> > index ed9eb0c24c..9088b9ef2c 100644
> > --- a/arch/riscv/config.mk
> > +++ b/arch/riscv/config.mk
> > @@ -14,16 +14,12 @@
> > 64bit-emul := elf64lriscv
> >
> > ifdef CONFIG_32BIT
> > -PLATFORM_CPPFLAGS += -march=rv32ima -mabi=ilp32
> > PLATFORM_LDFLAGS += -m $(32bit-emul)
> > -CFLAGS_EFI += -march=rv32ima -mabi=ilp32
> > EFI_LDS := elf_riscv32_efi.lds
> > endif
> >
> > ifdef CONFIG_64BIT
> > -PLATFORM_CPPFLAGS += -march=rv64ima -mabi=lp64
> > PLATFORM_LDFLAGS += -m $(64bit-emul)
> > -CFLAGS_EFI += -march=rv64ima -mabi=lp64
> > EFI_LDS := elf_riscv64_efi.lds
> > endif
> >
> > --
>
> Regards,
> Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
