Message ID | 20220523020209.11810-1-ojeda@kernel.org (mailing list archive) |
---|---|
Headers | show |
Series | Rust support | expand |
On Mon, May 23, 2022 at 4:42 PM Gary Guo <gary@garyguo.net> wrote: > > On Mon, 23 May 2022 11:37:16 -0700 > Nick Desaulniers <ndesaulniers@google.com> wrote: > > > Also, I'm not sure my concern about explicit build failures for C code > > was ever addressed? We have a constant problem with `long long` > > division on ARCH=arm32 and ARCH=i386 in C code. > > https://lore.kernel.org/lkml/CAKwvOdk+A2PBdjSFVUhj4xyCGCKujtej1uPgywQgrKPiK2ksPw@mail.gmail.com/ > > > > > +#[cfg(target_arch = "arm")] > > > +define_panicking_intrinsics!("`u64` division/modulo should not be > > > used", { > > > + __aeabi_uldivmod, > > > + __mulodi4, > > > +}); > > Starting in LLVM 14 (used in Rust 1.60+), __mulodi4 will no longer be > generated. So that can be removed. I'm familiar, but good catch. ;) https://reviews.llvm.org/D108936 https://reviews.llvm.org/D108842 https://reviews.llvm.org/D108844 https://reviews.llvm.org/D112750 https://reviews.llvm.org/D108928 https://reviews.llvm.org/D108939 https://bugs.llvm.org/show_bug.cgi?id=28629 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103034 > > As for __aeabi_uldivmod, is there any reason that it can't just be > defined in arch/arm/lib? There are quite a few __aeabi functions already > defined there. Indeed. arch/arm/kernel/armksyms.c and arch/arm/lib/lib1funcs.S This is the previous thread I recall w/ Linus: https://lore.kernel.org/llvm/CAHk-=wiydA=Oay+NB2m2ewCHpPEcoU51qPFrzsekFoPu7QPtuw@mail.gmail.com/ If CONFIG_RUST provides those symbols, it will hide the linkage failures that we try to use to spot & avoid 64b division that's open coded using the / binary operator, rather than the kernel's do_div() and friends. > > The source of __aeabi_uldivmod in compiler-rt seems quite simple, just > delegating to __uldivmoddi4. I think just changing that to > div64_u64_rem should do the job? Maybe; send a patch and see what happens. There's probably other 32b architectures that will need other symbols that also handle 64b division though, so it's not as simple as providing __aeabi_uldivmod for ARM. There's probably someone from linux-arm-kernel that can provide additional context. https://lore.kernel.org/linux-arm-kernel/alpine.LFD.2.00.1104271305580.24613@xanadu.home/ is pretty old, but refers to policies that seem to pre-exist other references to __aeabi_uldivmod on that list. arch/nios2/kernel/nios2_ksyms.c exports __udivmoddi4, but it also explicitly links against libgcc. I'm guessing that's frowned upon, but not out of the question relative to having the kernel ported to the architecture at all. > > https://android.googlesource.com/toolchain/compiler-rt/+/release_32/lib/arm/aeabi_uldivmod.S Here's the latest source. https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/builtins/arm/aeabi_uldivmod.S and __uldivmoddi4: https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/builtins/udivmoddi4.c By chance, does any of the rust code in this series use open coded division w/ 64 bit operands (rather than using do_div) by accident? I'm also curious about the panic message for 128b operands. IIUC, those are functions that may have `long long` operands. On 32b ARM, which is ILP32, I'd have expected `long long` to be 64b, not 128b. Message might be misleading.
Hey, Maybe I am just missing something blatantly obvious here, but trying to build rust support in -next fails for me. I am using ClangBuiltLinux clang version 15.0.0 5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa and LLD 15.0.0. Is it just expected that building -next with rust support is not a good idea? My defconfig is the default RISC-V one plus: CONFIG_RUST=y CONFIG_SAMPLES=y CONFIG_SAMPLES_RUST=y CONFIG_SAMPLE_RUST_MINIMAL=y Thanks, Conor. Fail log: UPD rust/target.json BINDGEN rust/bindings_generated.rs BINDGEN rust/bindings_helpers_generated.rs RUSTC L rust/core.o EXPORTS rust/exports_core_generated.h RUSTC P rust/libmacros.so RUSTC L rust/compiler_builtins.o RUSTC L rust/alloc.o RUSTC L rust/build_error.o EXPORTS rust/exports_alloc_generated.h RUSTC L rust/kernel.o error[E0428]: the name `maple_enode` is defined multiple times --> linux/rust/bindings_generated.rs:18009:1 | 18006 | pub struct maple_enode { | ---------------------- previous definition of the type `maple_enode` here ... 18009 | pub type maple_enode = *mut maple_enode; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `maple_enode` redefined here | = note: `maple_enode` must be defined only once in the type namespace of this module error[E0428]: the name `maple_pnode` is defined multiple times --> linux/rust/bindings_generated.rs:18015:1 | 18012 | pub struct maple_pnode { | ---------------------- previous definition of the type `maple_pnode` here ... 18015 | pub type maple_pnode = *mut maple_pnode; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `maple_pnode` redefined here | = note: `maple_pnode` must be defined only once in the type namespace of this module error[E0391]: cycle detected when expanding type alias `bindings::bindings_raw::maple_pnode` --> linux/rust/bindings_generated.rs:18015:29 | 18015 | pub type maple_pnode = *mut maple_pnode; | ^^^^^^^^^^^ | = note: ...which immediately requires expanding type alias `bindings::bindings_raw::maple_pnode` again = note: type aliases cannot be recursive = help: consider using a struct, enum, or union instead to break the cycle = help: see <https://doc.rust-lang.org/reference/types.html#recursive-types> for more information note: cycle used when computing type of `bindings::bindings_raw::maple_range_64::parent` --> linux/rust/bindings_generated.rs:18058:22 | 18058 | pub parent: *mut maple_pnode, | ^^^^^^^^^^^ error[E0391]: cycle detected when expanding type alias `bindings::bindings_raw::maple_enode` --> linux/rust/bindings_generated.rs:18009:29 | 18009 | pub type maple_enode = *mut maple_enode; | ^^^^^^^^^^^ | = note: ...which immediately requires expanding type alias `bindings::bindings_raw::maple_enode` again = note: type aliases cannot be recursive = help: consider using a struct, enum, or union instead to break the cycle = help: see <https://doc.rust-lang.org/reference/types.html#recursive-types> for more information note: cycle used when computing type of `bindings::bindings_raw::maple_topiary::next` --> linux/rust/bindings_generated.rs:18340:20 | 18340 | pub next: *mut maple_enode, | ^^^^^^^^^^^ error[E0117]: only traits defined in the current crate can be implemented for arbitrary types --> linux/rust/bindings_generated.rs:18005:10 | 18005 | #[derive(Copy, Clone)] | ^^^^ | | | impl doesn't use only types from inside the current crate | `*mut [type error]` is not defined in the current crate | = note: define and implement a trait or new type instead = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0117]: only traits defined in the current crate can be implemented for arbitrary types --> linux/rust/bindings_generated.rs:18011:10 | 18011 | #[derive(Copy, Clone)] | ^^^^ | | | impl doesn't use only types from inside the current crate | `*mut [type error]` is not defined in the current crate | = note: define and implement a trait or new type instead = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0117]: only traits defined in the current crate can be implemented for arbitrary types --> linux/rust/bindings_generated.rs:18005:16 | 18005 | #[derive(Copy, Clone)] | ^^^^^ | | | impl doesn't use only types from inside the current crate | `*mut [type error]` is not defined in the current crate | = note: define and implement a trait or new type instead = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0117]: only traits defined in the current crate can be implemented for arbitrary types --> linux/rust/bindings_generated.rs:18011:16 | 18011 | #[derive(Copy, Clone)] | ^^^^^ | | | impl doesn't use only types from inside the current crate | `*mut [type error]` is not defined in the current crate | = note: define and implement a trait or new type instead = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 8 previous errors
Hi Conor, On Sat, Jul 16, 2022 at 2:42 PM Conor Dooley <conor@kernel.org> wrote: > > Maybe I am just missing something blatantly obvious here, but trying > to build rust support in -next fails for me. I am using ClangBuiltLinux > clang version 15.0.0 5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa and LLD > 15.0.0. Is it just expected that building -next with rust support is > not a good idea? Please see https://github.com/Rust-for-Linux/linux/issues/795 for details about the maple tree issue. I will update the `rust-next` branch next week with the new version of the patches; but if you are interested in developing, please use the development `rust` branch instead in GitHub for the moment. Cheers, Miguel
On 16/07/2022 14:36, Miguel Ojeda wrote: > Hi Conor, > > On Sat, Jul 16, 2022 at 2:42 PM Conor Dooley <conor@kernel.org> wrote: >> >> Maybe I am just missing something blatantly obvious here, but trying >> to build rust support in -next fails for me. I am using ClangBuiltLinux >> clang version 15.0.0 5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa and LLD >> 15.0.0. Is it just expected that building -next with rust support is >> not a good idea? > > Please see https://github.com/Rust-for-Linux/linux/issues/795 for > details about the maple tree issue. Ah right, sorry for the noise so. I checked the ml but didn't see a report there. > > I will update the `rust-next` branch next week with the new version of > the patches; but if you are interested in developing, please use the > development `rust` branch instead in GitHub for the moment. Thanks Miguel, good to know! I'll just wait around for a new version. Just been trying to get my CI etc in order for when rust support lands, but it sounds like I should be okay as it's a known problem & not some only-broken-on-riscv thing. Thanks, Conor.
On Sat, Jul 16, 2022 at 3:51 PM <Conor.Dooley@microchip.com> wrote: > > Ah right, sorry for the noise so. I checked the ml but didn't see a > report there. No apologies needed -- thanks to you for the report, instead! :) > Thanks Miguel, good to know! I'll just wait around for a new version. > Just been trying to get my CI etc in order for when rust support lands, > but it sounds like I should be okay as it's a known problem & not some > only-broken-on-riscv thing. Yeah, it is a simple `bindgen` issue. Thanks a lot for making the effort to prepare your CI in advance! Cheers, Miguel