2023-03-13 23:02:05

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH] riscv: Handle zicsr/zifencei issues between clang and binutils

There are two related issues that appear in certain combinations with
clang and GNU binutils.

The first occurs when a version of clang that supports zicsr or zifencei
via '-march=' [1] (i.e, >= 17.x) is used in combination with a version
of GNU binutils that do not recognize zicsr and zifencei in the
'-march=' value (i.e., < 2.36):

riscv64-linux-gnu-ld: -march=rv64i2p0_m2p0_a2p0_c2p0_zicsr2p0_zifencei2p0: Invalid or unknown z ISA extension: 'zifencei'
riscv64-linux-gnu-ld: failed to merge target specific data of file fs/efivarfs/file.o
riscv64-linux-gnu-ld: -march=rv64i2p0_m2p0_a2p0_c2p0_zicsr2p0_zifencei2p0: Invalid or unknown z ISA extension: 'zifencei'
riscv64-linux-gnu-ld: failed to merge target specific data of file fs/efivarfs/super.o

The second occurs when a version of clang that does not support zicsr or
zifencei via '-march=' (i.e., <= 16.x) is used in combination with a
version of GNU as that defaults to a newer ISA base spec, which requires
specifying zicsr and zifencei in the '-march=' value explicitly (i.e, >=
2.38):

../arch/riscv/kernel/kexec_relocate.S: Assembler messages:
../arch/riscv/kernel/kexec_relocate.S:147: Error: unrecognized opcode `fence.i', extension `zifencei' required
clang-12: error: assembler command failed with exit code 1 (use -v to see invocation)

This is the same issue addressed by commit 6df2a016c0c8 ("riscv: fix
build with binutils 2.38") (see [2] for additional information) but
older versions of clang miss out on it because the cc-option check
fails:

clang-12: error: invalid arch name 'rv64imac_zicsr_zifencei', unsupported standard user-level extension 'zicsr'
clang-12: error: invalid arch name 'rv64imac_zicsr_zifencei', unsupported standard user-level extension 'zicsr'

To resolve the first issue, only attempt to add zicsr and zifencei to
the march string when using the GNU assembler 2.38 or newer, which is
when the default ISA spec was updated, requiring these extensions to be
specified explicitly. LLVM implements an older version of the base
specification for all currently released versions, so these instructions
are available as part of the 'i' extension. If LLVM's implementation is
updated in the future, a CONFIG_AS_IS_LLVM condition can be added to
CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI.

To resolve the second issue, use version 2.2 of the base ISA spec when
using an older version of clang that does not support zicsr or zifencei
via '-march=', as that is the spec version most compatible with the one
clang/LLVM implements and avoids the need to specify zicsr and zifencei
explicitly due to still being a part of 'i'.

[1]: https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
[2]: https://lore.kernel.org/[email protected]/

Cc: [email protected]
Link: https://github.com/ClangBuiltLinux/linux/issues/1808
Co-developed-by: Conor Dooley <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---
This is essentially a v3 of Conor's v1 and v2 but since I am sending the
patch after finding a separate but related issue, I left it at v1:

- v1: https://lore.kernel.org/[email protected]/
- v2: https://lore.kernel.org/[email protected]/

I have built allmodconfig with the following toolchain combinations to
confirm this problem is resolved:

- clang 12/17 + GNU as and ld 2.35/2.39
- clang 12/17 with the integrated assembler + GNU ld 2.35/2.39
- clang 12/17 with the integrated assembler + ld.lld

There are a couple of other incompatibilities between clang-17 and GNU
binutils that I had to patch to get allmodconfig to build successfully
but those are less likely to be hit in practice because the full LLVM
stack can be used with LLVM versions 13.x and newer. I will follow up
with separate issues and patches.
---
arch/riscv/Kconfig | 22 ++++++++++++++++++++++
arch/riscv/Makefile | 10 ++++++----
2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index c5e42cc37604..5b182d1c196c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -464,6 +464,28 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE
depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zihintpause)
depends on LLD_VERSION >= 150000 || LD_VERSION >= 23600

+config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
+ def_bool y
+ # https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
+ 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
+ extensions.
+
+config TOOLCHAIN_NEEDS_OLD_ISA_SPEC
+ def_bool y
+ depends on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
+ # https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
+ depends on CC_IS_CLANG && CLANG_VERSION < 170000
+ help
+ Certain versions of clang do not support zicsr and zifencei via -march
+ but newer versions of binutils require it for the reasons noted in the
+ help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
+ option causes an older ISA spec compatible with these older versions
+ of clang to be passed to GAS, which has the same result as passing zicsr
+ and zifencei to -march.
+
config FPU
bool "FPU support"
default y
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 4de83b9b1772..b05e833a022d 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -57,10 +57,12 @@ 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
+ifdef CONFIG_TOOLCHAIN_NEEDS_OLD_ISA_SPEC
+KBUILD_CFLAGS += -Wa,-misa-spec=2.2
+KBUILD_AFLAGS += -Wa,-misa-spec=2.2
+else
+riscv-march-$(CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI) := $(riscv-march-y)_zicsr_zifencei
+endif

# Check if the toolchain supports Zihintpause extension
riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause

---
base-commit: eeac8ede17557680855031c6f305ece2378af326
change-id: 20230313-riscv-zicsr-zifencei-fiasco-2941caebe7dc

Best regards,
--
Nathan Chancellor <[email protected]>



2023-03-14 17:08:22

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] riscv: Handle zicsr/zifencei issues between clang and binutils

Hey Nathan,

On Mon, Mar 13, 2023 at 04:00:23PM -0700, Nathan Chancellor wrote:
> There are two related issues that appear in certain combinations with
> clang and GNU binutils.
>
> The first occurs when a version of clang that supports zicsr or zifencei
> via '-march=' [1] (i.e, >= 17.x) is used in combination with a version
> of GNU binutils that do not recognize zicsr and zifencei in the
> '-march=' value (i.e., < 2.36):
>
> riscv64-linux-gnu-ld: -march=rv64i2p0_m2p0_a2p0_c2p0_zicsr2p0_zifencei2p0: Invalid or unknown z ISA extension: 'zifencei'
> riscv64-linux-gnu-ld: failed to merge target specific data of file fs/efivarfs/file.o
> riscv64-linux-gnu-ld: -march=rv64i2p0_m2p0_a2p0_c2p0_zicsr2p0_zifencei2p0: Invalid or unknown z ISA extension: 'zifencei'
> riscv64-linux-gnu-ld: failed to merge target specific data of file fs/efivarfs/super.o
>
> The second occurs when a version of clang that does not support zicsr or
> zifencei via '-march=' (i.e., <= 16.x) is used in combination with a
> version of GNU as that defaults to a newer ISA base spec, which requires
> specifying zicsr and zifencei in the '-march=' value explicitly (i.e, >=
> 2.38):
>
> ../arch/riscv/kernel/kexec_relocate.S: Assembler messages:
> ../arch/riscv/kernel/kexec_relocate.S:147: Error: unrecognized opcode `fence.i', extension `zifencei' required
> clang-12: error: assembler command failed with exit code 1 (use -v to see invocation)
>
> This is the same issue addressed by commit 6df2a016c0c8 ("riscv: fix
> build with binutils 2.38") (see [2] for additional information) but
> older versions of clang miss out on it because the cc-option check
> fails:
>
> clang-12: error: invalid arch name 'rv64imac_zicsr_zifencei', unsupported standard user-level extension 'zicsr'
> clang-12: error: invalid arch name 'rv64imac_zicsr_zifencei', unsupported standard user-level extension 'zicsr'
>
> To resolve the first issue, only attempt to add zicsr and zifencei to
> the march string when using the GNU assembler 2.38 or newer, which is
> when the default ISA spec was updated, requiring these extensions to be
> specified explicitly. LLVM implements an older version of the base
> specification for all currently released versions, so these instructions
> are available as part of the 'i' extension. If LLVM's implementation is
> updated in the future, a CONFIG_AS_IS_LLVM condition can be added to
> CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI.
>
> To resolve the second issue, use version 2.2 of the base ISA spec when
> using an older version of clang that does not support zicsr or zifencei
> via '-march=', as that is the spec version most compatible with the one
> clang/LLVM implements and avoids the need to specify zicsr and zifencei
> explicitly due to still being a part of 'i'.
>
> [1]: https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
> [2]: https://lore.kernel.org/[email protected]/
>
> Cc: [email protected]
> Link: https://github.com/ClangBuiltLinux/linux/issues/1808
> Co-developed-by: Conor Dooley <[email protected]>
> Signed-off-by: Conor Dooley <[email protected]>

TBH, barely identifiable as having much to do with my V1 anymore, so
could easily have dropped those.
Either way, thanks for sorting this out while I've been sick :)

Acked-by: Conor Dooley <[email protected]>

> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> This is essentially a v3 of Conor's v1 and v2 but since I am sending the
> patch after finding a separate but related issue, I left it at v1:
>
> - v1: https://lore.kernel.org/[email protected]/
> - v2: https://lore.kernel.org/[email protected]/
>
> I have built allmodconfig with the following toolchain combinations to
> confirm this problem is resolved:
>
> - clang 12/17 + GNU as and ld 2.35/2.39
> - clang 12/17 with the integrated assembler + GNU ld 2.35/2.39
> - clang 12/17 with the integrated assembler + ld.lld
>
> There are a couple of other incompatibilities between clang-17 and GNU
> binutils that I had to patch to get allmodconfig to build successfully
> but those are less likely to be hit in practice because the full LLVM
> stack can be used with LLVM versions 13.x and newer.

> I will follow up
> with separate issues and patches.

Cool, I'll "look forward" to those...

Thanks,
Conor.


Attachments:
(No filename) (4.31 kB)
signature.asc (228.00 B)
Download all attachments

2023-03-23 20:53:40

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] riscv: Handle zicsr/zifencei issues between clang and binutils

On Mon, 13 Mar 2023 16:00:23 PDT (-0700), [email protected] wrote:
> There are two related issues that appear in certain combinations with
> clang and GNU binutils.
>
> The first occurs when a version of clang that supports zicsr or zifencei
> via '-march=' [1] (i.e, >= 17.x) is used in combination with a version
> of GNU binutils that do not recognize zicsr and zifencei in the
> '-march=' value (i.e., < 2.36):
>
> riscv64-linux-gnu-ld: -march=rv64i2p0_m2p0_a2p0_c2p0_zicsr2p0_zifencei2p0: Invalid or unknown z ISA extension: 'zifencei'
> riscv64-linux-gnu-ld: failed to merge target specific data of file fs/efivarfs/file.o
> riscv64-linux-gnu-ld: -march=rv64i2p0_m2p0_a2p0_c2p0_zicsr2p0_zifencei2p0: Invalid or unknown z ISA extension: 'zifencei'
> riscv64-linux-gnu-ld: failed to merge target specific data of file fs/efivarfs/super.o
>
> The second occurs when a version of clang that does not support zicsr or
> zifencei via '-march=' (i.e., <= 16.x) is used in combination with a
> version of GNU as that defaults to a newer ISA base spec, which requires
> specifying zicsr and zifencei in the '-march=' value explicitly (i.e, >=
> 2.38):
>
> ../arch/riscv/kernel/kexec_relocate.S: Assembler messages:
> ../arch/riscv/kernel/kexec_relocate.S:147: Error: unrecognized opcode `fence.i', extension `zifencei' required
> clang-12: error: assembler command failed with exit code 1 (use -v to see invocation)
>
> This is the same issue addressed by commit 6df2a016c0c8 ("riscv: fix
> build with binutils 2.38") (see [2] for additional information) but
> older versions of clang miss out on it because the cc-option check
> fails:
>
> clang-12: error: invalid arch name 'rv64imac_zicsr_zifencei', unsupported standard user-level extension 'zicsr'
> clang-12: error: invalid arch name 'rv64imac_zicsr_zifencei', unsupported standard user-level extension 'zicsr'
>
> To resolve the first issue, only attempt to add zicsr and zifencei to
> the march string when using the GNU assembler 2.38 or newer, which is
> when the default ISA spec was updated, requiring these extensions to be
> specified explicitly. LLVM implements an older version of the base
> specification for all currently released versions, so these instructions
> are available as part of the 'i' extension. If LLVM's implementation is
> updated in the future, a CONFIG_AS_IS_LLVM condition can be added to
> CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI.
>
> To resolve the second issue, use version 2.2 of the base ISA spec when
> using an older version of clang that does not support zicsr or zifencei
> via '-march=', as that is the spec version most compatible with the one
> clang/LLVM implements and avoids the need to specify zicsr and zifencei
> explicitly due to still being a part of 'i'.
>
> [1]: https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
> [2]: https://lore.kernel.org/[email protected]/
>
> Cc: [email protected]
> Link: https://github.com/ClangBuiltLinux/linux/issues/1808
> Co-developed-by: Conor Dooley <[email protected]>
> Signed-off-by: Conor Dooley <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
> This is essentially a v3 of Conor's v1 and v2 but since I am sending the
> patch after finding a separate but related issue, I left it at v1:
>
> - v1: https://lore.kernel.org/[email protected]/
> - v2: https://lore.kernel.org/[email protected]/
>
> I have built allmodconfig with the following toolchain combinations to
> confirm this problem is resolved:
>
> - clang 12/17 + GNU as and ld 2.35/2.39
> - clang 12/17 with the integrated assembler + GNU ld 2.35/2.39
> - clang 12/17 with the integrated assembler + ld.lld
>
> There are a couple of other incompatibilities between clang-17 and GNU
> binutils that I had to patch to get allmodconfig to build successfully
> but those are less likely to be hit in practice because the full LLVM
> stack can be used with LLVM versions 13.x and newer. I will follow up
> with separate issues and patches.
> ---
> arch/riscv/Kconfig | 22 ++++++++++++++++++++++
> arch/riscv/Makefile | 10 ++++++----
> 2 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index c5e42cc37604..5b182d1c196c 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -464,6 +464,28 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE
> depends on !32BIT || $(cc-option,-mabi=ilp32 -march=rv32ima_zihintpause)
> depends on LLD_VERSION >= 150000 || LD_VERSION >= 23600
>
> +config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
> + def_bool y
> + # https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc
> + 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
> + extensions.
> +
> +config TOOLCHAIN_NEEDS_OLD_ISA_SPEC
> + def_bool y
> + depends on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI
> + # https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16
> + depends on CC_IS_CLANG && CLANG_VERSION < 170000
> + help
> + Certain versions of clang do not support zicsr and zifencei via -march
> + but newer versions of binutils require it for the reasons noted in the
> + help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This
> + option causes an older ISA spec compatible with these older versions
> + of clang to be passed to GAS, which has the same result as passing zicsr
> + and zifencei to -march.
> +
> config FPU
> bool "FPU support"
> default y
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 4de83b9b1772..b05e833a022d 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -57,10 +57,12 @@ 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
> +ifdef CONFIG_TOOLCHAIN_NEEDS_OLD_ISA_SPEC
> +KBUILD_CFLAGS += -Wa,-misa-spec=2.2
> +KBUILD_AFLAGS += -Wa,-misa-spec=2.2
> +else
> +riscv-march-$(CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI) := $(riscv-march-y)_zicsr_zifencei
> +endif
>
> # Check if the toolchain supports Zihintpause extension
> riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause
>
> ---
> base-commit: eeac8ede17557680855031c6f305ece2378af326
> change-id: 20230313-riscv-zicsr-zifencei-fiasco-2941caebe7dc

Is that a b4 thing? Having change IDs with names is nice, it's way
easier to remember what's what when sorting through backports.

Also, this is on fixes.

Thanks!

> Best regards,

2023-03-23 20:56:53

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] riscv: Handle zicsr/zifencei issues between clang and binutils


On Mon, 13 Mar 2023 16:00:23 -0700, Nathan Chancellor wrote:
> There are two related issues that appear in certain combinations with
> clang and GNU binutils.
>
> The first occurs when a version of clang that supports zicsr or zifencei
> via '-march=' [1] (i.e, >= 17.x) is used in combination with a version
> of GNU binutils that do not recognize zicsr and zifencei in the
> '-march=' value (i.e., < 2.36):
>
> [...]

Applied, thanks!

[1/1] riscv: Handle zicsr/zifencei issues between clang and binutils
https://git.kernel.org/palmer/c/e89c2e815e76

Best regards,
--
Palmer Dabbelt <[email protected]>

2023-03-23 21:02:43

by Conor Dooley

[permalink] [raw]
Subject: b4 send (was Re: [PATCH] riscv: Handle zicsr/zifencei issues between clang and binutils)

On Thu, Mar 23, 2023 at 01:49:57PM -0700, Palmer Dabbelt wrote:
> On Mon, 13 Mar 2023 16:00:23 PDT (-0700), [email protected] wrote:

> > base-commit: eeac8ede17557680855031c6f305ece2378af326
> > change-id: 20230313-riscv-zicsr-zifencei-fiasco-2941caebe7dc
>
> Is that a b4 thing? Having change IDs with names is nice, it's way easier
> to remember what's what when sorting through backports.

It's `b4 send`, for anyone that's lurking on the list and hasn't seen it
before: https://b4.docs.kernel.org/en/latest/contributor/send.html

b4 is now a tool for contributing, not just maintaining, although I'm not
sure that the distro copies of it are recent enough to make use of those
features.


Attachments:
(No filename) (711.00 B)
signature.asc (235.00 B)
Download all attachments
Subject: Re: [PATCH] riscv: Handle zicsr/zifencei issues between clang and binutils

Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <[email protected]>:

On Mon, 13 Mar 2023 16:00:23 -0700 you wrote:
> There are two related issues that appear in certain combinations with
> clang and GNU binutils.
>
> The first occurs when a version of clang that supports zicsr or zifencei
> via '-march=' [1] (i.e, >= 17.x) is used in combination with a version
> of GNU binutils that do not recognize zicsr and zifencei in the
> '-march=' value (i.e., < 2.36):
>
> [...]

Here is the summary with links:
- riscv: Handle zicsr/zifencei issues between clang and binutils
https://git.kernel.org/riscv/c/e89c2e815e76

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html