Re: [PATCH RFC 1/3] rust: add useful ops for u64

["Alexandre Courbot" <acourbot@nvidia.com>]

On Tue Feb 18, 2025 at 7:07 PM JST, Dirk Behme wrote:
> On 17/02/2025 15:04, Alexandre Courbot wrote:
>> It is common to build a u64 from its high and low parts obtained from
>> two 32-bit registers. Conversely, it is also common to split a u64 into
>> two u32s to write them into registers. Add an extension trait for u64
>> that implement these methods in a new `num` module.
>> 
>> It is expected that this trait will be extended with other useful
>> operations, and similar extension traits implemented for other types.
>> 
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>>  rust/kernel/lib.rs |  1 +
>>  rust/kernel/num.rs | 32 ++++++++++++++++++++++++++++++++
>>  2 files changed, 33 insertions(+)
>> 
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index 496ed32b0911a9fdbce5d26738b9cf7ef910b269..8c0c7c20a16aa96e3d3e444be3e03878650ddf77 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -59,6 +59,7 @@
>>  pub mod miscdevice;
>>  #[cfg(CONFIG_NET)]
>>  pub mod net;
>> +pub mod num;
>>  pub mod of;
>>  pub mod page;
>>  #[cfg(CONFIG_PCI)]
>> diff --git a/rust/kernel/num.rs b/rust/kernel/num.rs
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..5e714cbda4575b8d74f50660580dc4c5683f8c2b
>> --- /dev/null
>> +++ b/rust/kernel/num.rs
>> @@ -0,0 +1,32 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Numerical and binary utilities for primitive types.
>> +
>> +/// Useful operations for `u64`.
>> +pub trait U64Ext {
>> +    /// Build a `u64` by combining its `high` and `low` parts.
>> +    ///
>> +    /// ```
>> +    /// use kernel::num::U64Ext;
>> +    /// assert_eq!(u64::from_u32s(0x01234567, 0x89abcdef), 0x01234567_89abcdef);
>> +    /// ```
>> +    fn from_u32s(high: u32, low: u32) -> Self;
>> +
>> +    /// Returns the `(high, low)` u32s that constitute `self`.
>> +    ///
>> +    /// ```
>> +    /// use kernel::num::U64Ext;
>> +    /// assert_eq!(u64::into_u32s(0x01234567_89abcdef), (0x1234567, 0x89abcdef));
>> +    /// ```
>> +    fn into_u32s(self) -> (u32, u32);
>> +}
>> +
>> +impl U64Ext for u64 {
>> +    fn from_u32s(high: u32, low: u32) -> Self {
>> +        ((high as u64) << u32::BITS) | low as u64
>> +    }
>> +
>> +    fn into_u32s(self) -> (u32, u32) {
>> +        ((self >> u32::BITS) as u32, self as u32)
>> +    }
>> +}
> Just as a question: Would it make sense to make this more generic?
>
> For example
>
> u64 -> u32, u32 / u32, u32 -> u64 (as done here)
> u32 -> u16, u16 / u16, u16 -> u32
> u16 -> u8, u8 / u8, u8 -> u16
>
> Additionally, I wonder if this might be combined with the Integer trait
> [1]? But the usize and signed ones might not make sense here...
>
> Dirk
>
> [1] E.g.
>
> https://github.com/senekor/linux/commit/7291dcc98e8ab74e34c1600784ec9ff3e2fa32d0

I agree something more generic would be nice. One drawback I see though
is that it would have to use more generic (and lengthy) method names -
i.e. `from_components(u32, u32)` instead of `from_u32s`.

I quickly tried to write a completely generic trait where the methods
are auto-implemented from constants and associated types, but got stuck
by the impossibility to use `as` in that context without a macro.

Regardless, I was looking for an already existing trait/module to
leverage instead of introducing a whole new one, maybe the one you
linked is what I was looking for?