All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] RISC-V: avoid build issues for clang/llvm-17 with binutils 2.35
@ 2023-02-23 22:05 ` Conor Dooley
  0 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2023-02-23 22:05 UTC (permalink / raw)
  To: palmer; +Cc: conor, Conor Dooley, nathan, naresh.kamboju, linux-riscv, llvm

From: Conor Dooley <conor.dooley@microchip.com>

Hey,
Here's an attempted (interim?) fix for issues on v5.10 due to the
presence of zifencei & zicsr in object files. I'm seeing another issue
that only appears with ld from binutils-2.35 & not with ld from
binutils-2.37 on riscv/for-next:
/stuff/toolchains/binutils-2.35/bin/riscv64-linux-gnu-ld: .init.data has both ordered [`__patchable_function_entries' in init/main.o] and unordered [`.meminit.data' in mm/sparse.o] sections
/stuff/toolchains/binutils-2.35/bin/riscv64-linux-gnu-ld: final link failed: bad value
But that's far far further on in the build than we were getting before
these patches.

Even if we end up dealing with the emitted strings a different way [1],
I think the first patch here has some value in moving the check to
Kconfig so that it's far easier to tell if the toolchain does need/use
these extensions from someone's build artifacts.

Cheers,
Conor.

1 - https://lore.kernel.org/linux-riscv/20230223195112.10489-1-palmer@rivosinc.com/

cc: nathan@kernel.org
cc: palmer@dabbelt.com
cc: naresh.kamboju@linaro.org
cc: linux-riscv@lists.infradead.org
cc: llvm@lists.linux.dev

Conor Dooley (2):
  RISC-V: move zicsr/zifencei spec version check to Kconfig
  RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only

 arch/riscv/Kconfig  | 9 +++++++++
 arch/riscv/Makefile | 5 +----
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.39.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 0/2] RISC-V: avoid build issues for clang/llvm-17 with binutils 2.35
@ 2023-02-23 22:05 ` Conor Dooley
  0 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2023-02-23 22:05 UTC (permalink / raw)
  To: palmer; +Cc: conor, Conor Dooley, nathan, naresh.kamboju, linux-riscv, llvm

From: Conor Dooley <conor.dooley@microchip.com>

Hey,
Here's an attempted (interim?) fix for issues on v5.10 due to the
presence of zifencei & zicsr in object files. I'm seeing another issue
that only appears with ld from binutils-2.35 & not with ld from
binutils-2.37 on riscv/for-next:
/stuff/toolchains/binutils-2.35/bin/riscv64-linux-gnu-ld: .init.data has both ordered [`__patchable_function_entries' in init/main.o] and unordered [`.meminit.data' in mm/sparse.o] sections
/stuff/toolchains/binutils-2.35/bin/riscv64-linux-gnu-ld: final link failed: bad value
But that's far far further on in the build than we were getting before
these patches.

Even if we end up dealing with the emitted strings a different way [1],
I think the first patch here has some value in moving the check to
Kconfig so that it's far easier to tell if the toolchain does need/use
these extensions from someone's build artifacts.

Cheers,
Conor.

1 - https://lore.kernel.org/linux-riscv/20230223195112.10489-1-palmer@rivosinc.com/

cc: nathan@kernel.org
cc: palmer@dabbelt.com
cc: naresh.kamboju@linaro.org
cc: linux-riscv@lists.infradead.org
cc: llvm@lists.linux.dev

Conor Dooley (2):
  RISC-V: move zicsr/zifencei spec version check to Kconfig
  RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only

 arch/riscv/Kconfig  | 9 +++++++++
 arch/riscv/Makefile | 5 +----
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.39.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
  2023-02-23 22:05 ` Conor Dooley
@ 2023-02-23 22:05   ` Conor Dooley
  -1 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2023-02-23 22:05 UTC (permalink / raw)
  To: palmer; +Cc: conor, Conor Dooley, nathan, naresh.kamboju, linux-riscv, llvm

From: Conor Dooley <conor.dooley@microchip.com>

Checking whether binutils defaults to using a version of the spec
requiring zicsr/zifencei being done in our Makefile is functionally
sufficient at present, but makes it harder to tell after the fact
which extensions are enabled.
By moving it to Kconfig, it's easy to tell from standard build artifacts
what has been done & the road is paved for dealing with this differently
for both binutils and LLVM.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/Kconfig  | 10 ++++++++++
 arch/riscv/Makefile |  5 +----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index cebf0c5f8824..4eb0ef8314b3 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -288,6 +288,16 @@ config ARCH_RV64I
 
 endchoice
 
+config TOOLCHAIN_NEEDS_SPEC_20191213
+	bool
+	default y
+	depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zicsr_zifencei)
+	depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zicsr_zifencei)
+	help
+	  Newer binutils versions default to ISA spec version 20191213 which
+	  moves some instructions from the I extension to the Zicsr and Zifencei
+	  extensions.
+
 # We must be able to map all physical memory into the kernel, but the compiler
 # is still a bit more efficient when generating code if it's setup in a manner
 # such that it can only map 2GiB of memory.
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 12d91b0a73d8..7e1267b0e73f 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -53,10 +53,7 @@ riscv-march-$(CONFIG_ARCH_RV64I)	:= rv64ima
 riscv-march-$(CONFIG_FPU)		:= $(riscv-march-y)fd
 riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
 
-# Newer binutils versions default to ISA spec version 20191213 which moves some
-# instructions from the I extension to the Zicsr and Zifencei extensions.
-toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei)
-riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei
+riscv-march-$(CONFIG_TOOLCHAIN_NEEDS_SPEC_20191213) := $(riscv-march-y)_zicsr_zifencei
 
 # Check if the toolchain supports Zicbom extension
 riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZICBOM) := $(riscv-march-y)_zicbom
-- 
2.39.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
@ 2023-02-23 22:05   ` Conor Dooley
  0 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2023-02-23 22:05 UTC (permalink / raw)
  To: palmer; +Cc: conor, Conor Dooley, nathan, naresh.kamboju, linux-riscv, llvm

From: Conor Dooley <conor.dooley@microchip.com>

Checking whether binutils defaults to using a version of the spec
requiring zicsr/zifencei being done in our Makefile is functionally
sufficient at present, but makes it harder to tell after the fact
which extensions are enabled.
By moving it to Kconfig, it's easy to tell from standard build artifacts
what has been done & the road is paved for dealing with this differently
for both binutils and LLVM.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/Kconfig  | 10 ++++++++++
 arch/riscv/Makefile |  5 +----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index cebf0c5f8824..4eb0ef8314b3 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -288,6 +288,16 @@ config ARCH_RV64I
 
 endchoice
 
+config TOOLCHAIN_NEEDS_SPEC_20191213
+	bool
+	default y
+	depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zicsr_zifencei)
+	depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zicsr_zifencei)
+	help
+	  Newer binutils versions default to ISA spec version 20191213 which
+	  moves some instructions from the I extension to the Zicsr and Zifencei
+	  extensions.
+
 # We must be able to map all physical memory into the kernel, but the compiler
 # is still a bit more efficient when generating code if it's setup in a manner
 # such that it can only map 2GiB of memory.
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 12d91b0a73d8..7e1267b0e73f 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -53,10 +53,7 @@ riscv-march-$(CONFIG_ARCH_RV64I)	:= rv64ima
 riscv-march-$(CONFIG_FPU)		:= $(riscv-march-y)fd
 riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
 
-# Newer binutils versions default to ISA spec version 20191213 which moves some
-# instructions from the I extension to the Zicsr and Zifencei extensions.
-toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei)
-riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei
+riscv-march-$(CONFIG_TOOLCHAIN_NEEDS_SPEC_20191213) := $(riscv-march-y)_zicsr_zifencei
 
 # Check if the toolchain supports Zicbom extension
 riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZICBOM) := $(riscv-march-y)_zicbom
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 2/2] RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only
  2023-02-23 22:05 ` Conor Dooley
@ 2023-02-23 22:05   ` Conor Dooley
  -1 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2023-02-23 22:05 UTC (permalink / raw)
  To: palmer
  Cc: conor, Conor Dooley, nathan, naresh.kamboju, linux-riscv, llvm, stable

From: Conor Dooley <conor.dooley@microchip.com>

Quoting the llvm docs:
> Between versions 2.0 and 2.1 of the base I specification, a backwards
> incompatible change was made to remove selected instructions and CSRs
> from the base ISA. These instructions were grouped into a set of new
> extensions, but were no longer required by the base ISA. (snip) LLVM
> currently implements version 2.0 of the base specification. Thus,
> instructions from these extensions are accepted as part of the base
> ISA.

There is therefore no need (at present!) to carry out a $cc-option
check, and instead just gate presence of zicsr and zifencei in march
on the version of binutils that commit 6df2a016c0c8 ("riscv: fix build
with binutils 2.38") highlights as the introduction of the requirement.

In fact, the status quo creates some issues with mixed llvm/binutils
builds, specifically building with llvm-17 and ld from binutils-2.35.
Odd combo you may think, but this is what tuxsuite's debian stable uses
while testing 5.10 stable kernels as doesn't support LLD.

CC: stable@vger.kernel.org # needs RISC-V: move zicsr/zifencei spec version check to Kconfi
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Link: https://lore.kernel.org/all/CA+G9fYt9T=ELCLaB9byxaLW2Qf4pZcDO=huCA0D8ug2V2+irJQ@mail.gmail.com/
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 4eb0ef8314b3..c6902f4c5650 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -291,8 +291,7 @@ endchoice
 config TOOLCHAIN_NEEDS_SPEC_20191213
 	bool
 	default y
-	depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zicsr_zifencei)
-	depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zicsr_zifencei)
+	depends on AS_IS_GNU && AS_VERSION >= 23800
 	help
 	  Newer binutils versions default to ISA spec version 20191213 which
 	  moves some instructions from the I extension to the Zicsr and Zifencei
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 2/2] RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only
@ 2023-02-23 22:05   ` Conor Dooley
  0 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2023-02-23 22:05 UTC (permalink / raw)
  To: palmer
  Cc: conor, Conor Dooley, nathan, naresh.kamboju, linux-riscv, llvm, stable

From: Conor Dooley <conor.dooley@microchip.com>

Quoting the llvm docs:
> Between versions 2.0 and 2.1 of the base I specification, a backwards
> incompatible change was made to remove selected instructions and CSRs
> from the base ISA. These instructions were grouped into a set of new
> extensions, but were no longer required by the base ISA. (snip) LLVM
> currently implements version 2.0 of the base specification. Thus,
> instructions from these extensions are accepted as part of the base
> ISA.

There is therefore no need (at present!) to carry out a $cc-option
check, and instead just gate presence of zicsr and zifencei in march
on the version of binutils that commit 6df2a016c0c8 ("riscv: fix build
with binutils 2.38") highlights as the introduction of the requirement.

In fact, the status quo creates some issues with mixed llvm/binutils
builds, specifically building with llvm-17 and ld from binutils-2.35.
Odd combo you may think, but this is what tuxsuite's debian stable uses
while testing 5.10 stable kernels as doesn't support LLD.

CC: stable@vger.kernel.org # needs RISC-V: move zicsr/zifencei spec version check to Kconfi
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Link: https://lore.kernel.org/all/CA+G9fYt9T=ELCLaB9byxaLW2Qf4pZcDO=huCA0D8ug2V2+irJQ@mail.gmail.com/
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 4eb0ef8314b3..c6902f4c5650 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -291,8 +291,7 @@ endchoice
 config TOOLCHAIN_NEEDS_SPEC_20191213
 	bool
 	default y
-	depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zicsr_zifencei)
-	depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zicsr_zifencei)
+	depends on AS_IS_GNU && AS_VERSION >= 23800
 	help
 	  Newer binutils versions default to ISA spec version 20191213 which
 	  moves some instructions from the I extension to the Zicsr and Zifencei
-- 
2.39.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/2] RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only
  2023-02-23 22:05   ` Conor Dooley
@ 2023-02-24 16:32     ` Nathan Chancellor
  -1 siblings, 0 replies; 18+ messages in thread
From: Nathan Chancellor @ 2023-02-24 16:32 UTC (permalink / raw)
  To: Conor Dooley
  Cc: palmer, Conor Dooley, naresh.kamboju, linux-riscv, llvm, stable

Hi Conor,

On Thu, Feb 23, 2023 at 10:05:46PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Quoting the llvm docs:
> > Between versions 2.0 and 2.1 of the base I specification, a backwards
> > incompatible change was made to remove selected instructions and CSRs
> > from the base ISA. These instructions were grouped into a set of new
> > extensions, but were no longer required by the base ISA. (snip) LLVM
> > currently implements version 2.0 of the base specification. Thus,
> > instructions from these extensions are accepted as part of the base
> > ISA.
> 
> There is therefore no need (at present!) to carry out a $cc-option
> check, and instead just gate presence of zicsr and zifencei in march
> on the version of binutils that commit 6df2a016c0c8 ("riscv: fix build
> with binutils 2.38") highlights as the introduction of the requirement.
> 
> In fact, the status quo creates some issues with mixed llvm/binutils
> builds, specifically building with llvm-17 and ld from binutils-2.35.
> Odd combo you may think, but this is what tuxsuite's debian stable uses
> while testing 5.10 stable kernels as doesn't support LLD.
> 
> CC: stable@vger.kernel.org # needs RISC-V: move zicsr/zifencei spec version check to Kconfi

I think it would be better to drop this comment and just mark patch 1
for stable directly. However, if it remains, 'Kconfi' -> 'Kconfig' :)

> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Link: https://lore.kernel.org/all/CA+G9fYt9T=ELCLaB9byxaLW2Qf4pZcDO=huCA0D8ug2V2+irJQ@mail.gmail.com/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

Thanks for helping getting down to the bottom of this!

> ---
>  arch/riscv/Kconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 4eb0ef8314b3..c6902f4c5650 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -291,8 +291,7 @@ endchoice
>  config TOOLCHAIN_NEEDS_SPEC_20191213
>  	bool
>  	default y
> -	depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zicsr_zifencei)
> -	depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zicsr_zifencei)
> +	depends on AS_IS_GNU && AS_VERSION >= 23800
>  	help
>  	  Newer binutils versions default to ISA spec version 20191213 which
>  	  moves some instructions from the I extension to the Zicsr and Zifencei
> -- 
> 2.39.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/2] RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only
@ 2023-02-24 16:32     ` Nathan Chancellor
  0 siblings, 0 replies; 18+ messages in thread
From: Nathan Chancellor @ 2023-02-24 16:32 UTC (permalink / raw)
  To: Conor Dooley
  Cc: palmer, Conor Dooley, naresh.kamboju, linux-riscv, llvm, stable

Hi Conor,

On Thu, Feb 23, 2023 at 10:05:46PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Quoting the llvm docs:
> > Between versions 2.0 and 2.1 of the base I specification, a backwards
> > incompatible change was made to remove selected instructions and CSRs
> > from the base ISA. These instructions were grouped into a set of new
> > extensions, but were no longer required by the base ISA. (snip) LLVM
> > currently implements version 2.0 of the base specification. Thus,
> > instructions from these extensions are accepted as part of the base
> > ISA.
> 
> There is therefore no need (at present!) to carry out a $cc-option
> check, and instead just gate presence of zicsr and zifencei in march
> on the version of binutils that commit 6df2a016c0c8 ("riscv: fix build
> with binutils 2.38") highlights as the introduction of the requirement.
> 
> In fact, the status quo creates some issues with mixed llvm/binutils
> builds, specifically building with llvm-17 and ld from binutils-2.35.
> Odd combo you may think, but this is what tuxsuite's debian stable uses
> while testing 5.10 stable kernels as doesn't support LLD.
> 
> CC: stable@vger.kernel.org # needs RISC-V: move zicsr/zifencei spec version check to Kconfi

I think it would be better to drop this comment and just mark patch 1
for stable directly. However, if it remains, 'Kconfi' -> 'Kconfig' :)

> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Link: https://lore.kernel.org/all/CA+G9fYt9T=ELCLaB9byxaLW2Qf4pZcDO=huCA0D8ug2V2+irJQ@mail.gmail.com/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

Thanks for helping getting down to the bottom of this!

> ---
>  arch/riscv/Kconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 4eb0ef8314b3..c6902f4c5650 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -291,8 +291,7 @@ endchoice
>  config TOOLCHAIN_NEEDS_SPEC_20191213
>  	bool
>  	default y
> -	depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zicsr_zifencei)
> -	depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zicsr_zifencei)
> +	depends on AS_IS_GNU && AS_VERSION >= 23800
>  	help
>  	  Newer binutils versions default to ISA spec version 20191213 which
>  	  moves some instructions from the I extension to the Zicsr and Zifencei
> -- 
> 2.39.1
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
  2023-02-23 22:05   ` Conor Dooley
@ 2023-02-24 16:37     ` Nathan Chancellor
  -1 siblings, 0 replies; 18+ messages in thread
From: Nathan Chancellor @ 2023-02-24 16:37 UTC (permalink / raw)
  To: Conor Dooley; +Cc: palmer, Conor Dooley, naresh.kamboju, linux-riscv, llvm

On Thu, Feb 23, 2023 at 10:05:45PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Checking whether binutils defaults to using a version of the spec
> requiring zicsr/zifencei being done in our Makefile is functionally
> sufficient at present, but makes it harder to tell after the fact
> which extensions are enabled.
> By moving it to Kconfig, it's easy to tell from standard build artifacts
> what has been done & the road is paved for dealing with this differently
> for both binutils and LLVM.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/riscv/Kconfig  | 10 ++++++++++
>  arch/riscv/Makefile |  5 +----
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index cebf0c5f8824..4eb0ef8314b3 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -288,6 +288,16 @@ config ARCH_RV64I
>  
>  endchoice
>  
> +config TOOLCHAIN_NEEDS_SPEC_20191213

This symbol's name is a little confusing to me, how does the toolchain
need spec version 20191213. Maybe 'SPEC_20191213' should be something
like 'EXPLICIT_ZICSR_ZIFENCEI'? Otherwise, maybe it should be something
like 'TOOLCHAIN_HAS_DEFAULT_SPEC_20191213'? Sorry for the bikeshed but I
think most Kconfig symbols should be self describing.

> +	bool
> +	default y
> +	depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zicsr_zifencei)
> +	depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zicsr_zifencei)
> +	help
> +	  Newer binutils versions default to ISA spec version 20191213 which
> +	  moves some instructions from the I extension to the Zicsr and Zifencei
> +	  extensions.
> +
>  # We must be able to map all physical memory into the kernel, but the compiler
>  # is still a bit more efficient when generating code if it's setup in a manner
>  # such that it can only map 2GiB of memory.
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 12d91b0a73d8..7e1267b0e73f 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -53,10 +53,7 @@ riscv-march-$(CONFIG_ARCH_RV64I)	:= rv64ima
>  riscv-march-$(CONFIG_FPU)		:= $(riscv-march-y)fd
>  riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
>  
> -# Newer binutils versions default to ISA spec version 20191213 which moves some
> -# instructions from the I extension to the Zicsr and Zifencei extensions.
> -toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei)
> -riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei
> +riscv-march-$(CONFIG_TOOLCHAIN_NEEDS_SPEC_20191213) := $(riscv-march-y)_zicsr_zifencei
>  
>  # Check if the toolchain supports Zicbom extension
>  riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZICBOM) := $(riscv-march-y)_zicbom
> -- 
> 2.39.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
@ 2023-02-24 16:37     ` Nathan Chancellor
  0 siblings, 0 replies; 18+ messages in thread
From: Nathan Chancellor @ 2023-02-24 16:37 UTC (permalink / raw)
  To: Conor Dooley; +Cc: palmer, Conor Dooley, naresh.kamboju, linux-riscv, llvm

On Thu, Feb 23, 2023 at 10:05:45PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Checking whether binutils defaults to using a version of the spec
> requiring zicsr/zifencei being done in our Makefile is functionally
> sufficient at present, but makes it harder to tell after the fact
> which extensions are enabled.
> By moving it to Kconfig, it's easy to tell from standard build artifacts
> what has been done & the road is paved for dealing with this differently
> for both binutils and LLVM.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/riscv/Kconfig  | 10 ++++++++++
>  arch/riscv/Makefile |  5 +----
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index cebf0c5f8824..4eb0ef8314b3 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -288,6 +288,16 @@ config ARCH_RV64I
>  
>  endchoice
>  
> +config TOOLCHAIN_NEEDS_SPEC_20191213

This symbol's name is a little confusing to me, how does the toolchain
need spec version 20191213. Maybe 'SPEC_20191213' should be something
like 'EXPLICIT_ZICSR_ZIFENCEI'? Otherwise, maybe it should be something
like 'TOOLCHAIN_HAS_DEFAULT_SPEC_20191213'? Sorry for the bikeshed but I
think most Kconfig symbols should be self describing.

> +	bool
> +	default y
> +	depends on !64BIT || $(cc-option,-mabi=lp64 -march=rv64ima_zicsr_zifencei)
> +	depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zicsr_zifencei)
> +	help
> +	  Newer binutils versions default to ISA spec version 20191213 which
> +	  moves some instructions from the I extension to the Zicsr and Zifencei
> +	  extensions.
> +
>  # We must be able to map all physical memory into the kernel, but the compiler
>  # is still a bit more efficient when generating code if it's setup in a manner
>  # such that it can only map 2GiB of memory.
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 12d91b0a73d8..7e1267b0e73f 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -53,10 +53,7 @@ riscv-march-$(CONFIG_ARCH_RV64I)	:= rv64ima
>  riscv-march-$(CONFIG_FPU)		:= $(riscv-march-y)fd
>  riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
>  
> -# Newer binutils versions default to ISA spec version 20191213 which moves some
> -# instructions from the I extension to the Zicsr and Zifencei extensions.
> -toolchain-need-zicsr-zifencei := $(call cc-option-yn, -march=$(riscv-march-y)_zicsr_zifencei)
> -riscv-march-$(toolchain-need-zicsr-zifencei) := $(riscv-march-y)_zicsr_zifencei
> +riscv-march-$(CONFIG_TOOLCHAIN_NEEDS_SPEC_20191213) := $(riscv-march-y)_zicsr_zifencei
>  
>  # Check if the toolchain supports Zicbom extension
>  riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZICBOM) := $(riscv-march-y)_zicbom
> -- 
> 2.39.1
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
  2023-02-24 16:37     ` Nathan Chancellor
@ 2023-02-24 17:00       ` Conor Dooley
  -1 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2023-02-24 17:00 UTC (permalink / raw)
  To: Nathan Chancellor; +Cc: palmer, Conor Dooley, naresh.kamboju, linux-riscv, llvm

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

On Fri, Feb 24, 2023 at 09:37:28AM -0700, Nathan Chancellor wrote:
> On Thu, Feb 23, 2023 at 10:05:45PM +0000, Conor Dooley wrote:

> > ---
> >  arch/riscv/Kconfig  | 10 ++++++++++
> >  arch/riscv/Makefile |  5 +----
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index cebf0c5f8824..4eb0ef8314b3 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -288,6 +288,16 @@ config ARCH_RV64I
> >  
> >  endchoice
> >  
> > +config TOOLCHAIN_NEEDS_SPEC_20191213
> 
> This symbol's name is a little confusing to me, how does the toolchain
> need spec version 20191213. Maybe 'SPEC_20191213' should be something
> like 'EXPLICIT_ZICSR_ZIFENCEI'? Otherwise, maybe it should be something
> like 'TOOLCHAIN_HAS_DEFAULT_SPEC_20191213'? Sorry for the bikeshed but I
> think most Kconfig symbols should be self describing.

Yah I can do that. I actually had this as the DEFAULT_SPEC variant
before and changed it - I'll switch it to the EXPLICIT one.
Thanks for taking a look Nathan :)

Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
@ 2023-02-24 17:00       ` Conor Dooley
  0 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2023-02-24 17:00 UTC (permalink / raw)
  To: Nathan Chancellor; +Cc: palmer, Conor Dooley, naresh.kamboju, linux-riscv, llvm


[-- Attachment #1.1: Type: text/plain, Size: 1101 bytes --]

On Fri, Feb 24, 2023 at 09:37:28AM -0700, Nathan Chancellor wrote:
> On Thu, Feb 23, 2023 at 10:05:45PM +0000, Conor Dooley wrote:

> > ---
> >  arch/riscv/Kconfig  | 10 ++++++++++
> >  arch/riscv/Makefile |  5 +----
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index cebf0c5f8824..4eb0ef8314b3 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -288,6 +288,16 @@ config ARCH_RV64I
> >  
> >  endchoice
> >  
> > +config TOOLCHAIN_NEEDS_SPEC_20191213
> 
> This symbol's name is a little confusing to me, how does the toolchain
> need spec version 20191213. Maybe 'SPEC_20191213' should be something
> like 'EXPLICIT_ZICSR_ZIFENCEI'? Otherwise, maybe it should be something
> like 'TOOLCHAIN_HAS_DEFAULT_SPEC_20191213'? Sorry for the bikeshed but I
> think most Kconfig symbols should be self describing.

Yah I can do that. I actually had this as the DEFAULT_SPEC variant
before and changed it - I'll switch it to the EXPLICIT one.
Thanks for taking a look Nathan :)

Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
  2023-02-24 17:00       ` Conor Dooley
@ 2023-03-06 23:52         ` Palmer Dabbelt
  -1 siblings, 0 replies; 18+ messages in thread
From: Palmer Dabbelt @ 2023-03-06 23:52 UTC (permalink / raw)
  To: Conor Dooley; +Cc: nathan, Conor Dooley, naresh.kamboju, linux-riscv, llvm

On Fri, 24 Feb 2023 09:00:09 PST (-0800), Conor Dooley wrote:
> On Fri, Feb 24, 2023 at 09:37:28AM -0700, Nathan Chancellor wrote:
>> On Thu, Feb 23, 2023 at 10:05:45PM +0000, Conor Dooley wrote:
>
>> > ---
>> >  arch/riscv/Kconfig  | 10 ++++++++++
>> >  arch/riscv/Makefile |  5 +----
>> >  2 files changed, 11 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> > index cebf0c5f8824..4eb0ef8314b3 100644
>> > --- a/arch/riscv/Kconfig
>> > +++ b/arch/riscv/Kconfig
>> > @@ -288,6 +288,16 @@ config ARCH_RV64I
>> >  
>> >  endchoice
>> >  
>> > +config TOOLCHAIN_NEEDS_SPEC_20191213
>> 
>> This symbol's name is a little confusing to me, how does the toolchain
>> need spec version 20191213. Maybe 'SPEC_20191213' should be something
>> like 'EXPLICIT_ZICSR_ZIFENCEI'? Otherwise, maybe it should be something
>> like 'TOOLCHAIN_HAS_DEFAULT_SPEC_20191213'? Sorry for the bikeshed but I
>> think most Kconfig symbols should be self describing.
>
> Yah I can do that. I actually had this as the DEFAULT_SPEC variant
> before and changed it - I'll switch it to the EXPLICIT one.
> Thanks for taking a look Nathan :)

Another option here would be to use the `-misa-spec` argument for GCC, 
to make it match LLVM's spec version.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
@ 2023-03-06 23:52         ` Palmer Dabbelt
  0 siblings, 0 replies; 18+ messages in thread
From: Palmer Dabbelt @ 2023-03-06 23:52 UTC (permalink / raw)
  To: Conor Dooley; +Cc: nathan, Conor Dooley, naresh.kamboju, linux-riscv, llvm

On Fri, 24 Feb 2023 09:00:09 PST (-0800), Conor Dooley wrote:
> On Fri, Feb 24, 2023 at 09:37:28AM -0700, Nathan Chancellor wrote:
>> On Thu, Feb 23, 2023 at 10:05:45PM +0000, Conor Dooley wrote:
>
>> > ---
>> >  arch/riscv/Kconfig  | 10 ++++++++++
>> >  arch/riscv/Makefile |  5 +----
>> >  2 files changed, 11 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> > index cebf0c5f8824..4eb0ef8314b3 100644
>> > --- a/arch/riscv/Kconfig
>> > +++ b/arch/riscv/Kconfig
>> > @@ -288,6 +288,16 @@ config ARCH_RV64I
>> >  
>> >  endchoice
>> >  
>> > +config TOOLCHAIN_NEEDS_SPEC_20191213
>> 
>> This symbol's name is a little confusing to me, how does the toolchain
>> need spec version 20191213. Maybe 'SPEC_20191213' should be something
>> like 'EXPLICIT_ZICSR_ZIFENCEI'? Otherwise, maybe it should be something
>> like 'TOOLCHAIN_HAS_DEFAULT_SPEC_20191213'? Sorry for the bikeshed but I
>> think most Kconfig symbols should be self describing.
>
> Yah I can do that. I actually had this as the DEFAULT_SPEC variant
> before and changed it - I'll switch it to the EXPLICIT one.
> Thanks for taking a look Nathan :)

Another option here would be to use the `-misa-spec` argument for GCC, 
to make it match LLVM's spec version.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
  2023-03-06 23:52         ` Palmer Dabbelt
@ 2023-03-07 19:14           ` Conor Dooley
  -1 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2023-03-07 19:14 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: nathan, Conor Dooley, naresh.kamboju, linux-riscv, llvm

[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]

On Mon, Mar 06, 2023 at 03:52:27PM -0800, Palmer Dabbelt wrote:
> On Fri, 24 Feb 2023 09:00:09 PST (-0800), Conor Dooley wrote:
> > On Fri, Feb 24, 2023 at 09:37:28AM -0700, Nathan Chancellor wrote:
> > > On Thu, Feb 23, 2023 at 10:05:45PM +0000, Conor Dooley wrote:
> > 
> > > > ---
> > > >  arch/riscv/Kconfig  | 10 ++++++++++
> > > >  arch/riscv/Makefile |  5 +----
> > > >  2 files changed, 11 insertions(+), 4 deletions(-)
> > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > index cebf0c5f8824..4eb0ef8314b3 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > > > @@ -288,6 +288,16 @@ config ARCH_RV64I
> > > >  >  endchoice
> > > >  > +config TOOLCHAIN_NEEDS_SPEC_20191213
> > > 
> > > This symbol's name is a little confusing to me, how does the toolchain
> > > need spec version 20191213. Maybe 'SPEC_20191213' should be something
> > > like 'EXPLICIT_ZICSR_ZIFENCEI'? Otherwise, maybe it should be something
> > > like 'TOOLCHAIN_HAS_DEFAULT_SPEC_20191213'? Sorry for the bikeshed but I
> > > think most Kconfig symbols should be self describing.
> > 
> > Yah I can do that. I actually had this as the DEFAULT_SPEC variant
> > before and changed it - I'll switch it to the EXPLICIT one.
> > Thanks for taking a look Nathan :)
> 
> Another option here would be to use the `-misa-spec` argument for GCC, to
> make it match LLVM's spec version.

I'm not entirely sure what you're getting at.
You're the toolchain guy, but my understanding was that gas and gcc have
that option, but not ld, which rules out my initial interpretation. 
Are you suggesting that we always pass -misa-spec=<pre-zicsr_zifencei> to
gcc so that we are using the same misa-spec for both gnu and llvm
toolchain bits?

My instinctive thought was that that is kinda fragile to things
misaligning between gcc/llvm, but given I seem to end up looking at this
every month, we're really fragile to changes in either camp anyway...

I can go spin an alternative v2 of this up if I have understood you
correctly.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
@ 2023-03-07 19:14           ` Conor Dooley
  0 siblings, 0 replies; 18+ messages in thread
From: Conor Dooley @ 2023-03-07 19:14 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: nathan, Conor Dooley, naresh.kamboju, linux-riscv, llvm


[-- Attachment #1.1: Type: text/plain, Size: 2090 bytes --]

On Mon, Mar 06, 2023 at 03:52:27PM -0800, Palmer Dabbelt wrote:
> On Fri, 24 Feb 2023 09:00:09 PST (-0800), Conor Dooley wrote:
> > On Fri, Feb 24, 2023 at 09:37:28AM -0700, Nathan Chancellor wrote:
> > > On Thu, Feb 23, 2023 at 10:05:45PM +0000, Conor Dooley wrote:
> > 
> > > > ---
> > > >  arch/riscv/Kconfig  | 10 ++++++++++
> > > >  arch/riscv/Makefile |  5 +----
> > > >  2 files changed, 11 insertions(+), 4 deletions(-)
> > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > index cebf0c5f8824..4eb0ef8314b3 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > > > @@ -288,6 +288,16 @@ config ARCH_RV64I
> > > >  >  endchoice
> > > >  > +config TOOLCHAIN_NEEDS_SPEC_20191213
> > > 
> > > This symbol's name is a little confusing to me, how does the toolchain
> > > need spec version 20191213. Maybe 'SPEC_20191213' should be something
> > > like 'EXPLICIT_ZICSR_ZIFENCEI'? Otherwise, maybe it should be something
> > > like 'TOOLCHAIN_HAS_DEFAULT_SPEC_20191213'? Sorry for the bikeshed but I
> > > think most Kconfig symbols should be self describing.
> > 
> > Yah I can do that. I actually had this as the DEFAULT_SPEC variant
> > before and changed it - I'll switch it to the EXPLICIT one.
> > Thanks for taking a look Nathan :)
> 
> Another option here would be to use the `-misa-spec` argument for GCC, to
> make it match LLVM's spec version.

I'm not entirely sure what you're getting at.
You're the toolchain guy, but my understanding was that gas and gcc have
that option, but not ld, which rules out my initial interpretation. 
Are you suggesting that we always pass -misa-spec=<pre-zicsr_zifencei> to
gcc so that we are using the same misa-spec for both gnu and llvm
toolchain bits?

My instinctive thought was that that is kinda fragile to things
misaligning between gcc/llvm, but given I seem to end up looking at this
every month, we're really fragile to changes in either camp anyway...

I can go spin an alternative v2 of this up if I have understood you
correctly.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
  2023-03-07 19:14           ` Conor Dooley
@ 2023-03-07 19:24             ` Palmer Dabbelt
  -1 siblings, 0 replies; 18+ messages in thread
From: Palmer Dabbelt @ 2023-03-07 19:24 UTC (permalink / raw)
  To: Conor Dooley; +Cc: nathan, Conor Dooley, naresh.kamboju, linux-riscv, llvm

On Tue, 07 Mar 2023 11:14:54 PST (-0800), Conor Dooley wrote:
> On Mon, Mar 06, 2023 at 03:52:27PM -0800, Palmer Dabbelt wrote:
>> On Fri, 24 Feb 2023 09:00:09 PST (-0800), Conor Dooley wrote:
>> > On Fri, Feb 24, 2023 at 09:37:28AM -0700, Nathan Chancellor wrote:
>> > > On Thu, Feb 23, 2023 at 10:05:45PM +0000, Conor Dooley wrote:
>> > 
>> > > > ---
>> > > >  arch/riscv/Kconfig  | 10 ++++++++++
>> > > >  arch/riscv/Makefile |  5 +----
>> > > >  2 files changed, 11 insertions(+), 4 deletions(-)
>> > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> > > > index cebf0c5f8824..4eb0ef8314b3 100644
>> > > > --- a/arch/riscv/Kconfig
>> > > > +++ b/arch/riscv/Kconfig
>> > > > @@ -288,6 +288,16 @@ config ARCH_RV64I
>> > > >  >  endchoice
>> > > >  > +config TOOLCHAIN_NEEDS_SPEC_20191213
>> > > 
>> > > This symbol's name is a little confusing to me, how does the toolchain
>> > > need spec version 20191213. Maybe 'SPEC_20191213' should be something
>> > > like 'EXPLICIT_ZICSR_ZIFENCEI'? Otherwise, maybe it should be something
>> > > like 'TOOLCHAIN_HAS_DEFAULT_SPEC_20191213'? Sorry for the bikeshed but I
>> > > think most Kconfig symbols should be self describing.
>> > 
>> > Yah I can do that. I actually had this as the DEFAULT_SPEC variant
>> > before and changed it - I'll switch it to the EXPLICIT one.
>> > Thanks for taking a look Nathan :)
>> 
>> Another option here would be to use the `-misa-spec` argument for GCC, to
>> make it match LLVM's spec version.
>
> I'm not entirely sure what you're getting at.
> You're the toolchain guy, but my understanding was that gas and gcc have
> that option, but not ld, which rules out my initial interpretation. 
> Are you suggesting that we always pass -misa-spec=<pre-zicsr_zifencei> to
> gcc so that we are using the same misa-spec for both gnu and llvm
> toolchain bits?

Pretty much, I think just $(cc-option -misa-spec=2.2) should do it.  
That should also cover older GCC versions, as we added the argument long 
before we changed the default.

> My instinctive thought was that that is kinda fragile to things
> misaligning between gcc/llvm, but given I seem to end up looking at this
> every month, we're really fragile to changes in either camp anyway...

Pretty much, but in this case we're essentially just disabling a form of 
change by sticking to an ISA spec version.

> I can go spin an alternative v2 of this up if I have understood you
> correctly.

Thanks

>
> Thanks,
> Conor.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
@ 2023-03-07 19:24             ` Palmer Dabbelt
  0 siblings, 0 replies; 18+ messages in thread
From: Palmer Dabbelt @ 2023-03-07 19:24 UTC (permalink / raw)
  To: Conor Dooley; +Cc: nathan, Conor Dooley, naresh.kamboju, linux-riscv, llvm

On Tue, 07 Mar 2023 11:14:54 PST (-0800), Conor Dooley wrote:
> On Mon, Mar 06, 2023 at 03:52:27PM -0800, Palmer Dabbelt wrote:
>> On Fri, 24 Feb 2023 09:00:09 PST (-0800), Conor Dooley wrote:
>> > On Fri, Feb 24, 2023 at 09:37:28AM -0700, Nathan Chancellor wrote:
>> > > On Thu, Feb 23, 2023 at 10:05:45PM +0000, Conor Dooley wrote:
>> > 
>> > > > ---
>> > > >  arch/riscv/Kconfig  | 10 ++++++++++
>> > > >  arch/riscv/Makefile |  5 +----
>> > > >  2 files changed, 11 insertions(+), 4 deletions(-)
>> > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> > > > index cebf0c5f8824..4eb0ef8314b3 100644
>> > > > --- a/arch/riscv/Kconfig
>> > > > +++ b/arch/riscv/Kconfig
>> > > > @@ -288,6 +288,16 @@ config ARCH_RV64I
>> > > >  >  endchoice
>> > > >  > +config TOOLCHAIN_NEEDS_SPEC_20191213
>> > > 
>> > > This symbol's name is a little confusing to me, how does the toolchain
>> > > need spec version 20191213. Maybe 'SPEC_20191213' should be something
>> > > like 'EXPLICIT_ZICSR_ZIFENCEI'? Otherwise, maybe it should be something
>> > > like 'TOOLCHAIN_HAS_DEFAULT_SPEC_20191213'? Sorry for the bikeshed but I
>> > > think most Kconfig symbols should be self describing.
>> > 
>> > Yah I can do that. I actually had this as the DEFAULT_SPEC variant
>> > before and changed it - I'll switch it to the EXPLICIT one.
>> > Thanks for taking a look Nathan :)
>> 
>> Another option here would be to use the `-misa-spec` argument for GCC, to
>> make it match LLVM's spec version.
>
> I'm not entirely sure what you're getting at.
> You're the toolchain guy, but my understanding was that gas and gcc have
> that option, but not ld, which rules out my initial interpretation. 
> Are you suggesting that we always pass -misa-spec=<pre-zicsr_zifencei> to
> gcc so that we are using the same misa-spec for both gnu and llvm
> toolchain bits?

Pretty much, I think just $(cc-option -misa-spec=2.2) should do it.  
That should also cover older GCC versions, as we added the argument long 
before we changed the default.

> My instinctive thought was that that is kinda fragile to things
> misaligning between gcc/llvm, but given I seem to end up looking at this
> every month, we're really fragile to changes in either camp anyway...

Pretty much, but in this case we're essentially just disabling a form of 
change by sticking to an ISA spec version.

> I can go spin an alternative v2 of this up if I have understood you
> correctly.

Thanks

>
> Thanks,
> Conor.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-03-07 19:24 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-23 22:05 [PATCH v1 0/2] RISC-V: avoid build issues for clang/llvm-17 with binutils 2.35 Conor Dooley
2023-02-23 22:05 ` Conor Dooley
2023-02-23 22:05 ` [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig Conor Dooley
2023-02-23 22:05   ` Conor Dooley
2023-02-24 16:37   ` Nathan Chancellor
2023-02-24 16:37     ` Nathan Chancellor
2023-02-24 17:00     ` Conor Dooley
2023-02-24 17:00       ` Conor Dooley
2023-03-06 23:52       ` Palmer Dabbelt
2023-03-06 23:52         ` Palmer Dabbelt
2023-03-07 19:14         ` Conor Dooley
2023-03-07 19:14           ` Conor Dooley
2023-03-07 19:24           ` Palmer Dabbelt
2023-03-07 19:24             ` Palmer Dabbelt
2023-02-23 22:05 ` [PATCH v1 2/2] RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only Conor Dooley
2023-02-23 22:05   ` Conor Dooley
2023-02-24 16:32   ` Nathan Chancellor
2023-02-24 16:32     ` Nathan Chancellor

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.