Add an equivalent to the `fls` (Find Last Set bit) C function to Rust
unsigned types.
It is to be first used by the nova-core driver.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/num.rs | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/rust/kernel/num.rs b/rust/kernel/num.rs
index ee0f67ad1a89e69f5f8d2077eba5541b472e7d8a..934afe17719f789c569dbd54534adc2e26fe59f2 100644
--- a/rust/kernel/num.rs
+++ b/rust/kernel/num.rs
@@ -171,3 +171,34 @@ fn borrow(&self) -> &T {
&self.0
}
}
+
+macro_rules! impl_fls {
+ ($($t:ty),+) => {
+ $(
+ ::kernel::macros::paste! {
+ /// Find Last Set Bit: return the 1-based index of the last (i.e. most significant) set
+ /// bit in `v`.
+ ///
+ /// Equivalent to the C `fls` function.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::num::fls_u32;
+ ///
+ /// assert_eq!(fls_u32(0x0), 0);
+ /// assert_eq!(fls_u32(0x1), 1);
+ /// assert_eq!(fls_u32(0x10), 5);
+ /// assert_eq!(fls_u32(0xffff), 16);
+ /// assert_eq!(fls_u32(0x8000_0000), 32);
+ /// ```
+ #[inline(always)]
+ pub const fn [<fls_ $t>](v: $t) -> u32 {
+ $t::BITS - v.leading_zeros()
+ }
+ }
+ )+
+ };
+}
+
+impl_fls!(usize, u8, u16, u32, u64, u128);
--
2.49.0On Thu, Jun 12, 2025 at 4:02 PM Alexandre Courbot <acourbot@nvidia.com> wrote: > > + /// ``` > + /// use kernel::num::fls_u32; > + /// > + /// assert_eq!(fls_u32(0x0), 0); > + /// assert_eq!(fls_u32(0x1), 1); > + /// assert_eq!(fls_u32(0x10), 5); > + /// assert_eq!(fls_u32(0xffff), 16); > + /// assert_eq!(fls_u32(0x8000_0000), 32); > + /// ``` For a future patch series: this could provide examples per type (passing them in the `impl_fls!` call). I can create a good first issue if this lands and it is not somewhere already. Cheers, Miguel
On Sun Jun 15, 2025 at 6:37 PM JST, Miguel Ojeda wrote: > On Thu, Jun 12, 2025 at 4:02 PM Alexandre Courbot <acourbot@nvidia.com> wrote: >> >> + /// ``` >> + /// use kernel::num::fls_u32; >> + /// >> + /// assert_eq!(fls_u32(0x0), 0); >> + /// assert_eq!(fls_u32(0x1), 1); >> + /// assert_eq!(fls_u32(0x10), 5); >> + /// assert_eq!(fls_u32(0xffff), 16); >> + /// assert_eq!(fls_u32(0x8000_0000), 32); >> + /// ``` > > For a future patch series: this could provide examples per type > (passing them in the `impl_fls!` call). > > I can create a good first issue if this lands and it is not somewhere already. I was worried that the examples would be mostly duplicated, although it is true that seeing how the function behaves at the limits of each type is valuable. I'll prepare a patch to either squash for v6 or submit as a follow-up.
On Sun Jun 15, 2025 at 7:51 PM JST, Alexandre Courbot wrote: > On Sun Jun 15, 2025 at 6:37 PM JST, Miguel Ojeda wrote: >> On Thu, Jun 12, 2025 at 4:02 PM Alexandre Courbot <acourbot@nvidia.com> wrote: >>> >>> + /// ``` >>> + /// use kernel::num::fls_u32; >>> + /// >>> + /// assert_eq!(fls_u32(0x0), 0); >>> + /// assert_eq!(fls_u32(0x1), 1); >>> + /// assert_eq!(fls_u32(0x10), 5); >>> + /// assert_eq!(fls_u32(0xffff), 16); >>> + /// assert_eq!(fls_u32(0x8000_0000), 32); >>> + /// ``` >> >> For a future patch series: this could provide examples per type >> (passing them in the `impl_fls!` call). >> >> I can create a good first issue if this lands and it is not somewhere already. > > I was worried that the examples would be mostly duplicated, although > it is true that seeing how the function behaves at the limits of each > type is valuable. I'll prepare a patch to either squash for v6 or submit > as a follow-up. Also, although this will work nicely for `impl_fls!` which is a single function, I'm afraid this won't scale well for `power_of_two_impl!`, which defines 6 functions per type... Any suggestions for this case?
On Sun, Jun 15, 2025 at 12:58 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> Also, although this will work nicely for `impl_fls!` which is a single
> function, I'm afraid this won't scale well for `power_of_two_impl!`,
> which defines 6 functions per type... Any suggestions for this case?
We can always generate the same "cases", i.e. sharing as much as
possible the lines, and just passing the values (numbers) that
actually differ, which you then plug into the example line
concatenating.
The standard library does that for their integer macros, e.g.
https://doc.rust-lang.org/src/core/num/int_macros.rs.html#3639-3644
If that happened to be too onerous for some reason, then we could
ignore it for the time being (i.e. we don't need to delay things just
for that), or we could put them as `#[test]`s to at least have them as
tests.
Cheers,
Miguel
On Sun Jun 15, 2025 at 10:25 PM JST, Miguel Ojeda wrote: > On Sun, Jun 15, 2025 at 12:58 PM Alexandre Courbot <acourbot@nvidia.com> wrote: >> >> Also, although this will work nicely for `impl_fls!` which is a single >> function, I'm afraid this won't scale well for `power_of_two_impl!`, >> which defines 6 functions per type... Any suggestions for this case? > > We can always generate the same "cases", i.e. sharing as much as > possible the lines, and just passing the values (numbers) that > actually differ, which you then plug into the example line > concatenating. > > The standard library does that for their integer macros, e.g. > > https://doc.rust-lang.org/src/core/num/int_macros.rs.html#3639-3644 > > If that happened to be too onerous for some reason, then we could > ignore it for the time being (i.e. we don't need to delay things just > for that), or we could put them as `#[test]`s to at least have them as > tests. Thanks, this appears to work quite nicely (if a bit verbose), and I can adjust the tests to avoid the need to take extra arguments.
On Thu Jun 12, 2025 at 4:01 PM CEST, Alexandre Courbot wrote:
> Add an equivalent to the `fls` (Find Last Set bit) C function to Rust
> unsigned types.
Have you tried to upstream this?
> It is to be first used by the nova-core driver.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> rust/kernel/num.rs | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/rust/kernel/num.rs b/rust/kernel/num.rs
> index ee0f67ad1a89e69f5f8d2077eba5541b472e7d8a..934afe17719f789c569dbd54534adc2e26fe59f2 100644
> --- a/rust/kernel/num.rs
> +++ b/rust/kernel/num.rs
> @@ -171,3 +171,34 @@ fn borrow(&self) -> &T {
> &self.0
> }
> }
> +
> +macro_rules! impl_fls {
> + ($($t:ty),+) => {
> + $(
> + ::kernel::macros::paste! {
> + /// Find Last Set Bit: return the 1-based index of the last (i.e. most significant) set
> + /// bit in `v`.
> + ///
> + /// Equivalent to the C `fls` function.
> + ///
> + /// # Examples
> + ///
> + /// ```
> + /// use kernel::num::fls_u32;
> + ///
> + /// assert_eq!(fls_u32(0x0), 0);
> + /// assert_eq!(fls_u32(0x1), 1);
> + /// assert_eq!(fls_u32(0x10), 5);
> + /// assert_eq!(fls_u32(0xffff), 16);
> + /// assert_eq!(fls_u32(0x8000_0000), 32);
> + /// ```
> + #[inline(always)]
> + pub const fn [<fls_ $t>](v: $t) -> u32 {
Can we name this `find_last_set_bit_ $t`? When the upstream function
lands, we should also rename this one.
---
Cheers,
Benno
> + $t::BITS - v.leading_zeros()
> + }
> + }
> + )+
> + };
> +}
> +
> +impl_fls!(usize, u8, u16, u32, u64, u128);
On Sun Jun 15, 2025 at 4:16 AM JST, Benno Lossin wrote:
> On Thu Jun 12, 2025 at 4:01 PM CEST, Alexandre Courbot wrote:
>> Add an equivalent to the `fls` (Find Last Set bit) C function to Rust
>> unsigned types.
>
> Have you tried to upstream this?
I will consider alongside `prev_multiple_of` that we discussed during v4. :)
>
>> It is to be first used by the nova-core driver.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> rust/kernel/num.rs | 31 +++++++++++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/rust/kernel/num.rs b/rust/kernel/num.rs
>> index ee0f67ad1a89e69f5f8d2077eba5541b472e7d8a..934afe17719f789c569dbd54534adc2e26fe59f2 100644
>> --- a/rust/kernel/num.rs
>> +++ b/rust/kernel/num.rs
>> @@ -171,3 +171,34 @@ fn borrow(&self) -> &T {
>> &self.0
>> }
>> }
>> +
>> +macro_rules! impl_fls {
>> + ($($t:ty),+) => {
>> + $(
>> + ::kernel::macros::paste! {
>> + /// Find Last Set Bit: return the 1-based index of the last (i.e. most significant) set
>> + /// bit in `v`.
>> + ///
>> + /// Equivalent to the C `fls` function.
>> + ///
>> + /// # Examples
>> + ///
>> + /// ```
>> + /// use kernel::num::fls_u32;
>> + ///
>> + /// assert_eq!(fls_u32(0x0), 0);
>> + /// assert_eq!(fls_u32(0x1), 1);
>> + /// assert_eq!(fls_u32(0x10), 5);
>> + /// assert_eq!(fls_u32(0xffff), 16);
>> + /// assert_eq!(fls_u32(0x8000_0000), 32);
>> + /// ```
>> + #[inline(always)]
>> + pub const fn [<fls_ $t>](v: $t) -> u32 {
>
> Can we name this `find_last_set_bit_ $t`? When the upstream function
> lands, we should also rename this one.
We can - but as for `align_up`/`next_multiple_of`, I am not sure which
naming scheme (kernel-like or closer to Rust conventions) is favored in
such cases, and so far it seems to come down to personal preference. I
tend to think that staying close to kernel conventions make it easier to
understand when a function is the equivalent of a C one, but whichever
policy we adopt it would be nice to codify it somewhere (apologies if it
is already and I missed it).
On Mon Jun 16, 2025 at 8:41 AM CEST, Alexandre Courbot wrote:
> On Sun Jun 15, 2025 at 4:16 AM JST, Benno Lossin wrote:
>> On Thu Jun 12, 2025 at 4:01 PM CEST, Alexandre Courbot wrote:
>>> + #[inline(always)]
>>> + pub const fn [<fls_ $t>](v: $t) -> u32 {
>>
>> Can we name this `find_last_set_bit_ $t`? When the upstream function
>> lands, we should also rename this one.
>
> We can - but as for `align_up`/`next_multiple_of`, I am not sure which
> naming scheme (kernel-like or closer to Rust conventions) is favored in
> such cases, and so far it seems to come down to personal preference. I
> tend to think that staying close to kernel conventions make it easier to
> understand when a function is the equivalent of a C one, but whichever
> policy we adopt it would be nice to codify it somewhere (apologies if it
> is already and I missed it).
I don't think we have it written down anywhere. I don't think that we
should have a global rule for this. Certain things are more in the
purview of the kernel and others are more on the Rust side.
My opinion is that this, since it will hopefully be in `core` at some
point, should go with the Rust naming.
---
Cheers,
Benno
On Thu Jun 19, 2025 at 4:24 AM JST, Benno Lossin wrote:
> On Mon Jun 16, 2025 at 8:41 AM CEST, Alexandre Courbot wrote:
>> On Sun Jun 15, 2025 at 4:16 AM JST, Benno Lossin wrote:
>>> On Thu Jun 12, 2025 at 4:01 PM CEST, Alexandre Courbot wrote:
>>>> + #[inline(always)]
>>>> + pub const fn [<fls_ $t>](v: $t) -> u32 {
>>>
>>> Can we name this `find_last_set_bit_ $t`? When the upstream function
>>> lands, we should also rename this one.
>>
>> We can - but as for `align_up`/`next_multiple_of`, I am not sure which
>> naming scheme (kernel-like or closer to Rust conventions) is favored in
>> such cases, and so far it seems to come down to personal preference. I
>> tend to think that staying close to kernel conventions make it easier to
>> understand when a function is the equivalent of a C one, but whichever
>> policy we adopt it would be nice to codify it somewhere (apologies if it
>> is already and I missed it).
>
> I don't think we have it written down anywhere. I don't think that we
> should have a global rule for this. Certain things are more in the
> purview of the kernel and others are more on the Rust side.
>
> My opinion is that this, since it will hopefully be in `core` at some
> point, should go with the Rust naming.
I guess in that case we should go with `last_set_bit`, as `find_` is not
really used as a prefix for this kind of operations (e.g.
`leading_zeros` and friends).
On Thu Jun 19, 2025 at 3:26 PM CEST, Alexandre Courbot wrote:
> On Thu Jun 19, 2025 at 4:24 AM JST, Benno Lossin wrote:
>> On Mon Jun 16, 2025 at 8:41 AM CEST, Alexandre Courbot wrote:
>>> On Sun Jun 15, 2025 at 4:16 AM JST, Benno Lossin wrote:
>>>> On Thu Jun 12, 2025 at 4:01 PM CEST, Alexandre Courbot wrote:
>>>>> + #[inline(always)]
>>>>> + pub const fn [<fls_ $t>](v: $t) -> u32 {
>>>>
>>>> Can we name this `find_last_set_bit_ $t`? When the upstream function
>>>> lands, we should also rename this one.
>>>
>>> We can - but as for `align_up`/`next_multiple_of`, I am not sure which
>>> naming scheme (kernel-like or closer to Rust conventions) is favored in
>>> such cases, and so far it seems to come down to personal preference. I
>>> tend to think that staying close to kernel conventions make it easier to
>>> understand when a function is the equivalent of a C one, but whichever
>>> policy we adopt it would be nice to codify it somewhere (apologies if it
>>> is already and I missed it).
>>
>> I don't think we have it written down anywhere. I don't think that we
>> should have a global rule for this. Certain things are more in the
>> purview of the kernel and others are more on the Rust side.
>>
>> My opinion is that this, since it will hopefully be in `core` at some
>> point, should go with the Rust naming.
>
> I guess in that case we should go with `last_set_bit`, as `find_` is not
> really used as a prefix for this kind of operations (e.g.
> `leading_zeros` and friends).
Sounds good!
---
Cheers,
Benno
© 2016 - 2025 Red Hat, Inc.