public inbox for llvm@lists.linux.dev
 help / color / mirror / Atom feed
From: Bin Meng <bmeng.cn@gmail.com>
To: conor@kernel.org
Cc: aurelien@aurel32.net, conor.dooley@microchip.com,
	linux-riscv@lists.infradead.org, llvm@lists.linux.dev,
	naresh.kamboju@linaro.org, nathan@kernel.org,
	ndesaulniers@google.com, palmer@dabbelt.com, palmer@rivosinc.com,
	stable@vger.kernel.org
Subject: Re: [PATCH v2] RISC-V: remove I-extension ISA spec version dance
Date: Fri, 10 Mar 2023 23:07:54 +0800	[thread overview]
Message-ID: <20230310150754.535425-1-bmeng.cn@gmail.com> (raw)
In-Reply-To: <20230308220842.1231003-1-conor@kernel.org>

> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The spec folk, in their infinite wisdom, moved both control and status
> registers & the FENCE.I instructions out of the I extension into their
> own extensions (Zicsr, Zifencei) in the 20190608 version of the ISA
> spec [0].
> The GCC/binutils crew decided [1] to move their default version of the
> ISA spec to the 20191213 version of the ISA spec, which came into being
> for version 2.38 of binutils and GCC 12. Building with this toolchain
> configuration would result in assembler issues:
>   CC      arch/riscv/kernel/vdso/vgettimeofday.o
>   <<BUILDDIR>>/arch/riscv/include/asm/vdso/gettimeofday.h: Assembler messages:
>   <<BUILDDIR>>/arch/riscv/include/asm/vdso/gettimeofday.h:71: Error: unrecognized opcode `csrr a5,0xc01'
>   <<BUILDDIR>>/arch/riscv/include/asm/vdso/gettimeofday.h:71: Error: unrecognized opcode `csrr a5,0xc01'
>   <<BUILDDIR>>/arch/riscv/include/asm/vdso/gettimeofday.h:71: Error: unrecognized opcode `csrr a5,0xc01'
>   <<BUILDDIR>>/arch/riscv/include/asm/vdso/gettimeofday.h:71: Error: unrecognized opcode `csrr a5,0xc01'
> This was fixed in commit 6df2a016c0c8 ("riscv: fix build with binutils
> 2.38") by Aurelien Jarno, but has proven fragile.
> 
> Before LLVM 17, LLVM did not support these extensions and, as such, the
> cc-option check added by Aurelien worked. Since commit 22e199e6afb1
> ("[RISCV] Accept zicsr and zifencei command line options") however, LLVM
> *does* support them and the cc-option check passes.
> 
> This surfaced as a problem while building the 5.10 stable kernel using
> the default Tuxmake Debian image [2], as 5.10 did not yet support ld.lld,
> and uses the Debian provided binutils 2.35.
> Versions of ld prior to 2.38 will refuse to link if they encounter
> unknown ISA extensions, and unfortunately Zifencei is not supported by
> bintuils 2.35.
> 
> Instead of dancing around with adding these extensions to march, as we
> currently do, Palmer suggested locking GCC builds to the same version of
> the ISA spec that is used by LLVM. As far as I can tell, that is 2.2,
> with, apparently [3], a lack of interest in implementing a flag like
> GCC's -misa-spec at present.
> 
> Add {cc,as}-option checks to add -misa-spec to KBUILD_{A,C}FLAGS when
> GCC is used & remove the march dance.
> 
> As clang does not accept this argument, I had expected to encounter
> issues with the assembler, as neither zicsr nor zifencei are present in
> the ISA string and the spec version *should* be defaulting to a version
> that requires them to be present. The build passed however and the
> resulting kernel worked perfectly fine for me on a PolarFire SoC...
> 
> Link: https://riscv.org/wp-content/uploads/2019/06/riscv-spec.pdf [0]
> Link: https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/aE1ZeHHCYf4 [1]
> Link: https://lore.kernel.org/all/CA+G9fYt9T=ELCLaB9byxaLW2Qf4pZcDO=huCA0D8ug2V2+irJQ@mail.gmail.com/ [2]
> Link: https://discourse.llvm.org/t/specifying-unpriviledge-spec-version-misa-spec-gcc-flag-equivalent/66935 [3]
> CC: stable@vger.kernel.org
> Suggested-by: Palmer Dabbelt <palmer@rivosinc.com>
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> I think Aurelien's original commit message might actually not be quite
> correct? I found, in my limited testing, that it is not the default
> behaviour of gas that matters, but rather the toolchain itself?
> My binutils versions (both those built using the clang-built-linux
> tc-build scripts which do not set an ISA spec version, and one built
> using the riscv-gnu-toolchain infra w/ an explicit 20191213 spec version
> set) do not encounter these issues.

I am unable to reproduce the build failure as reported by commit 6df2a016c0c8
("riscv: fix build with binutils 2.38") by Aurelien Jarno using kernel.org
pre-built GCC 11.3.0 [1] which includes binutils 2.38.

[1] https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.3.0/x86_64-gcc-11.3.0-nolibc-x86_64-linux.tar.xz

The defconfig of v5.16 kernel (commit 6df2a016c0c8 lands in v5.17) builds fine
for me. Anything I am missing?

> >From *my* testing, I was only able to reproduce the above build failures
> because of *GCC* defaulting to a newer ISA spec version, and saw no
> issues with CC=clang builds, where -misa-spec is not (AFAICT) passed to
> gas.
> I'm far from a toolchain person, so I am very very happy to have the
> reason for that explained to me, as I've been scratching my head about
> it all evening.
> 
> I'm also not super confident in my "as-option"ing, but it worked for me,
> so it's gotta be perfect, right? riiight??
> 
> Changes from v1:
> - entirely new approach to the issue
> ---
>  arch/riscv/Makefile | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 6203c3378922..2df7a5dc071c 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -57,10 +57,8 @@ 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
> +KBUILD_CFLAGS += $(call cc-option,-misa-spec=2.2)
> +KBUILD_AFLAGS += $(call as-option,-Wa$(comma)-misa-spec=2.2)
>  
>  # Check if the toolchain supports Zihintpause extension
>  riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause
> -- 

  parent reply	other threads:[~2023-03-10 15:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-08 22:08 [PATCH v2] RISC-V: remove I-extension ISA spec version dance Conor Dooley
2023-03-09  2:19 ` Jessica Clarke
2023-03-09  6:19   ` Conor Dooley
2023-03-10 15:07 ` Bin Meng [this message]
2023-03-10 15:35   ` Bin Meng
2023-03-10 16:40     ` Conor Dooley
2023-03-11  5:41       ` Bin Meng
2023-03-11 10:49         ` Aurelien Jarno
2023-03-11 13:09           ` Bin Meng
2023-03-11 10:11       ` Aurelien Jarno
2023-03-11 10:40         ` Conor Dooley
2023-03-11 10:54           ` Aurelien Jarno
2023-03-11  8:54     ` Aurelien Jarno
2023-03-11  8:52   ` Aurelien Jarno

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230310150754.535425-1-bmeng.cn@gmail.com \
    --to=bmeng.cn@gmail.com \
    --cc=aurelien@aurel32.net \
    --cc=conor.dooley@microchip.com \
    --cc=conor@kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=llvm@lists.linux.dev \
    --cc=naresh.kamboju@linaro.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=palmer@dabbelt.com \
    --cc=palmer@rivosinc.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox