[PATCH] RISC-V: Fix building rust when using GCC toolchain

[PATCH] RISC-V: Fix building rust when using GCC toolchain

[Jason Montleon]

Clang does not support '-mno-riscv-attribute' resulting in the error
error: unknown argument: '-mno-riscv-attribute'

Not setting BINDGEN_TARGET_riscv results in the in the error
error: unsupported argument 'medany' to option '-mcmodel=' for target \
'unknown'
error: unknown target triple 'unknown'

Signed-off-by: Jason Montleon <jmontleo@redhat.com>
Cc: stable@vger.kernel.org
---
 rust/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/rust/Makefile b/rust/Makefile
index f168d2c98a15..73eceaaae61e 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -228,11 +228,12 @@ bindgen_skip_c_flags := -mno-fp-ret-in-387 -mpreferred-stack-boundary=% \
 	-fzero-call-used-regs=% -fno-stack-clash-protection \
 	-fno-inline-functions-called-once -fsanitize=bounds-strict \
 	-fstrict-flex-arrays=% -fmin-function-alignment=% \
-	--param=% --param asan-%
+	--param=% --param asan-% -mno-riscv-attribute
 
 # Derived from `scripts/Makefile.clang`.
 BINDGEN_TARGET_x86	:= x86_64-linux-gnu
 BINDGEN_TARGET_arm64	:= aarch64-linux-gnu
+BINDGEN_TARGET_riscv	:= riscv64-linux-gnu
 BINDGEN_TARGET		:= $(BINDGEN_TARGET_$(SRCARCH))
 
 # All warnings are inhibited since GCC builds are very experimental,

base-commit: ad060dbbcfcfcba624ef1a75e1d71365a98b86d8
-- 
2.46.0

Re: [PATCH] RISC-V: Fix building rust when using GCC toolchain

[Conor Dooley]

On 17 September 2024 01:08:48 IST, Jason Montleon <jmontleo@redhat.com> wrote:
>Clang does not support '-mno-riscv-attribute' resulting in the error
>error: unknown argument: '-mno-riscv-attribute'

This appears to conflict with your subject, which cities gcc, but I suspect that's due to poor wording of the body of the commit message than a mistake in the subject.
I'd rather disable rust on riscv when building with gcc, I've never been satisfied with the interaction between gcc and rustc's libclang w.r.t. extensions.

Cheers,
Conor.

>
>Not setting BINDGEN_TARGET_riscv results in the in the error
>error: unsupported argument 'medany' to option '-mcmodel=' for target \
>'unknown'
>error: unknown target triple 'unknown'
>
>Signed-off-by: Jason Montleon <jmontleo@redhat.com>
>Cc: stable@vger.kernel.org
>---
> rust/Makefile | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/rust/Makefile b/rust/Makefile
>index f168d2c98a15..73eceaaae61e 100644
>--- a/rust/Makefile
>+++ b/rust/Makefile
>@@ -228,11 +228,12 @@ bindgen_skip_c_flags := -mno-fp-ret-in-387 -mpreferred-stack-boundary=% \
> 	-fzero-call-used-regs=% -fno-stack-clash-protection \
> 	-fno-inline-functions-called-once -fsanitize=bounds-strict \
> 	-fstrict-flex-arrays=% -fmin-function-alignment=% \
>-	--param=% --param asan-%
>+	--param=% --param asan-% -mno-riscv-attribute
> 
> # Derived from `scripts/Makefile.clang`.
> BINDGEN_TARGET_x86	:= x86_64-linux-gnu
> BINDGEN_TARGET_arm64	:= aarch64-linux-gnu
>+BINDGEN_TARGET_riscv	:= riscv64-linux-gnu
> BINDGEN_TARGET		:= $(BINDGEN_TARGET_$(SRCARCH))
> 
> # All warnings are inhibited since GCC builds are very experimental,
>
>base-commit: ad060dbbcfcfcba624ef1a75e1d71365a98b86d8

Re: [PATCH] RISC-V: Fix building rust when using GCC toolchain

[Gary Guo]

On Tue, 17 Sep 2024 10:35:12 +0100
Conor Dooley <conor@kernel.org> wrote:

> On 17 September 2024 01:08:48 IST, Jason Montleon <jmontleo@redhat.com> wrote:
> >Clang does not support '-mno-riscv-attribute' resulting in the error
> >error: unknown argument: '-mno-riscv-attribute'  
> 
> This appears to conflict with your subject, which cities gcc, but I suspect that's due to poor wording of the body of the commit message than a mistake in the subject.
> I'd rather disable rust on riscv when building with gcc, I've never been satisfied with the interaction between gcc and rustc's libclang w.r.t. extensions.
> 
> Cheers,
> Conor.

Hi Conor,

What happens is that when building against GCC, Kbuild gathers flag
assuming CC is GCC, but bindgen uses clang instead. In this case, the
CC is GCC and all C code is built by GCC. We have a filtering mechanism
to only give bindgen (libclang) flags that it can understand.

While I do think this is a bit fragile, this is what I think all
distros that enable Rust use. They still prefer to build C code with
GCC. So I hope we can still keep that option around.

Best,
Gary


> 
> >
> >Not setting BINDGEN_TARGET_riscv results in the in the error
> >error: unsupported argument 'medany' to option '-mcmodel=' for target \
> >'unknown'
> >error: unknown target triple 'unknown'
> >
> >Signed-off-by: Jason Montleon <jmontleo@redhat.com>
> >Cc: stable@vger.kernel.org
> >---
> > rust/Makefile | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/rust/Makefile b/rust/Makefile
> >index f168d2c98a15..73eceaaae61e 100644
> >--- a/rust/Makefile
> >+++ b/rust/Makefile
> >@@ -228,11 +228,12 @@ bindgen_skip_c_flags := -mno-fp-ret-in-387 -mpreferred-stack-boundary=% \
> > 	-fzero-call-used-regs=% -fno-stack-clash-protection \
> > 	-fno-inline-functions-called-once -fsanitize=bounds-strict \
> > 	-fstrict-flex-arrays=% -fmin-function-alignment=% \
> >-	--param=% --param asan-%
> >+	--param=% --param asan-% -mno-riscv-attribute
> > 
> > # Derived from `scripts/Makefile.clang`.
> > BINDGEN_TARGET_x86	:= x86_64-linux-gnu
> > BINDGEN_TARGET_arm64	:= aarch64-linux-gnu
> >+BINDGEN_TARGET_riscv	:= riscv64-linux-gnu
> > BINDGEN_TARGET		:= $(BINDGEN_TARGET_$(SRCARCH))
> > 
> > # All warnings are inhibited since GCC builds are very experimental,
> >
> >base-commit: ad060dbbcfcfcba624ef1a75e1d71365a98b86d8  

Re: [PATCH] RISC-V: Fix building rust when using GCC toolchain

[Conor Dooley]

On 17 September 2024 15:29:50 GMT+02:00, Gary Guo <gary@garyguo.net> wrote:
>On Tue, 17 Sep 2024 10:35:12 +0100
>Conor Dooley <conor@kernel.org> wrote:
>
>> On 17 September 2024 01:08:48 IST, Jason Montleon <jmontleo@redhat.com> wrote:
>> >Clang does not support '-mno-riscv-attribute' resulting in the error
>> >error: unknown argument: '-mno-riscv-attribute'  
>> 
>> This appears to conflict with your subject, which cities gcc, but I suspect that's due to poor wording of the body of the commit message than a mistake in the subject.
>> I'd rather disable rust on riscv when building with gcc, I've never been satisfied with the interaction between gcc and rustc's libclang w.r.t. extensions.
>> 
>> Cheers,
>> Conor.
>
>Hi Conor,
>
>What happens is that when building against GCC, Kbuild gathers flag
>assuming CC is GCC, but bindgen uses clang instead. In this case, the
>CC is GCC and all C code is built by GCC. We have a filtering mechanism
>to only give bindgen (libclang) flags that it can understand.

Yes, but unfortunately I already knew how it worked. It's not flags I am worried about, it is extensions.
Even using a libclang that doesn't match clang could be a problem, but we can at least declare that unsupported.
Not digging it out on an airport bus, but we discussed the lack of GCC support on the original patch adding riscv, and decided against it.

>
>While I do think this is a bit fragile, this is what I think all
>distros that enable Rust use. They still prefer to build C code with
>GCC. So I hope we can still keep that option around.
>
>Best,
>Gary
>
>
>> 
>> >
>> >Not setting BINDGEN_TARGET_riscv results in the in the error
>> >error: unsupported argument 'medany' to option '-mcmodel=' for target \
>> >'unknown'
>> >error: unknown target triple 'unknown'
>> >
>> >Signed-off-by: Jason Montleon <jmontleo@redhat.com>
>> >Cc: stable@vger.kernel.org
>> >---
>> > rust/Makefile | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/rust/Makefile b/rust/Makefile
>> >index f168d2c98a15..73eceaaae61e 100644
>> >--- a/rust/Makefile
>> >+++ b/rust/Makefile
>> >@@ -228,11 +228,12 @@ bindgen_skip_c_flags := -mno-fp-ret-in-387 -mpreferred-stack-boundary=% \
>> > 	-fzero-call-used-regs=% -fno-stack-clash-protection \
>> > 	-fno-inline-functions-called-once -fsanitize=bounds-strict \
>> > 	-fstrict-flex-arrays=% -fmin-function-alignment=% \
>> >-	--param=% --param asan-%
>> >+	--param=% --param asan-% -mno-riscv-attribute
>> > 
>> > # Derived from `scripts/Makefile.clang`.
>> > BINDGEN_TARGET_x86	:= x86_64-linux-gnu
>> > BINDGEN_TARGET_arm64	:= aarch64-linux-gnu
>> >+BINDGEN_TARGET_riscv	:= riscv64-linux-gnu
>> > BINDGEN_TARGET		:= $(BINDGEN_TARGET_$(SRCARCH))
>> > 
>> > # All warnings are inhibited since GCC builds are very experimental,
>> >
>> >base-commit: ad060dbbcfcfcba624ef1a75e1d71365a98b86d8  
>

Re: [PATCH] RISC-V: Fix building rust when using GCC toolchain

[Miguel Ojeda]

On Tue, Sep 17, 2024 at 5:26 PM Conor Dooley <conor@kernel.org> wrote:
>
> Yes, but unfortunately I already knew how it worked. It's not flags I am worried about, it is extensions.
> Even using a libclang that doesn't match clang could be a problem, but we can at least declare that unsupported.
> Not digging it out on an airport bus, but we discussed the lack of GCC support on the original patch adding riscv, and decided against it.

Do you mean you would prefer to avoid supporting the mixed GCC-Clang
builds? If so, do you mean you would prefer to not pick the patch,
i.e. avoid supporting this at all? (If so, then perhaps it would be a
good idea to add a comment there and perhaps a note to
https://docs.kernel.org/rust/arch-support.html).

Otherwise, please let me know if I am misunderstanding -- thanks!

Cheers,
Miguel

Re: [PATCH] RISC-V: Fix building rust when using GCC toolchain

[Conor Dooley]

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

On Thu, Sep 26, 2024 at 05:40:19PM +0200, Miguel Ojeda wrote:
> On Tue, Sep 17, 2024 at 5:26 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > Yes, but unfortunately I already knew how it worked. It's not flags I am worried about, it is extensions.
> > Even using a libclang that doesn't match clang could be a problem, but we can at least declare that unsupported.
> > Not digging it out on an airport bus, but we discussed the lack of GCC support on the original patch adding riscv, and decided against it.
> 
> Do you mean you would prefer to avoid supporting the mixed GCC-Clang
> builds?

Mixed builds are allowed on the c side, since we can figure out what the
versions of each tool are. If there's a way to detect the version of
libclang in use by the rust side, then I would be okay with mixed gcc +
rustc builds.

> If so, do you mean you would prefer to not pick the patch,
> i.e. avoid supporting this at all?

Yes, I would rather this was not applied at all. My plan was to send a
patch making HAVE_RUST depend on CC_IS_CLANG, but just ain't got around
to it yet, partly cos I was kinda hoping to mention this to you guys at
LPC last week, but I never got the chance to talk to any rust people (or
go to any rust talks either!).

> (If so, then perhaps it would be a
> good idea to add a comment there and perhaps a note to
> https://docs.kernel.org/rust/arch-support.html).

Sure, I can add a comment there. 

> Otherwise, please let me know if I am misunderstanding -- thanks!

In sorta related news, is there a plan for config "options" that will
allow us to detect gcc-rs or the gcc rust backend?

Cheers,
Conor.

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

Re: [PATCH] RISC-V: Fix building rust when using GCC toolchain

[Miguel Ojeda]

On Thu, Sep 26, 2024 at 5:56 PM Conor Dooley <conor@kernel.org> wrote:
>
> Mixed builds are allowed on the c side, since we can figure out what the

I am not sure what you mean by allowed on the C side. For out-of-tree
modules you mean?

> versions of each tool are. If there's a way to detect the version of
> libclang in use by the rust side, then I would be okay with mixed gcc +
> rustc builds.

If you mean the libclang used by bindgen, yes, we have such a check
(warning) in scripts/rust_is_available.sh. We can also have it as a
Kconfig symbol if needed.

Regarding rustc's LLVM version, I wanted to have a similar check in
scripts/rust_is_available.sh (also a warning), but it would have been
quite noisy, and if LTO is not enabled it should generally be OK. So
we are adding instead a Kconfig symbol for that, which will be used
for a couple things. Gary has a WIP patch for this one.

> Yes, I would rather this was not applied at all. My plan was to send a
> patch making HAVE_RUST depend on CC_IS_CLANG, but just ain't got around
> to it yet, partly cos I was kinda hoping to mention this to you guys at
> LPC last week, but I never got the chance to talk to any rust people (or
> go to any rust talks either!).

To be clear, that `depends on` would need to be only for RISC-V, i.e.
other architectures are "OK" with those. It is really, really
experimental, as we have always warned, but some people is using it
successfully, apparently.

> Sure, I can add a comment there.

Thanks!

> In sorta related news, is there a plan for config "options" that will
> allow us to detect gcc-rs or the gcc rust backend?

gccrs is way too early to even think about that. rustc_codegen_gcc,
yeah, if needed, we can add a symbol for that when we start supporting
the backend.

Cheers,
Miguel

Re: [PATCH] RISC-V: Fix building rust when using GCC toolchain

[Conor Dooley]

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

On Thu, Sep 26, 2024 at 06:11:17PM +0200, Miguel Ojeda wrote:
> On Thu, Sep 26, 2024 at 5:56 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > Mixed builds are allowed on the c side, since we can figure out what the
> 
> I am not sure what you mean by allowed on the C side. For out-of-tree
> modules you mean?

No. Things like clang + ld & gas.
I don't care about out of tree modules ;)

> > versions of each tool are. If there's a way to detect the version of
> > libclang in use by the rust side, then I would be okay with mixed gcc +
> > rustc builds.
> 
> If you mean the libclang used by bindgen, yes, we have such a check
> (warning) in scripts/rust_is_available.sh. We can also have it as a
> Kconfig symbol if needed.

Okay. Short term then is deny gcc + rust, longer term is allow it with
the same caveats as the aforementioned mixed stuff.

> Regarding rustc's LLVM version, I wanted to have a similar check in
> scripts/rust_is_available.sh (also a warning), but it would have been
> quite noisy, and if LTO is not enabled it should generally be OK. So
> we are adding instead a Kconfig symbol for that, which will be used
> for a couple things. Gary has a WIP patch for this one.

Cool, I'll check that out.

> > Yes, I would rather this was not applied at all. My plan was to send a
> > patch making HAVE_RUST depend on CC_IS_CLANG, but just ain't got around
> > to it yet, partly cos I was kinda hoping to mention this to you guys at
> > LPC last week, but I never got the chance to talk to any rust people (or
> > go to any rust talks either!).
> 
> To be clear, that `depends on` would need to be only for RISC-V, i.e.
> other architectures are "OK" with those. It is really, really
> experimental, as we have always warned, but some people is using it
> successfully, apparently.

Yes, just for riscv. The logic in our Kconfig menu is currently something
like
select HAVE_RUST if RUSTC_SUPPORTS_RISCV
so that would just become
select HAVE_RUST if RUSTC_SUPPORTS_RISCV && CC_IS_CLANG

> 
> > Sure, I can add a comment there.
> 
> Thanks!
> 
> > In sorta related news, is there a plan for config "options" that will
> > allow us to detect gcc-rs or the gcc rust backend?
> 
> gccrs is way too early to even think about that. rustc_codegen_gcc,
> yeah, if needed, we can add a symbol for that when we start supporting
> the backend.

Cool, sounds good.

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

Re: [PATCH] RISC-V: Fix building rust when using GCC toolchain

[Miguel Ojeda]

On Thu, Sep 26, 2024 at 6:21 PM Conor Dooley <conor@kernel.org> wrote:
>
> No. Things like clang + ld & gas.
> I don't care about out of tree modules ;)

Ah, other tools, not just the C compiler, I see -- that is why I
mentioned out-of-tree modules, because otherwise I was not sure how
you would get mix the C compiler itself, i.e. GCC and Clang.

> Yes, just for riscv. The logic in our Kconfig menu is currently something
> like
> select HAVE_RUST if RUSTC_SUPPORTS_RISCV
> so that would just become
> select HAVE_RUST if RUSTC_SUPPORTS_RISCV && CC_IS_CLANG

If that is what you think it is best for RISC-V, then sure, up to you -- thanks!

Cheers,
Miguel

Re: [PATCH] RISC-V: Fix building rust when using GCC toolchain

[Gary Guo]

On Mon, 16 Sep 2024 20:08:48 -0400
Jason Montleon <jmontleo@redhat.com> wrote:

> Clang does not support '-mno-riscv-attribute' resulting in the error
> error: unknown argument: '-mno-riscv-attribute'
> 
> Not setting BINDGEN_TARGET_riscv results in the in the error
> error: unsupported argument 'medany' to option '-mcmodel=' for target \
> 'unknown'
> error: unknown target triple 'unknown'
> 
> Signed-off-by: Jason Montleon <jmontleo@redhat.com>
> Cc: stable@vger.kernel.org

I also carry a similar patch locally (haven't get around to submit it
yet), so I can confirm

Tested-by: Gary Guo <garyguo.net>

As Conor points out, the commit message could be improved. Perhaps add
a bit of context about the flag filtering.

Best,
Gary

> ---
>  rust/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/Makefile b/rust/Makefile
> index f168d2c98a15..73eceaaae61e 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -228,11 +228,12 @@ bindgen_skip_c_flags := -mno-fp-ret-in-387 -mpreferred-stack-boundary=% \
>  	-fzero-call-used-regs=% -fno-stack-clash-protection \
>  	-fno-inline-functions-called-once -fsanitize=bounds-strict \
>  	-fstrict-flex-arrays=% -fmin-function-alignment=% \
> -	--param=% --param asan-%
> +	--param=% --param asan-% -mno-riscv-attribute
>  
>  # Derived from `scripts/Makefile.clang`.
>  BINDGEN_TARGET_x86	:= x86_64-linux-gnu
>  BINDGEN_TARGET_arm64	:= aarch64-linux-gnu
> +BINDGEN_TARGET_riscv	:= riscv64-linux-gnu
>  BINDGEN_TARGET		:= $(BINDGEN_TARGET_$(SRCARCH))
>  
>  # All warnings are inhibited since GCC builds are very experimental,
> 
> base-commit: ad060dbbcfcfcba624ef1a75e1d71365a98b86d8