| Message ID | 20201029085840.2862264-1-anup.patel@wdc.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Makefile: Add option to use toolchain default ABI and ISA string | expand |
On Thu, 2020-10-29 at 14:28 +0530, Anup Patel wrote: > When PLATFORM_RISCV_ABI and PLATFORM_RISCV_ISA are not specified, > we force "-mabi=lp64 -march=rv64gc" for RV64 and force "-mabi=ilp32 > -march=rv32gc" for RV32. This can prevent users from using the > toolchain default "-mabi" and "-march" options. > > To allow using toolchain defaults, we add compile-time option > PLATFORM_RISCV_TOOLCHAIN_DEFAULT which when enabled forces the > top-level makefile to use toolchain default ABI and ISA string. > > To enable the option, pass "PLATFORM_RISCV_TOOLCHAIN_DEFAULT=1" > to top-level make. > > Reported-by: Alistair Francis <alistair.francis@wdc.com> > Signed-off-by: Anup Patel <anup.patel@wdc.com> Thanks! Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > Makefile | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/Makefile b/Makefile > index a231881..d6f097d 100644 > --- a/Makefile > +++ b/Makefile > @@ -94,6 +94,8 @@ DTC = dtc > > # Guess the compillers xlen > OPENSBI_CC_XLEN := $(shell TMP=`$(CC) -dumpmachine | sed > 's/riscv\([0-9][0-9]\).*/\1/'`; echo $${TMP}) > +OPENSBI_CC_ABI := $(shell TMP=`$(CC) -v 2>&1 | sed -n 's/.*\(with\- > abi=\([a-zA-Z0-9]*\)\).*/\2/p'`; echo $${TMP}) > +OPENSBI_CC_ISA := $(shell TMP=`$(CC) -v 2>&1 | sed -n 's/.*\(with\- > arch=\([a-zA-Z0-9]*\)\).*/\2/p'`; echo $${TMP}) > > # Setup platform XLEN > ifndef PLATFORM_RISCV_XLEN > @@ -143,14 +145,22 @@ deps-y+=$(firmware-objs-path-y:.o=.dep) > > # Setup platform ABI, ISA and Code Model > ifndef PLATFORM_RISCV_ABI > - ifeq ($(PLATFORM_RISCV_XLEN), 32) > - PLATFORM_RISCV_ABI = ilp$(PLATFORM_RISCV_XLEN) > + ifneq ($(PLATFORM_RISCV_TOOLCHAIN_DEFAULT), 1) > + ifeq ($(PLATFORM_RISCV_XLEN), 32) > + PLATFORM_RISCV_ABI = ilp$(PLATFORM_RISCV_XLEN) > + else > + PLATFORM_RISCV_ABI = lp$(PLATFORM_RISCV_XLEN) > + endif > else > - PLATFORM_RISCV_ABI = lp$(PLATFORM_RISCV_XLEN) > + PLATFORM_RISCV_ABI = $(OPENSBI_CC_ABI) > endif > endif > ifndef PLATFORM_RISCV_ISA > - PLATFORM_RISCV_ISA = rv$(PLATFORM_RISCV_XLEN)imafdc > + ifneq ($(PLATFORM_RISCV_TOOLCHAIN_DEFAULT), 1) > + PLATFORM_RISCV_ISA = rv$(PLATFORM_RISCV_XLEN)imafdc > + else > + PLATFORM_RISCV_ISA = $(OPENSBI_CC_ISA) > + endif > endif > ifndef PLATFORM_RISCV_CODE_MODEL > PLATFORM_RISCV_CODE_MODEL = medany
On 29 Oct 2020, at 17:08, Alistair Francis <Alistair.Francis@wdc.com> wrote: > > On Thu, 2020-10-29 at 14:28 +0530, Anup Patel wrote: >> When PLATFORM_RISCV_ABI and PLATFORM_RISCV_ISA are not specified, >> we force "-mabi=lp64 -march=rv64gc" for RV64 and force "-mabi=ilp32 >> -march=rv32gc" for RV32. This can prevent users from using the >> toolchain default "-mabi" and "-march" options. >> >> To allow using toolchain defaults, we add compile-time option >> PLATFORM_RISCV_TOOLCHAIN_DEFAULT which when enabled forces the >> top-level makefile to use toolchain default ABI and ISA string. >> >> To enable the option, pass "PLATFORM_RISCV_TOOLCHAIN_DEFAULT=1" >> to top-level make. >> >> Reported-by: Alistair Francis <alistair.francis@wdc.com> >> Signed-off-by: Anup Patel <anup.patel@wdc.com> > > Thanks! > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> FYI, --with-arch/--with-abi are GCC-specific configure flags. Clang doesn't have such a notion, and even if it did it's using CMake so those would be differently-named. Though I don't understand why we need to probe to see what they are; if you want the default, just don't specify -march/-mabi? Seems a bit unnecessary to go and ask what the default is only to then explicitly give it to the tool you just asked. Jess
> -----Original Message----- > From: Jessica Clarke <jrtc27@jrtc27.com> > Sent: 30 October 2020 03:28 > To: Alistair Francis <Alistair.Francis@wdc.com> > Cc: Atish Patra <Atish.Patra@wdc.com>; Anup Patel > <Anup.Patel@wdc.com>; anup@brainfault.org; opensbi@lists.infradead.org > Subject: Re: [PATCH] Makefile: Add option to use toolchain default ABI and > ISA string > > On 29 Oct 2020, at 17:08, Alistair Francis <Alistair.Francis@wdc.com> wrote: > > > > On Thu, 2020-10-29 at 14:28 +0530, Anup Patel wrote: > >> When PLATFORM_RISCV_ABI and PLATFORM_RISCV_ISA are not > specified, we > >> force "-mabi=lp64 -march=rv64gc" for RV64 and force "-mabi=ilp32 > >> -march=rv32gc" for RV32. This can prevent users from using the > >> toolchain default "-mabi" and "-march" options. > >> > >> To allow using toolchain defaults, we add compile-time option > >> PLATFORM_RISCV_TOOLCHAIN_DEFAULT which when enabled forces the > >> top-level makefile to use toolchain default ABI and ISA string. > >> > >> To enable the option, pass "PLATFORM_RISCV_TOOLCHAIN_DEFAULT=1" > >> to top-level make. > >> > >> Reported-by: Alistair Francis <alistair.francis@wdc.com> > >> Signed-off-by: Anup Patel <anup.patel@wdc.com> > > > > Thanks! > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > FYI, --with-arch/--with-abi are GCC-specific configure flags. Clang doesn't > have such a notion, and even if it did it's using CMake so those would be > differently-named. Though I don't understand why we need to probe to see > what they are; if you want the default, just don't specify -march/-mabi? > Seems a bit unnecessary to go and ask what the default is only to then > explicitly give it to the tool you just asked. Okay, sounds good to me. Let's just skip specifying "-mabi/-march" when "PLATFORM_RISCV_TOOLCHAIN_DEFAULT=1" Regards, Anup
> -----Original Message----- > From: Anup Patel > Sent: 02 November 2020 09:04 > To: Jessica Clarke <jrtc27@jrtc27.com>; Alistair Francis > <Alistair.Francis@wdc.com> > Cc: Atish Patra <Atish.Patra@wdc.com>; anup@brainfault.org; > opensbi@lists.infradead.org > Subject: RE: [PATCH] Makefile: Add option to use toolchain default ABI and > ISA string > > > > > -----Original Message----- > > From: Jessica Clarke <jrtc27@jrtc27.com> > > Sent: 30 October 2020 03:28 > > To: Alistair Francis <Alistair.Francis@wdc.com> > > Cc: Atish Patra <Atish.Patra@wdc.com>; Anup Patel > > <Anup.Patel@wdc.com>; anup@brainfault.org; > opensbi@lists.infradead.org > > Subject: Re: [PATCH] Makefile: Add option to use toolchain default ABI > > and ISA string > > > > On 29 Oct 2020, at 17:08, Alistair Francis <Alistair.Francis@wdc.com> wrote: > > > > > > On Thu, 2020-10-29 at 14:28 +0530, Anup Patel wrote: > > >> When PLATFORM_RISCV_ABI and PLATFORM_RISCV_ISA are not > > specified, we > > >> force "-mabi=lp64 -march=rv64gc" for RV64 and force "-mabi=ilp32 > > >> -march=rv32gc" for RV32. This can prevent users from using the > > >> toolchain default "-mabi" and "-march" options. > > >> > > >> To allow using toolchain defaults, we add compile-time option > > >> PLATFORM_RISCV_TOOLCHAIN_DEFAULT which when enabled forces > the > > >> top-level makefile to use toolchain default ABI and ISA string. > > >> > > >> To enable the option, pass > "PLATFORM_RISCV_TOOLCHAIN_DEFAULT=1" > > >> to top-level make. > > >> > > >> Reported-by: Alistair Francis <alistair.francis@wdc.com> > > >> Signed-off-by: Anup Patel <anup.patel@wdc.com> > > > > > > Thanks! > > > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > > FYI, --with-arch/--with-abi are GCC-specific configure flags. Clang > > doesn't have such a notion, and even if it did it's using CMake so > > those would be differently-named. Though I don't understand why we > > need to probe to see what they are; if you want the default, just don't > specify -march/-mabi? > > Seems a bit unnecessary to go and ask what the default is only to then > > explicitly give it to the tool you just asked. > > Okay, sounds good to me. Let's just skip specifying "-mabi/-march" when > "PLATFORM_RISCV_TOOLCHAIN_DEFAULT=1" I tried this approach but it breaks "make install" because OpenSBI installs Library and firmware in a sub-directory named based on PLATFORM_RISCV_ABI Let's stay with current approach of guessing default ABI and ISA from GCC configure flags. For Clang, we will extend OPENSBI_CC_xyz guess work to first test which compiler it is GCC/Clang and then based on that guess OPENSBI_CC_xyz accordingly. Regards, Anup
> -----Original Message----- > From: Alistair Francis <Alistair.Francis@wdc.com> > Sent: 29 October 2020 22:39 > To: Atish Patra <Atish.Patra@wdc.com>; Anup Patel > <Anup.Patel@wdc.com> > Cc: anup@brainfault.org; opensbi@lists.infradead.org > Subject: Re: [PATCH] Makefile: Add option to use toolchain default ABI and > ISA string > > On Thu, 2020-10-29 at 14:28 +0530, Anup Patel wrote: > > When PLATFORM_RISCV_ABI and PLATFORM_RISCV_ISA are not specified, > we > > force "-mabi=lp64 -march=rv64gc" for RV64 and force "-mabi=ilp32 > > -march=rv32gc" for RV32. This can prevent users from using the > > toolchain default "-mabi" and "-march" options. > > > > To allow using toolchain defaults, we add compile-time option > > PLATFORM_RISCV_TOOLCHAIN_DEFAULT which when enabled forces the > > top-level makefile to use toolchain default ABI and ISA string. > > > > To enable the option, pass "PLATFORM_RISCV_TOOLCHAIN_DEFAULT=1" > > to top-level make. > > > > Reported-by: Alistair Francis <alistair.francis@wdc.com> > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > Thanks! > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > Alistair > > > --- > > Makefile | 18 ++++++++++++++---- > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index a231881..d6f097d 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -94,6 +94,8 @@ DTC = dtc > > > > # Guess the compillers xlen > > OPENSBI_CC_XLEN := $(shell TMP=`$(CC) -dumpmachine | sed > > 's/riscv\([0-9][0-9]\).*/\1/'`; echo $${TMP}) > > +OPENSBI_CC_ABI := $(shell TMP=`$(CC) -v 2>&1 | sed -n 's/.*\(with\- > > abi=\([a-zA-Z0-9]*\)\).*/\2/p'`; echo $${TMP}) > > +OPENSBI_CC_ISA := $(shell TMP=`$(CC) -v 2>&1 | sed -n 's/.*\(with\- > > arch=\([a-zA-Z0-9]*\)\).*/\2/p'`; echo $${TMP}) > > > > # Setup platform XLEN > > ifndef PLATFORM_RISCV_XLEN > > @@ -143,14 +145,22 @@ deps-y+=$(firmware-objs-path-y:.o=.dep) > > > > # Setup platform ABI, ISA and Code Model ifndef PLATFORM_RISCV_ABI > > - ifeq ($(PLATFORM_RISCV_XLEN), 32) > > - PLATFORM_RISCV_ABI = ilp$(PLATFORM_RISCV_XLEN) > > + ifneq ($(PLATFORM_RISCV_TOOLCHAIN_DEFAULT), 1) > > + ifeq ($(PLATFORM_RISCV_XLEN), 32) > > + PLATFORM_RISCV_ABI = ilp$(PLATFORM_RISCV_XLEN) > > + else > > + PLATFORM_RISCV_ABI = lp$(PLATFORM_RISCV_XLEN) > > + endif > > else > > - PLATFORM_RISCV_ABI = lp$(PLATFORM_RISCV_XLEN) > > + PLATFORM_RISCV_ABI = $(OPENSBI_CC_ABI) > > endif > > endif > > ifndef PLATFORM_RISCV_ISA > > - PLATFORM_RISCV_ISA = rv$(PLATFORM_RISCV_XLEN)imafdc > > + ifneq ($(PLATFORM_RISCV_TOOLCHAIN_DEFAULT), 1) > > + PLATFORM_RISCV_ISA = rv$(PLATFORM_RISCV_XLEN)imafdc else > > + PLATFORM_RISCV_ISA = $(OPENSBI_CC_ISA) endif > > endif > > ifndef PLATFORM_RISCV_CODE_MODEL > > PLATFORM_RISCV_CODE_MODEL = medany Applied this patch to the riscv/opensbi repo Regards, Anup
diff --git a/Makefile b/Makefile index a231881..d6f097d 100644 --- a/Makefile +++ b/Makefile @@ -94,6 +94,8 @@ DTC = dtc # Guess the compillers xlen OPENSBI_CC_XLEN := $(shell TMP=`$(CC) -dumpmachine | sed 's/riscv\([0-9][0-9]\).*/\1/'`; echo $${TMP}) +OPENSBI_CC_ABI := $(shell TMP=`$(CC) -v 2>&1 | sed -n 's/.*\(with\-abi=\([a-zA-Z0-9]*\)\).*/\2/p'`; echo $${TMP}) +OPENSBI_CC_ISA := $(shell TMP=`$(CC) -v 2>&1 | sed -n 's/.*\(with\-arch=\([a-zA-Z0-9]*\)\).*/\2/p'`; echo $${TMP}) # Setup platform XLEN ifndef PLATFORM_RISCV_XLEN @@ -143,14 +145,22 @@ deps-y+=$(firmware-objs-path-y:.o=.dep) # Setup platform ABI, ISA and Code Model ifndef PLATFORM_RISCV_ABI - ifeq ($(PLATFORM_RISCV_XLEN), 32) - PLATFORM_RISCV_ABI = ilp$(PLATFORM_RISCV_XLEN) + ifneq ($(PLATFORM_RISCV_TOOLCHAIN_DEFAULT), 1) + ifeq ($(PLATFORM_RISCV_XLEN), 32) + PLATFORM_RISCV_ABI = ilp$(PLATFORM_RISCV_XLEN) + else + PLATFORM_RISCV_ABI = lp$(PLATFORM_RISCV_XLEN) + endif else - PLATFORM_RISCV_ABI = lp$(PLATFORM_RISCV_XLEN) + PLATFORM_RISCV_ABI = $(OPENSBI_CC_ABI) endif endif ifndef PLATFORM_RISCV_ISA - PLATFORM_RISCV_ISA = rv$(PLATFORM_RISCV_XLEN)imafdc + ifneq ($(PLATFORM_RISCV_TOOLCHAIN_DEFAULT), 1) + PLATFORM_RISCV_ISA = rv$(PLATFORM_RISCV_XLEN)imafdc + else + PLATFORM_RISCV_ISA = $(OPENSBI_CC_ISA) + endif endif ifndef PLATFORM_RISCV_CODE_MODEL PLATFORM_RISCV_CODE_MODEL = medany
When PLATFORM_RISCV_ABI and PLATFORM_RISCV_ISA are not specified, we force "-mabi=lp64 -march=rv64gc" for RV64 and force "-mabi=ilp32 -march=rv32gc" for RV32. This can prevent users from using the toolchain default "-mabi" and "-march" options. To allow using toolchain defaults, we add compile-time option PLATFORM_RISCV_TOOLCHAIN_DEFAULT which when enabled forces the top-level makefile to use toolchain default ABI and ISA string. To enable the option, pass "PLATFORM_RISCV_TOOLCHAIN_DEFAULT=1" to top-level make. Reported-by: Alistair Francis <alistair.francis@wdc.com> Signed-off-by: Anup Patel <anup.patel@wdc.com> --- Makefile | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)