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

Alexandre Courbot
2025-09-08T03:26:51Z

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.