RISC-V: avoid build issues for clang/llvm-17 with binutils 2.35
Subsystems (2)
cover RISC-V: avoid build issues for clang/llvm-17 with binutils 2.35
From: Conor Dooley <[email protected]>
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/[email protected]/
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(-)
Signature · 1 line
2.39.1
PATCH 1/2 RISC-V: move zicsr/zifencei spec version check to Kconfig
From: Conor Dooley <[email protected]>
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.
--- arch/riscv/Kconfig | 10 ++++++++++ arch/riscv/Makefile | 5 +---- 2 files changed, 11 insertions(+), 4 deletions(-)
arch/riscv/Kconfig +10 -0
@@ -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.
arch/riscv/Makefile +1 -4
@@ -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
Signature · 1 line
2.39.1
Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
Quoted 11 lines · Conor Dooley
From: Conor Dooley <[email protected]>
Checking whether binutils defaults to using a version of the spec requiring zicsr/zifencei being done in our
Makefileis functionally sufficient at present, but makes it harder to tell after the fact which extensions are enabled.
By moving it toKconfig, 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 <[email protected]>
Quoted 10 lines
--- arch/riscv/Kconfig | 10 ++++++++++ arch/riscv/Makefile | 5 +---- 2 files changed, 11 insertions(+), 4 deletions(-)arch/riscv/Kconfig +0
-0
arch/riscv/Kconfig config ARCH_RV64I · line 291
Quoted 4 lines
arch/riscv/Kconfig +1
-0@@ -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.
Quoted 31 lines
arch/riscv/Kconfig +9
-0+ 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.arch/riscv/Makefile +1
-4@@ -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)_zicbomSignature · 1 line
2.39.1
Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
Quoted 15 lines · Nathan Chancellor
Quoted 13 lines · Conor Dooley
--- arch/riscv/Kconfig | 10 ++++++++++ arch/riscv/Makefile | 5 +---- 2 files changed, 11 insertions(+), 4 deletions(-)arch/riscv/Kconfig +0
-0@@ -288,6 +288,16 @@ config ARCH_RV64I endchoice
Quoted 7 lines
arch/riscv/Kconfig config ARCH_RV64I · line 291
Quoted 1 line
arch/riscv/Kconfig +1
-0+config TOOLCHAIN_NEEDS_SPEC_20191213This 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 mostKconfigsymbols 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.
Palmer Dabbelt
maintainerRe: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
Quoted 24 lines · Conor Dooley
Quoted 15 lines · Nathan Chancellor
Quoted 13 lines · Conor Dooley
--- arch/riscv/Kconfig | 10 ++++++++++ arch/riscv/Makefile | 5 +---- 2 files changed, 11 insertions(+), 4 deletions(-)arch/riscv/Kconfig +0
-0@@ -288,6 +288,16 @@ config ARCH_RV64I endchoiceQuoted 7 lines
arch/riscv/Kconfig config ARCH_RV64I · line 291
Quoted 1 line
arch/riscv/Kconfig +1
-0+config TOOLCHAIN_NEEDS_SPEC_20191213This 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 mostKconfigsymbols should be self describing.
Quoted 3 lines
Yah I can do that. I actually had this as the
DEFAULT_SPECvariant 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.
Re: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
Quoted 22 lines · Palmer Dabbelt
Quoted 21 lines · Conor Dooley
Quoted 19 lines · Nathan Chancellor
arch/riscv/Kconfig config ARCH_RV64I · line 289
Quoted 11 lines · Conor Dooley
--- arch/riscv/Kconfig | 10 ++++++++++ arch/riscv/Makefile | 5 +---- 2 files changed, 11 insertions(+), 4 deletions(-)arch/riscv/Kconfig
Quoted 1 line
arch/riscv/Kconfig +0
-0index
cebf0c5f8824..4eb0ef8314b3100644arch/riscv/Kconfig +0
-0@@ -288,6 +288,16 @@ config ARCH_RV64I > endchoice > +config TOOLCHAIN_NEEDS_SPEC_20191213This 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 mostKconfigsymbols should be self describing.
Quoted 6 lines
Quoted 3 lines
Yah I can do that. I actually had this as the
DEFAULT_SPECvariant 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-specargument 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.
Palmer Dabbelt
maintainerRe: [PATCH v1 1/2] RISC-V: move zicsr/zifencei spec version check to Kconfig
Quoted 27 lines · Conor Dooley
Quoted 26 lines · Palmer Dabbelt
Quoted 24 lines · Conor Dooley
Quoted 6 lines · Nathan Chancellor
Quoted 4 lines · Conor Dooley
--- arch/riscv/Kconfig | 10 ++++++++++ arch/riscv/Makefile | 5 +---- 2 files changed, 11 insertions(+), 4 deletions(-)Quoted 13 lines
arch/riscv/Kconfig config ARCH_RV64I · line 289
Quoted 7 lines
arch/riscv/Kconfig
Quoted 1 line
arch/riscv/Kconfig +0
-0index
cebf0c5f8824..4eb0ef8314b3100644arch/riscv/Kconfig +0
-0@@ -288,6 +288,16 @@ config ARCH_RV64I > endchoice > +config TOOLCHAIN_NEEDS_SPEC_20191213This 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 mostKconfigsymbols should be self describing.Yah I can do that. I actually had this as the
DEFAULT_SPECvariant before and changed it - I'll switch it to the EXPLICIT one. Thanks for taking a look Nathan :)
Quoted 8 lines
Quoted 2 lines
Another option here would be to use the
-misa-specargument 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.
Quoted 3 lines
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.
Quoted 2 lines
I can go spin an alternative v2 of this up if I have understood you correctly.
Thanks
Quoted 3 lines
Thanks,
Conor.
PATCH 2/2 RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only
In reply to Conor DooleyFrom: Conor Dooley <[email protected]>
Quoting the llvm docs:
Quoted 7 lines
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.
--- arch/riscv/Kconfig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/riscv/Kconfig +1 -2
@@ -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
Signature · 1 line
2.39.1
Re: [PATCH v1 2/2] RISC-V: make TOOLCHAIN_NEEDS_SPEC_20191213 gas only
Hi Conor,
Quoted 16 lines · Conor Dooley
From: Conor Dooley <[email protected]>
Quoting the llvm docs:
Quoted 7 lines
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.
Quoted 6 lines
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 [email protected] # 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' :)
Quoted 4 lines
Thanks for helping getting down to the bottom of this!
Quoted 21 lines
--- arch/riscv/Kconfig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)arch/riscv/Kconfig +1
-2@@ -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 ZifenceiSignature · 1 line
2.39.1