NVIDIA GPU driver infrastructure
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Eliot Courtney" <ecourtney@nvidia.com>
Cc: "Alice Ryhl" <aliceryhl@google.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Burak Emir" <burak.emir@gmail.com>,
	"Yury Norov" <yury.norov@gmail.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Onur Özkan" <work@onurozkan.dev>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Alistair Popple" <apopple@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>, "Zhi Wang" <zhiw@nvidia.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	nova-gpu@lists.linux.dev, dri-devel@lists.freedesktop.org,
	dri-devel <dri-devel-bounces@lists.freedesktop.org>
Subject: Re: [PATCH v8 03/12] rust: num: add cv! macro to create values from constant expressions
Date: Fri, 28 Aug 2026 12:40:13 +0900	[thread overview]
Message-ID: <DL09JQ5CRJLA.4Q6OTGU4CMWN@nvidia.com> (raw)
In-Reply-To: <DL051FB1HOBY.2UC911T3M9AIS@nvidia.com>

On Fri Aug 28, 2026 at 9:08 AM JST, Eliot Courtney wrote:
> On Thu Aug 27, 2026 at 11:55 PM JST, Alice Ryhl wrote:
>> On Thu, Aug 27, 2026 at 4:37 PM Gary Guo <gary@garyguo.net> wrote:
>>>
>>> On Thu Aug 27, 2026 at 3:29 PM BST, Alexandre Courbot wrote:
>>> > On Thu Aug 27, 2026 at 10:48 PM JST, Eliot Courtney wrote:
>>> >> On Thu Aug 27, 2026 at 8:12 PM JST, Alexandre Courbot wrote:
>>> >>> On Thu Aug 27, 2026 at 7:42 PM JST, Alexandre Courbot wrote:
>>> >>>> On Thu Aug 27, 2026 at 6:32 PM JST, Alice Ryhl wrote:
>>> >>>>> On Thu, Aug 27, 2026 at 04:28:31PM +0900, Eliot Courtney wrote:
>>> >>>>>> Currently, using NonZero/Bounded constants is quite verbose. It's
>>> >>>>>> unfortunate because it disincentivizes using it in interface boundaries.
>>> >>>>>> Introduce a macro to make it nicer to use. The macro `cv!` (for constant
>>> >>>>>> value) takes a const integer expression and widens it to i128 (at build
>>> >>>>>> time only) before passing it as a const generic value to a new trait
>>> >>>>>> function `FromConst::from_const`. The trait is implemented by NonZero,
>>> >>>>>> Bounded, and Alignment and lets values of each be constructed from
>>> >>>>>> constants without a verbose turbofish syntax. For example,
>>> >>>>>> `const { NonZero::new(1).unwrap() }` can be written as `cv!(1)`.
>>> >>>>>>
>>> >>>>>> Suggested-by: Gary Guo <gary@garyguo.net>
>>> >>>>>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>>> >>>>>
>>> >>>>> This doesn't work in const context, so I don't think this is a great
>>> >>>>> strategy.
>>> >>>>>
>>> >>>>> I would want to use it for cases like this:
>>> >>>>>
>>> >>>>> drivers/android/binder/netlink.rs
>>> >>>>>         const BINDER_CMD_REPORT: u8 = kernel::uapi::BINDER_CMD_REPORT as u8;
>>> >>>>>         const BINDER_A_REPORT_ERROR: c_int = kernel::uapi::BINDER_A_REPORT_ERROR as c_int;
>>> >>>>>         const BINDER_A_REPORT_CONTEXT: c_int = kernel::uapi::BINDER_A_REPORT_CONTEXT as c_int;
>>> >>>>>         const BINDER_A_REPORT_FROM_PID: c_int = kernel::uapi::BINDER_A_REPORT_FROM_PID as c_int;
>>> >>>>>         const BINDER_A_REPORT_FROM_TID: c_int = kernel::uapi::BINDER_A_REPORT_FROM_TID as c_int;
>>> >>>>>         const BINDER_A_REPORT_TO_PID: c_int = kernel::uapi::BINDER_A_REPORT_TO_PID as c_int;
>>> >>>>>         const BINDER_A_REPORT_TO_TID: c_int = kernel::uapi::BINDER_A_REPORT_TO_TID as c_int;
>>> >>>>>         const BINDER_A_REPORT_IS_REPLY: c_int = kernel::uapi::BINDER_A_REPORT_IS_REPLY as c_int;
>>> >>>>>         const BINDER_A_REPORT_FLAGS: c_int = kernel::uapi::BINDER_A_REPORT_FLAGS as c_int;
>>> >>>>>         const BINDER_A_REPORT_CODE: c_int = kernel::uapi::BINDER_A_REPORT_CODE as c_int;
>>> >>>>>         const BINDER_A_REPORT_DATA_SIZE: c_int = kernel::uapi::BINDER_A_REPORT_DATA_SIZE as c_int;
>>> >>>>
>>> >>>> `const_as!` [1] should do the trick for this, provided you don't need to
>>> >>>> create a const `NonZero`.
>>> >>>>
>>> >>>> [1] https://lore.kernel.org/all/20260825-const_as-v1-1-1ce712225fe2@nvidia.com/
>>> >>>
>>> >>> ... but I agree it would be nice to be able to use this in const
>>> >>> context. And there is an overlap with `const_as!` that becomes more
>>> >>> obvious the more I look at it.
>>> >>>
>>> >>> In for a penny, in for a pound of macro code as they say. Since we
>>> >>> agreed on using macros, how about unifying both under the same `cv!`
>>> >>> macro, with as many branches as we have types we want to initialize from
>>> >>> a constant value? For instance:
>>> >>>
>>> >>>     // Does what `const_as!` currently does under the hood.
>>> >>>     const BINDER_CMD_REPORT: u8 = cv!(u8::from(kernel::uapi::BINDER_CMD_REPORT));
>>> >>>     // Calls `NonZero::new().unwrap()` under the hood.
>>> >>>     const SOME_NONZERO: NonZero<u8> = cv!(NonZero::new(kernel::uapi::NONZERO_VALUE));
>>> >>>     // Calls `Bounded::new::<{ ...}>()` under the hood.
>>> >>>     const SOME_BOUNDED: Bounded<u32, 2> = cv!(Bounded::new(kernel::uapi::SMALL_VALUE));
>>> >>>
>>> >>> I.e. we would have one extra matching arm in `cv!` per type it handles
>>> >>> instead of implementing a trait. The syntax of the macro would look more
>>> >>> natural (bye bye `const_as`'s awkward `=>`), albeit it would have the
>>> >>> limitations of such a semantic dispatch.
>>> >>>
>>> >>> Even the name `const_as!` wasn't really accurate to begin with: what it
>>> >>> really emulates is a const `try_from`, and we even discussed
>>> >>> implementing it in these terms in the future.
>>> >>>
>>> >>> I'm sure the idea needs more polishing but I think there's something to
>>> >>> explore here.
>>> >>
>>> >> Yeah I agree that const_as! is similar and if we had const traits we
>>> >> could fully merge them and have it always work in a const context for
>>> >> both duties (which are really a const tryfrom as you said).
>>> >>
>>> >> I am not sure about the suggested syntax (e.g.
>>> >> cv!(Bounded::new(kernel::uapi::SMALL_VALUE))), since it seems very
>>> >> verbose.
>>> >
>>> > A bit, but what I like is that it looks very close to what you would
>>> > naturally write if you had const traits (minus the unwraps), so you
>>> > don't have to learn a new syntax. As long as it's not *more* verbose
>>> > than natural Rust, I think it's fine.
>>> >
>>> > It also has the benefit of relying less on type inference, i.e. `cv!(5)`
>>> > requires the caller to specify the type even with a `let` statement,
>>> > whereas you could do `let v = cv!(NonZero::new(5));` and it would work
>>> > as expected.
>>>
>>> Even with const try_from we'd still want `cv!()` to avoid having to write
>>>
>>>     const { Type::try_from(...).unwrap() }
>>>
>>> I think having `=>` syntax is great because it is a good place to *optionally*
>>> require type annotation.
>>>
>>> For enum repr for example, I think it'd be great that
>>>
>>>     const BINDER_CMD_REPORT: u8 = cv!(kernel::uapi::BINDER_CMD_REPORT);
>>>
>>> would work directly. It might need some tricks, which I have hard time coming up
>>> as I'm not feeling very well today, but I'll give it a shot over the weekend...
>>
>> One could potentially define a trait with a MIN and MAX value
>> constant, and then implement cv! like this:
>>
>> 1. Verify that the value lies between MIN and MAX.
>> 2. Cast the value to uNN of the same size as the target type.
>> 3. Transmute the uNN to the target type.
>>
>> Since the trait has no methods, this works in const eval.
>>
>> Alice
>
> Using an associated const by itself appears to work - I tried this which
> is very similar to Alice's suggested approach above:
>
> ```
> macro_rules! const_assert {
>     ($condition:expr $(,$arg:literal)?) => {
>         const { ::core::assert!($condition $(,$arg)?) };
>     };
> }

Any reason this cannot use the `const_assert` already in the kernel
crate?

>
> trait FromConst<const V: i128>: Sized {
>     const VALUE: Self;
> }
>
> macro_rules! cv {
>     (@widen $v:expr) => {{
>         #[allow(unused_comparisons, unused_assignments)]
>         {
>             let v = $v;
>             let r = v as i128;
>             let mut back = v;
>             back = r as _;
>
>             ::core::assert!(
>                 back == v && (v < 0) == (r < 0),
>                 "value cannot be losslessly widened to `i128`"
>             );
>
>             r
>         }
>     }};
>     ($v:expr => $t:ty) => {
>         <$t as FromConst<{ cv!(@widen $v) }>>::VALUE

nit for the actual posting: make sure to fully qualify `cv` when calling
it recursively (and make sure all symbols are fully qualified).

>     };
>     ($v:expr) => {
>         <_ as FromConst<{ cv!(@widen $v) }>>::VALUE
>     };
> }
>
> macro_rules! impl_from_const_int {
>     ($($t:ty)*) => {$(
>         impl<const V: i128> FromConst<V> for $t {
>             const VALUE: Self = {
>                 const_assert!(
>                     V >= <$t>::MIN as i128 && V <= <$t>::MAX as i128,
>                     "Constant cannot be represented by the target type."
>                 );
>                 V as $t
>             };
>         }
>
>         impl<const V: i128> FromConst<V> for NonZero<$t> {
>             const VALUE: Self = {
>                 const_assert!(
>                     V >= <$t>::MIN as i128 && V <= <$t>::MAX as i128,
>                     "Constant cannot be represented by the underlying type."
>                 );

Let's also have a `const_assert!(V != 0, ...)` to provide a better
error message than "unwrap on None" if users call this with 0.

>                 NonZero::new(V as $t).unwrap()
>             };
>         }
>     )*};
> }
> impl_from_const_int!(u8 u16 u32 u64 usize i8 i16 i32 i64 isize);
>
> impl<const V: i128> FromConst<V> for Alignment {
>     const VALUE: Self = {
>         const_assert!(V > 0 && V <= usize::MAX as i128);
>         // The unwrap fails the build if `V` is not a power of two.
>         Alignment::new_checked(V as usize).unwrap()
>     };
> }
>
> const A: u8 = cv!(200u32);
> const B: NonZero<u8> = cv!(5);
> const C: Alignment = cv!(4096);
> const D: u8 = cv!(200u32 => u8);
> ```

That looks like it could work! IIUC it even supports something like
`cv!(x => NonZero<u8>)`. The only limitation is see is that this cannot
take expressions using generic parameters, but we can probably work
around that.

I guess you'll want to split this out into its own series so it doesn't
remain hidden within the ranges/bitmap work. Basically as a replacement
for the `const_as` I was driving [1]. I'll recycle the `const_as` series
to just switch to the kernel converters, and will follow-up with using
`cv!` once it lands.

Since this is going to be a multi-cycle effort we should probably keep
the legacy `*_into_*` functions around for now and remove them in
another patch once all users are converted.

[1] https://lore.kernel.org/20260825-const_as-v1-0-1ce712225fe2@nvidia.com

  reply	other threads:[~2026-08-28  3:40 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-27  7:28 [PATCH v8 00/12] rust: Add support for reserving of ranges of IDs Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 01/12] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 02/12] rust: bitmap: restrict bitmap length to at most i32::MAX Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 03/12] rust: num: add cv! macro to create values from constant expressions Eliot Courtney
2026-08-27  9:32   ` Alice Ryhl
2026-08-27 10:42     ` Alexandre Courbot
2026-08-27 11:12       ` Alexandre Courbot
2026-08-27 13:48         ` Eliot Courtney
2026-08-27 13:59           ` Gary Guo
2026-08-27 14:29           ` Alexandre Courbot
2026-08-27 14:37             ` Gary Guo
2026-08-27 14:55               ` Alice Ryhl
2026-08-28  0:08                 ` Eliot Courtney
2026-08-28  3:40                   ` Alexandre Courbot [this message]
2026-08-27  7:28 ` [PATCH v8 04/12] rust: prelude: add `num::cv` Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 05/12] rust: use cv! to build Bounded values from constants Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 06/12] rust: sizes: add sub-1K size constants Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 07/12] rust: sizes: implement SizeConstants for Alignment Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 08/12] rust: use Alignment size constants Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 09/12] rust: bitmap: add contiguous area operations Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 10/12] rust: id_pool: add contiguous ID reservation Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 11/12] rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN Eliot Courtney
2026-08-27  7:28 ` [PATCH v8 12/12] gpu: nova-core: add ChannelIdPool Eliot Courtney

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=DL09JQ5CRJLA.4Q6OTGU4CMWN@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=burak.emir@gmail.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel-bounces@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ecourtney@nvidia.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jhubbard@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nova-gpu@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    --cc=work@onurozkan.dev \
    --cc=yury.norov@gmail.com \
    --cc=zhiw@nvidia.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox