Re: [PATCH 2/2] RISC-V: re-enable gcc + rust builds

Yo,

On Fri, Sep 05, 2025 at 06:56:35AM +0800, Asuna wrote:

quoted

One thing - please don't send new versions
of patchsets in response to earlier versions or other threads. It
doesn't do you any favours with mailbox visibility.

I apologize for this, I'm pretty much new to mailing lists, so I had
followed the step "Explicit In-Reply-To headers" [1] in doc. For future
patches I'll send them alone instead of replying to existing threads.

[1]: https://www.kernel.org/doc/html/v6.9/process/submitting-patches.html#explicit-in-reply-to-headers

quoted

Other than Zicsr/Zifencei that may need explicit handling in a dedicated
option, the approach here seems kinda backwards.
Individually these symbols don't actually mean what they say they do,
which is confusing: "recognises" here is true even when it may not be
true at all because TOOLCHAIN_HAS_FOO is not set. Why can these options
not be removed, and instead the TOOLCHAIN_HAS_FOO options grow a
"depends on !RUST || <condition>"?

Yes, it's kinda "backwards", which is intentional, based on the following
considerations:

1) As mentioned in rust/Makefile, filtering flags for libclang is a hack,
because currently bindgen only has libclang as backend, and ideally bindgen
should support GCC so that the passed CC flags are supposed to be fully
compatible. On the RISC-V side, I tend to think that version checking for
extensions for libclang is also a hack, which could have been accomplished
with just the cc-option function, ideally.

2) Rust bindgen only "generates" FFI stuff, it is not involved in the final
assembly stage. In other words, it doesn't matter so much what RISC-V
extensions to turn on for bindgen (although it does have a little impact,
like some macro switches), it's more matter to CC.

Therefore, I chose not to modify the original extension config conditions so
that if libclang doesn't support the CC flag for an extension, then the Rust
build is not supported, rather than treating the extension as not supported.

I don't agree with this take, I don't think that any extension should
"blindly" take priority over rust like this. Got two or three main gripes
with how it is being done here.
Firstly, you're lumping every extension into one option even though many
of them will not be even implemented on the target. There's no need to
disable rust if the user has no intention of even making use of the
extension that would block its use. That runs into the second point, in
that you're using TOOLCHAIN_HAS_FOO here, which is only an indicator of
whether the toolchain supports the extension not whether the kernel is
even going to use it. The third problem I have is that the symbol you're
interacting with is not user selectable, and therefore doesn't allow the
user to decide whether or not a particular extension or rust support
with the toolchain they have is the higher priority. If the check moves
to the individual TOOLCHAIN_HAS_FOO options, they could be a

	depends on !RUST || <condition>

which would allow the user to make a decision about which has a greater
priority while also handling the extensions individually.

Nonetheless, it occurred to me as I was writing this reply that if GCC
implements a new extension in the future that LLVM/Clang doesn't yet have,
this could once again lead to a break in GCC+Rust build support if the
kernel decides to use the new extension. So it's a trade-off, you guys
decide, I'm fine with both.

Regarding the name, initially I named it "compatible", and ended up changed
it to "recognize" before sending the patch. If we continue on this path, I'm
not sure what name is appropriate to use here, do you guys have any ideas?

quoted

What does the libclang >= 17 requirement actually do here? Is that the
version where llvm starts to require that Zicsr/Zifencei is set in order
to use them? I think a comment to that effect is required if so. This
doesn't actually need to be blocking either, should just be able to
filter it out of march when passing to bindgen, no?

libclang >= 17 starts recognizing Zicsr/Zifencei in -march, passing them to
-march doesn't generate an error, and passing them or not doesn't have any
real difference. (still follows ISA before version 20190608 --
Zicsr/Zifencei are included in base ISA). I should have written a comment
there to avoid confusion.

Reference commit in LLVM/Clang 22e199e6af ("[RISCV] Accept zicsr and
zifencei command line options")
https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16

quoted

What about the case where TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI is not
set at all? Currently your patch is going to block rust in that case,
when actually nothing needs to be done at all - no part of the toolchain
requires understanding Zicsr/Zifencei as standalone extensions in this
case.

This is a bug, I missed this case. So it should be corrected to:

   config RUST_BINDGEN_LIBCLANG_RECOGNIZES_ZICSR_ZIFENCEI
        def_bool y
        depends on TOOLCHAIN_NEEDS_OLD_ISA_SPEC ||
   !TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI ||
   RUST_BINDGEN_LIBCLANG_VERSION >= 170000

quoted

The TOOLCHAIN_NEEDS_OLD_ISA_SPEC handling I don't remember 100% how it
works, but if bindgen requires them to be set to use the extension
this will return true but do nothing to add the extensions to march?
That seems wrong to me.
I'd be fairly amenable to disabling rust though when used in combination
with gcc < 11.3 and gas >=2.36 since it's such a niche condition, rather
doing work to support it. That'd be effectively an inversion of your
first condition.

The current latest version of LLVM/Clang still does not require explicit
Zicsr/Zifence to enable these two extensions, Clang just accepts them in
-march and then silently ignores them.

Checking the usage of CONFIG_TOOLCHAIN_NEEDS_OLD_ISA_SPEC:

   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

It just uses -Wa to force an older ISA version to GAS. So the
RUST_BINDGEN_LIBCLANG_RECOGNIZES_ZICSR_ZIFENCEI I corrected above should be
fine now I guess? Or would you still prefer your idea of blocking Rust if
TOOLCHAIN_NEEDS_OLD_ISA_SPEC is true?

Nah, if the explicit setting isn't required then it should be fine to
not block on it being used. To be honest, I'm not concerned about
Zicsr/Zifencei being communicated across to bindgen as much as I would
be about other extensions, my motivation here is regarding build
breakages - in particular when things like TOOLCHAIN_NEEDS_OLD_ISA_SPEC
is set, since it's a very niche configuration that if someone told me
they were using I would tell them to stop. As I said, the original
reason for this existing was to support w/e old version of debian linaro
were using that could not do LLVM=1 builds and I think the person who
added to this handle gcc with older binutils was trying to do a gradual
move from an old toolchain in steps to a modern one, so neither were
instances of someone actually wanting to use such a strange mix.

(To be clear, the breaking changes regarding Zicsr/Zifence are since ISA
version 20190608, and versions 2.0, 2.1, 2.2 are older than 20190608)

The only thing I'm confused about is that according to the comment of
TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI, GCC-12.1.0 bumped the default ISA
to 20191213, but why doesn't the depends-on have condition || (CC_IS_GCC &&
GCC_VERSION >= 120100)?

It's probably something along the lines of there being no _C_ code that
produces the Zicsr and Zifencei instructions, and therefore no build
errors produced if they're missing. That's part of why I said my
motivation in this particular case is build breakage, more than anything
else.