| Message ID | 20240917000848.720765-2-jmontleo@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | RISC-V: Fix building rust when using GCC toolchain | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
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
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
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
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 >
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
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.
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
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.
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
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,
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(-) base-commit: ad060dbbcfcfcba624ef1a75e1d71365a98b86d8