From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bin Meng <bmeng.cn@gmail.com>
Date: Thu, 25 Oct 2018 09:56:04 +0800
Subject: [U-Boot] [PATCH 07/30] riscv: set -march and -mabi based on the
	Kconfig configuration
In-Reply-To: <6b73ad784d6037ba017e741d0f7732f2113c9381.camel@aisec.fraunhofer.de>
References: <20181019220743.15020-1-lukas.auer@aisec.fraunhofer.de>
 <20181019220743.15020-8-lukas.auer@aisec.fraunhofer.de>
 <CAEUhbmU_VPkivVB2PkzjsZRtGyCoG_9T_UoCiiFaMyLoF57eNQ@mail.gmail.com>
 <6b73ad784d6037ba017e741d0f7732f2113c9381.camel@aisec.fraunhofer.de>
Message-ID: <CAEUhbmXd49BFbXm3h0-9epn=ta3Ka0=sCPu+A2s+xkJDb4+L0A@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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