Message ID | 20181019220743.15020-8-lukas.auer@aisec.fraunhofer.de |
---|---|
State | Superseded |
Delegated to: | Andes |
Headers | show |
Series | General fixes / cleanup for RISC-V and improvements to qemu-riscv | expand |
Hi Lukas, On Sat, Oct 20, 2018 at 6:09 AM Lukas Auer <lukas.auer@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.auer@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 <rick@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) > 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
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.auer@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.auer@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 <rick@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
Hi Lukas, On Wed, Oct 24, 2018 at 11:57 PM Auer, Lukas <lukas.auer@aisec.fraunhofer.de> wrote: > > 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.auer@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.auer@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 <rick@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? > This looks good. Thanks! > 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) > Regards, Bin
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 <rick@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) + 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
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.auer@aisec.fraunhofer.de> --- arch/riscv/Makefile | 13 +++++++++++++ arch/riscv/config.mk | 4 ---- 2 files changed, 13 insertions(+), 4 deletions(-)