From: Nick Desaulniers <ndesaulniers@google.com> To: Gary Guo <gary@garyguo.net> Cc: Miguel Ojeda <ojeda@kernel.org>, Linus Torvalds <torvalds@linux-foundation.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Jarkko Sakkinen <jarkko@kernel.org>, Alex Gaynor <alex.gaynor@gmail.com>, Wedson Almeida Filho <wedsonaf@google.com>, Sven Van Asbroeck <thesven73@gmail.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Arnd Bergmann <arnd@kernel.org> Subject: Re: [PATCH v7 06/25] rust: add `compiler_builtins` crate Date: Wed, 25 May 2022 14:29:51 -0700 [thread overview] Message-ID: <CAKwvOdmDgaG_DT+rr4F7xx=q=bVEaM9z7CBFqSq-0Eg=NwO02w@mail.gmail.com> (raw) In-Reply-To: <20220524004156.0000790e@garyguo.net> 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. -- Thanks, ~Nick Desaulniers
WARNING: multiple messages have this Message-ID (diff)
From: Nick Desaulniers <ndesaulniers@google.com> To: Gary Guo <gary@garyguo.net> Cc: Miguel Ojeda <ojeda@kernel.org>, Linus Torvalds <torvalds@linux-foundation.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Jarkko Sakkinen <jarkko@kernel.org>, Alex Gaynor <alex.gaynor@gmail.com>, Wedson Almeida Filho <wedsonaf@google.com>, Sven Van Asbroeck <thesven73@gmail.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Arnd Bergmann <arnd@kernel.org> Subject: Re: [PATCH v7 06/25] rust: add `compiler_builtins` crate Date: Wed, 25 May 2022 14:29:51 -0700 [thread overview] Message-ID: <CAKwvOdmDgaG_DT+rr4F7xx=q=bVEaM9z7CBFqSq-0Eg=NwO02w@mail.gmail.com> (raw) In-Reply-To: <20220524004156.0000790e@garyguo.net> 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. -- Thanks, ~Nick Desaulniers _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-05-25 21:30 UTC|newest] Thread overview: 100+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-23 2:01 [PATCH v7 00/25] Rust support Miguel Ojeda 2022-05-23 2:01 ` Miguel Ojeda 2022-05-23 2:01 ` Miguel Ojeda 2022-05-23 2:01 ` Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 01/25] kallsyms: avoid hardcoding the buffer size Miguel Ojeda 2022-05-23 19:45 ` Jarkko Sakkinen 2022-05-23 19:55 ` Jarkko Sakkinen 2022-05-24 16:21 ` Miguel Ojeda 2022-05-26 4:54 ` Jarkko Sakkinen 2022-05-23 2:01 ` [PATCH v7 02/25] kallsyms: support "big" kernel symbols Miguel Ojeda 2022-05-23 20:30 ` Jarkko Sakkinen 2022-05-23 2:01 ` [PATCH v7 03/25] kallsyms: increase maximum kernel symbol length to 512 Miguel Ojeda 2022-05-23 20:31 ` Jarkko Sakkinen 2022-05-24 18:07 ` Miguel Ojeda 2022-05-27 16:25 ` Jarkko Sakkinen 2022-05-30 13:01 ` Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 04/25] kunit: take `kunit_assert` as `const` Miguel Ojeda 2022-05-23 17:15 ` Daniel Latypov 2022-05-23 18:14 ` Shuah Khan 2022-05-24 12:37 ` Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 05/25] rust: add C helpers Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 06/25] rust: add `compiler_builtins` crate Miguel Ojeda 2022-05-23 18:37 ` Nick Desaulniers 2022-05-23 23:41 ` Gary Guo 2022-05-25 21:29 ` Nick Desaulniers [this message] 2022-05-25 21:29 ` Nick Desaulniers 2022-05-24 12:29 ` Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 07/25] rust: import upstream `alloc` crate Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 08/25] rust: adapt `alloc` crate to the kernel Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 09/25] rust: add `build_error` crate Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 10/25] rust: add `macros` crate Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 11/25] rust: add `kernel` crate's `sync` module Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 12/25] rust: add `kernel` crate Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 13/25] rust: export generated symbols Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 14/25] vsprintf: add new `%pA` format specifier Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 15/25] scripts: checkpatch: diagnose uses of `%pA` in the C side Miguel Ojeda 2022-05-23 2:17 ` Joe Perches 2022-05-24 16:35 ` Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 16/25] scripts: checkpatch: enable language-independent checks for Rust Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 17/25] scripts: add `rustdoc_test_{builder,gen}.py` scripts Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 18/25] scripts: add `generate_rust_analyzer.py` scripts Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 19/25] scripts: decode_stacktrace: demangle Rust symbols Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 20/25] docs: add Rust documentation Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 21/25] Kbuild: add Rust support Miguel Ojeda 2022-05-23 2:01 ` Miguel Ojeda 2022-05-23 2:01 ` Miguel Ojeda 2022-05-23 2:01 ` Miguel Ojeda 2022-05-23 18:44 ` Nick Desaulniers 2022-05-23 18:44 ` Nick Desaulniers 2022-05-23 18:44 ` Nick Desaulniers 2022-05-23 18:44 ` Nick Desaulniers 2022-05-23 18:44 ` Nick Desaulniers 2022-05-24 15:12 ` Miguel Ojeda 2022-05-24 15:12 ` Miguel Ojeda 2022-05-24 15:12 ` Miguel Ojeda 2022-05-24 15:12 ` Miguel Ojeda 2022-05-24 15:12 ` Miguel Ojeda 2022-05-25 22:25 ` Nick Desaulniers 2022-05-25 22:25 ` Nick Desaulniers 2022-05-25 22:25 ` Nick Desaulniers 2022-05-25 22:25 ` Nick Desaulniers 2022-05-25 22:25 ` Nick Desaulniers 2022-05-30 13:39 ` Miguel Ojeda 2022-05-30 13:39 ` Miguel Ojeda 2022-05-30 13:39 ` Miguel Ojeda 2022-05-30 13:39 ` Miguel Ojeda 2022-05-30 13:39 ` Miguel Ojeda 2022-07-16 8:21 ` Masahiro Yamada 2022-07-16 8:21 ` Masahiro Yamada 2022-07-16 8:21 ` Masahiro Yamada 2022-07-16 8:21 ` Masahiro Yamada 2022-07-16 8:57 ` Miguel Ojeda 2022-07-16 8:57 ` Miguel Ojeda 2022-07-16 8:57 ` Miguel Ojeda 2022-07-16 8:57 ` Miguel Ojeda 2022-07-16 8:57 ` Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 22/25] samples: add Rust examples Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 23/25] MAINTAINERS: Rust Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 24/25] [RFC] drivers: gpio: PrimeCell PL061 in Rust Miguel Ojeda 2022-05-23 2:01 ` [PATCH v7 25/25] [RFC] drivers: android: Binder IPC " Miguel Ojeda 2022-07-16 12:42 ` [PATCH v7 00/25] Rust support Conor Dooley 2022-07-16 12:42 ` Conor Dooley 2022-07-16 12:42 ` Conor Dooley 2022-07-16 12:42 ` Conor Dooley 2022-07-16 12:42 ` Conor Dooley 2022-07-16 13:36 ` Miguel Ojeda 2022-07-16 13:36 ` Miguel Ojeda 2022-07-16 13:36 ` Miguel Ojeda 2022-07-16 13:36 ` Miguel Ojeda 2022-07-16 13:36 ` Miguel Ojeda 2022-07-16 13:51 ` Conor.Dooley 2022-07-16 13:51 ` Conor.Dooley 2022-07-16 13:51 ` Conor.Dooley 2022-07-16 13:51 ` Conor.Dooley 2022-07-16 13:51 ` Conor.Dooley 2022-07-16 13:56 ` Miguel Ojeda 2022-07-16 13:56 ` Miguel Ojeda 2022-07-16 13:56 ` Miguel Ojeda 2022-07-16 13:56 ` Miguel Ojeda 2022-07-16 13:56 ` Miguel Ojeda
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='CAKwvOdmDgaG_DT+rr4F7xx=q=bVEaM9z7CBFqSq-0Eg=NwO02w@mail.gmail.com' \ --to=ndesaulniers@google.com \ --cc=alex.gaynor@gmail.com \ --cc=arnd@kernel.org \ --cc=gary@garyguo.net \ --cc=gregkh@linuxfoundation.org \ --cc=jarkko@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=ojeda@kernel.org \ --cc=rust-for-linux@vger.kernel.org \ --cc=thesven73@gmail.com \ --cc=torvalds@linux-foundation.org \ --cc=wedsonaf@google.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.