Re: [PATCH v2 2/4] nova-core: bitstruct: Add support for different storage widths

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

On Thu Sep 4, 2025 at 6:54 AM JST, Joel Fernandes wrote:
> Previously, bitstructs were hardcoded to use u32 as the underlying
> storage type.  Add support for different storage types (u8, u16, u32,
> u64) to the bitstruct macro.
>
> New syntax is: struct Name: <type ex., u32> { ... }
>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
<snip>
>      // Generates the accessor methods for a single field.
>      (
> -        @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident
> +        @leaf_accessor $name:ident $storage:ty, $hi:tt:$lo:tt $field:ident
>              { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?;
>      ) => {
>          ::kernel::macros::paste!(
>          const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
> -        const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1);
> +        const [<$field:upper _MASK>]: $storage = {
> +            // Generate mask for shifting
> +            match ::core::mem::size_of::<$storage>() {
> +                1 => ::kernel::bits::genmask_u8($lo..=$hi) as $storage,
> +                2 => ::kernel::bits::genmask_u16($lo..=$hi) as $storage,
> +                4 => ::kernel::bits::genmask_u32($lo..=$hi) as $storage,
> +                8 => ::kernel::bits::genmask_u64($lo..=$hi) as $storage,
> +                _ => <$storage>::MAX

Since this is a const expression, you can use `build_error!` to make
compilation fail in the unlikely event the `_` is taken due to bug in
the code. 

> +            }
> +        };
>          const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper _MASK>].trailing_zeros();
>          );
>  
> @@ -211,7 +220,7 @@ impl $name {
>          #[inline(always)]
>          pub(crate) fn $field(self) -> $res_type {
>              ::kernel::macros::paste!(
> -            const MASK: u32 = $name::[<$field:upper _MASK>];
> +            const MASK: $storage = $name::[<$field:upper _MASK>];
>              const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
>              );
>              let field = ((self.0 & MASK) >> SHIFT);
> @@ -226,9 +235,9 @@ pub(crate) fn $field(self) -> $res_type {
>          )?
>          #[inline(always)]
>          pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
> -            const MASK: u32 = $name::[<$field:upper _MASK>];
> +            const MASK: $storage = $name::[<$field:upper _MASK>];
>              const SHIFT: u32 = $name::[<$field:upper _SHIFT>];
> -            let value = (u32::from(value) << SHIFT) & MASK;
> +            let value = (<$storage>::from(value) << SHIFT) & MASK;
>              self.0 = (self.0 & !MASK) | value;
>  
>              self
> @@ -237,7 +246,7 @@ pub(crate) fn [<set_ $field>](mut self, value: $to_type) -> Self {
>      };
>  
>      // Generates the `Debug` implementation for `$name`.
> -    (@debug $name:ident { $($field:ident;)* }) => {
> +    (@debug $name:ident $storage:ty { $($field:ident;)* }) => {

This rule doesn't make use of the `$storage` argument.

>          impl ::core::fmt::Debug for $name {
>              fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
>                  f.debug_struct(stringify!($name))
> @@ -251,7 +260,7 @@ fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
>      };
>  
>      // Generates the `Default` implementation for `$name`.
> -    (@default $name:ident { $($field:ident;)* }) => {
> +    (@default $name:ident $storage:ty { $($field:ident;)* }) => {

Neither does this one.