riscv: add CSR-defining macros #219

rmsyn · 2024-05-30T03:59:19Z

Adds helper macros for defining CSR types:

read-write:
- have both getter and setter methods
- able to read and write the CSR
read-only:
- only has getter methods
- able to read the CSR
write-only:
- only has setter methods
- able to write the CSR

Related to: #218

rmsyn · 2024-05-30T04:09:24Z

riscv/src/register/tests.rs

+read_write_csr! {
+    "test CSR register type",
+    Mtest: 0x000,
+    0b1111_1111,
+    "test single-bit field",
+    single: 0,
+    "setter test single-bit field",
+    set_single: 0,
+}
+
+read_write_csr_field! {
+    Mtest,
+    "multiple single-bit field range",
+    multi_range: 1..=3,
+    "setter multiple single-bit field range",
+    set_multi_range: 1..=3,
+}
+
+read_write_csr_field!(
+    Mtest,
+    "multi-bit field",
+    multi_field: [4:7],
+    "setter multi-bit field",
+    set_multi_field: [4:7],
+);


Test calls to the new macros. The above is just a basic compile test.

Still need to add unit tests, and macro calls for the read-only and write-only variants.

romancardenas · 2024-05-30T09:40:50Z

Thanks! I'll review it ASAP

romancardenas

Kind of like it! the mask idea is very nice. I miss two main things:

Less repetition in macros. Maybe we could atomize them and leave one "core" macro that creates the struct with bitmask and from_bits/bits, another for the get/set...
Using custom types (mainly enums) for setters and getters. That would be awesome (for example XLEN

riscv/src/register/macros.rs

rmsyn · 2024-06-04T20:57:39Z

riscv/src/register/macros.rs

+macro_rules! read_write_csr {
+    ($doc:expr,
+     $ty:ident: $csr:tt,
+     $mask:tt,
+     $(
+     $field_doc:expr,
+     $field:ident: $get_bit:literal,
+     $set_field_doc:expr,
+     $set_field:ident: $set_bit:literal$(,)?
+     )+) => {
+        $crate::read_write_csr!($doc, $ty: $csr, $mask);
+
+        $(
+            $crate::read_write_csr_field!(
+                $ty,
+                $field_doc,
+                $field: $get_bit,
+                $set_field_doc,
+                $set_field: $set_bit,
+            );
+        )+
+    };


Less repetition in macros. Maybe we could atomize them and leave one "core" macro that creates the struct with bitmask and from_bits/bits, another for the get/set...

That's essentially what is there, each of the branches for the various ways to call [read,write]_[only,write]_csr call the internal branch for creating the struct.

Then, the repeated part (in the $(...)+) creates the getters/setters. I guess we could also make read_write_csr_field into another branch of the macro...

Other than that, I'm not sure exactly what you're asking for. What changes would you like to see?

What I mean is that, for example, this part of the macro where the register struct is defined appears three times. It would be nice to have it somehow just once in a separate, basic macro and, depending on the macro branches, call this macro

Ah, I see what you mean. Because each of the macros has this helper branch, make it into its own macro that each one could call.

Alright, I'll work on separating it out. Maybe with a note that you can, but probably don't want to, call this helper macro on its own.

Edit: I added the changes as a fixup commit for easier review. If you like the approach, I'll squash the commit into the previous one.

rmsyn · 2024-06-04T21:02:35Z

Using custom types (mainly enums) for setters and getters.

We could add another branch that takes an $var_ty: ident, [$($variant:ident, $variant_val:literal),*]$(,). That would allow to define a custom enum type, and use that enum in the getters/setters.

I can try to work something out.

rmsyn · 2024-06-06T03:50:49Z

I can try to work something out.

I added the enum branches, and an extra helper macro.

Let me know what you think.

romancardenas · 2024-06-07T17:17:51Z

@rmsyn I'm on it, but macros are difficult to review... 😥

Looks great. The only thing now is that I prefer to leave some macros out of the equation, and then let us call the proper sequence of macros according to the CSR. I think this will help us in the future to deal with macros (modular vs monolithic).

Regarding from and into: if we can get help from From and TryFrom traits, I'd go for them instead of maintaining repetitive methods.

Great job!! I'll keep studying all your code 😁

riscv/src/register/macros.rs

rmsyn · 2024-06-08T02:15:25Z

riscv/src/register/macros.rs

+     $field:ident: $get_bit:literal,
+     $set_field_doc:expr,
+     $set_field:ident: $set_bit:literal$(,)?


Another thing I would like your opinion on @romancardenas, do you think there are use-cases where users would want to define different bits for the get and set functions?

I couldn't really think of a place where this would be a good idea, so maybe the bit argument could be a single parameter?

Same idea for the range and multi-bit variants.

I think these macros should work in the general use cases (i.e., the same bits for getters and setters). For particular cases, we can always just implement them by hand.

rmsyn · 2024-09-14T05:36:17Z

Updated the changes onto the latest HEAD commit.

Squashed the previous fixup commit to simplify review.

Added the necessary changes to incorporate the addition of fallible functions, and changed the doc-string arguments.

Let me know if you like the changes.

I still think the bit-field and bit-range macro arguments could be simpler, especially now with four versions of essentially the same macro argument (e.g. $field, $try_field, $set_field, $try_set_field).

Currently, it feels like having the user supply the same argument past the colon all four times is a potential footgun.

Instead, we could change the $field: $field_bit to $field for all four variants, and add a single $bit_arg: $bit at tthe end. Thoughts?

rmsyn · 2024-09-15T02:47:35Z

@romancardenas I added some more fixes + improvements to the macro helpers. Reduced some redundancy, and hopefully made the macro invocations more readable.

Please let me know what you think.

More or less, this PR is probably pretty close to "finished"(tm).

Let me know if there are any more changes you would like to see.

romancardenas · 2024-09-16T09:36:19Z

I'll try to review this later today. Thanks!

romancardenas · 2024-09-17T20:56:14Z

Looks good, but it is true that having to do something like:

    field: {
        /// Gets the `cycle[h]` inhibit field value.
        cy,
        /// Attempts to get the `cycle[h]` inhibit field value.
        try_cy,
        /// Sets the `cycle[h]` inhibit field value.
        ///
        /// **NOTE**: only updates the in-memory value without touching the CSR.
        set_cy,
        /// Attempts to set the `cycle[h]` inhibit field value.
        ///
        /// **NOTE**: only updates the in-memory value without touching the CSR.
        try_set_cy,
        bit: 0,
    },

is perhaps too verbose. Can't we do something like:

field: {
        /// `cycle[h]` inhibit field value
        cy,
        bit: 0,
    },

And then generate the same code? Just following the try and set notation.

Also, I feel like there are a lot of branches with similar code. Can't we simplify the macros using recursive calls that match patterns, consuming part of the tokens? I think in this way we would have simpler pattern matching and less duplicates.

rmsyn · 2024-09-18T01:21:54Z

Looks good, but it is true that having to do something like:

    field: {
        /// Gets the `cycle[h]` inhibit field value.
        cy,
        /// Attempts to get the `cycle[h]` inhibit field value.
        try_cy,
        /// Sets the `cycle[h]` inhibit field value.
        ///
        /// **NOTE**: only updates the in-memory value without touching the CSR.
        set_cy,
        /// Attempts to set the `cycle[h]` inhibit field value.
        ///
        /// **NOTE**: only updates the in-memory value without touching the CSR.
        try_set_cy,
        bit: 0,
    },

is perhaps too verbose. Can't we do something like:

field: {
        /// `cycle[h]` inhibit field value
        cy,
        bit: 0,
    },

And then generate the same code? Just following the try and set notation.

Yes, this is what introducing the paste dependency solves. It requires proc_macros at the very least, and rewriting at least part of the paste crate locally, if we don't import it.

Personally, using the crate dependency is the better option, as it is actively maintained + high code quality.

I really do think the use-case justifies the additional dependency.

I'll add a commit using the paste crate.

Also, I feel like there are a lot of branches with similar code. Can't we simplify the macros using recursive calls that match patterns, consuming part of the tokens? I think in this way we would have simpler pattern matching and less duplicates.

Could you make comments on the specific places where you think they could be refactored into recursive branches?

I have a couple of those branches in the read_only_csr_field and write_only_csr_field to allow for re-use in read_write_csr. Where else can be simplified in a similar way?

rmsyn · 2024-09-18T02:17:53Z

riscv/src/register/macros.rs

+
+    ($ty:ident,
+     $(#[$field_doc:meta])+
+     $field:ident: [$bit_start:literal : $bit_end:literal]$(,)?) => {
+        impl $ty {
+            paste::paste! {
+                $(#[$field_doc])+
+                #[inline]
+                pub fn $field(&self) -> usize {
+                    self.[<try_ $field>]().unwrap()
+                }
+
+                $(#[$field_doc])+
+                #[inline]
+                pub fn [<try_ $field>](&self) -> $crate::result::Result<usize> {
+                    let max_width = core::mem::size_of_val(&self.bits) * 8;
+
+                    if $bit_end < max_width && $bit_start <= $bit_end {
+                        Ok($crate::bits::bf_extract(self.bits, $bit_start, $bit_end - $bit_start + 1))
+                    } else {
+                        Err($crate::result::Error::IndexOutOfBounds {
+                            index: $bit_start,
+                            min: $bit_start,
+                            max: $bit_end,
+                        })
+                    }
+                }
+            }
+        }
+    };
+
+    ($ty:ident,
+     $(#[$field_doc:meta])+
+     $field:ident: {
+         $(#[$field_ty_doc:meta])+
+         $field_ty:ident {
+             range: [$field_start:literal : $field_end:literal],
+             default: $default_variant:ident,
+             $($variant:ident = $value:expr$(,)?)+
+         }$(,)?
+     }$(,)?
+    ) => {
+        $crate::csr_field_enum!(
+            $(#[$field_ty_doc])+
+            $field_ty {
+                range: [$field_start : $field_end],
+                default: $default_variant,
+                $($variant = $value,)+
+            },
+        );
+
+        $crate::read_only_csr_field!(
+            $ty,
+            $(#[$field_doc])*
+            $field: [$field_start : $field_end],
+            $field_ty,
+        );
+    };
+
+    ($ty:ident,
+     $(#[$field_doc:meta])+
+     $field:ident: [$field_start:literal : $field_end:literal],
+     $field_ty:ident$(,)?
+    ) => {
+         impl $ty {
+             paste::paste! {
+                 $(#[$field_doc])+
+                 #[inline]
+                 pub fn $field(&self) -> $field_ty {
+                     self.[<try_ $field>]().unwrap()
+                 }
+
+                 $(#[$field_doc])+
+                 #[inline]
+                 pub fn [<try_ $field>](&self) -> $crate::result::Result<$field_ty> {
+                    let max_width = core::mem::size_of_val(&self.bits) * 8;
+
+                     if $field_end < max_width && $field_start < $field_end {
+                         let value = $crate::bits::bf_extract(
+                             self.bits,
+                             $field_start,
+                             $field_end - $field_start + 1,
+                         );
+
+                         $field_ty::from_usize(value).ok_or($crate::result::Error::InvalidFieldVariant {
+                             field: stringify!($field),
+                             value,
+                         })
+                     } else {
+                        Err($crate::result::Error::IndexOutOfBounds {
+                            index: $field_start,
+                            min: $field_start,
+                            max: $field_end,
+                        })
+                     }
+                 }
+             }
+         }
+    };
+}


I rewrote the read_only_csr_field using the paste macro.

Please let me know if you are alright adding the dependency, and I'll rewrite the rest of the macros.

Please be sure about this decision, as it is a lot of work rewriting these macros.

Then let's cool this down until we resolve other design decisions. I can say that, as of now, I like how it looks

rmsyn · 2024-09-18T02:19:48Z

riscv/src/register/tests/read_only_csr.rs

+read_only_csr! {
+    /// test CSR register type
+    Mtest: 0x000,
+    mask: 0b1111_1111_1111,
+    /// test single-bit field
+    single: 0,
+}
+
+read_only_csr_field! {
+    Mtest,
+    /// multiple single-bit field range
+    multi_range: 1..=3,
+}
+
+read_only_csr_field!(
+    Mtest,
+    /// multi-bit field
+    multi_field: [4:7],
+);
+
+read_only_csr_field!(
+    Mtest,
+    /// multi-bit field
+    field_enum: {
+        /// field enum type with valid field variants
+        MtestFieldEnum {
+            range: [7:11],
+            default: Field1,
+            Field1 = 1,
+            Field2 = 2,
+            Field3 = 3,
+            Field4 = 15,
+        }
+    }
+);


You can see how much the paste crate simplifies macro invocation, it is substantial.

The savings are even greater for the read_write_csr* variants.

romancardenas · 2024-09-18T06:30:59Z

About reducing code size. Let's take only two pattern branches in read_only_csr_field:

#[macro_export]
macro_rules! read_only_csr {
    ($(#[$doc:meta])+
     $ty:ident: $csr:tt,
     mask: $mask:tt,
     $(
     $(#[$field_doc:meta])+
     $field:ident: $bit:literal$(,)?
     )+
    ) => {
        $crate::csr! { $(#[$doc])+ $ty: $csr, $mask }

        $(
            $crate::read_only_csr_field! {
                $ty,
                $(#[$field_doc])+
                $field: $bit,
            }
        )+

        $crate::read_csr_as!($ty, $csr);
    };

    ($(#[doc:meta])+
     $ty:ident: $csr:tt,
     mask: $mask:tt,
     $(
     $(#[$field_doc:meta])+
     $field:ident: $bit:literal ..= $bit_end:literal$(,)?
     )+) => {
        $crate::csr! { $(#[$doc])+ $ty: $csr, $mask }

        $(
            $crate::read_only_csr_field! {
                $ty,
                $(#[$field_doc])+
                $field: $bit ..= $bit_end,
            }
        )+

        $crate::read_csr_as!($ty, $csr);
    };
    ...
}

Both branches do something like:

        $crate::csr! { $(#[$doc])+ $ty: $csr, $mask }

        $(
            $crate::read_only_csr_field! {
                $ty,
                $(#[$field_doc])+
                <THIS LINE IS THE ONLY THING THAT CHANGES>
            }
        )+

        $crate::read_csr_as!($ty, $csr);

Looking at this specific example, the macro calls other macros (csr, read_csr_as, and read_only_csr_field a few times). So what is the benefit of having a single macro with many pattern branches that just propagate to other macro calls? Why don't we keep it simple and call in the user code to the read_only_csr_field all the times we want? In this way, we keep all macros simpler and users call them as many times as required.

riscv/src/register/macros.rs

romancardenas · 2024-06-07T17:11:42Z

riscv/src/register/macros.rs

+/// [read_only_csr](crate::read_only_csr), and [write_only_csr](crate::write_only_csr) macros.
+#[macro_export]
+macro_rules! csr {
+    ($doc:expr, $ty:ident: $csr:literal, $mask:literal) => {


We don't need to select the CSR number at this point

Suggested change

($doc:expr, $ty:ident: $csr:literal, $mask:literal) => {

($doc:expr, $ty:ident, $mask:literal) => {

romancardenas · 2024-06-07T17:13:45Z

riscv/src/register/macros.rs

+        $crate::read_csr_as!($ty, $csr);
+        $crate::write_csr_as!($ty, $csr);
+        $crate::set!($csr);
+        $crate::clear!($csr);


I think I'd leave these macros out and call them as we need them

Suggested change

$crate::read_csr_as!($ty, $csr);

$crate::write_csr_as!($ty, $csr);

$crate::set!($csr);

$crate::clear!($csr);

I agree about leaving out the set and clear macros, but think the read_csr_as and write_csr_as should still be included.

Would there ever be a case where one would call:

read_write_csr, but not read_csr_as and write_csr_as

read_only_csr, but not read_csr_as

write_only_csr, but not write_csr_as

In my opinion, including these macro calls avoids an author forgetting to include them. If you really think they should be removed, I'll take them out.

riscv/src/register/macros.rs

romancardenas · 2024-06-10T07:35:20Z

riscv/src/register/macros.rs

+     $field:ident: $get_bit:literal,
+     $set_field_doc:expr,
+     $set_field:ident: $set_bit:literal$(,)?


I think these macros should work in the general use cases (i.e., the same bits for getters and setters). For particular cases, we can always just implement them by hand.

riscv/src/register/macros.rs

romancardenas · 2024-09-18T06:23:17Z

riscv/src/register/macros.rs

+
+    ($ty:ident,
+     $(#[$field_doc:meta])+
+     $field:ident: [$bit_start:literal : $bit_end:literal]$(,)?) => {
+        impl $ty {
+            paste::paste! {
+                $(#[$field_doc])+
+                #[inline]
+                pub fn $field(&self) -> usize {
+                    self.[<try_ $field>]().unwrap()
+                }
+
+                $(#[$field_doc])+
+                #[inline]
+                pub fn [<try_ $field>](&self) -> $crate::result::Result<usize> {
+                    let max_width = core::mem::size_of_val(&self.bits) * 8;
+
+                    if $bit_end < max_width && $bit_start <= $bit_end {
+                        Ok($crate::bits::bf_extract(self.bits, $bit_start, $bit_end - $bit_start + 1))
+                    } else {
+                        Err($crate::result::Error::IndexOutOfBounds {
+                            index: $bit_start,
+                            min: $bit_start,
+                            max: $bit_end,
+                        })
+                    }
+                }
+            }
+        }
+    };
+
+    ($ty:ident,
+     $(#[$field_doc:meta])+
+     $field:ident: {
+         $(#[$field_ty_doc:meta])+
+         $field_ty:ident {
+             range: [$field_start:literal : $field_end:literal],
+             default: $default_variant:ident,
+             $($variant:ident = $value:expr$(,)?)+
+         }$(,)?
+     }$(,)?
+    ) => {
+        $crate::csr_field_enum!(
+            $(#[$field_ty_doc])+
+            $field_ty {
+                range: [$field_start : $field_end],
+                default: $default_variant,
+                $($variant = $value,)+
+            },
+        );
+
+        $crate::read_only_csr_field!(
+            $ty,
+            $(#[$field_doc])*
+            $field: [$field_start : $field_end],
+            $field_ty,
+        );
+    };
+
+    ($ty:ident,
+     $(#[$field_doc:meta])+
+     $field:ident: [$field_start:literal : $field_end:literal],
+     $field_ty:ident$(,)?
+    ) => {
+         impl $ty {
+             paste::paste! {
+                 $(#[$field_doc])+
+                 #[inline]
+                 pub fn $field(&self) -> $field_ty {
+                     self.[<try_ $field>]().unwrap()
+                 }
+
+                 $(#[$field_doc])+
+                 #[inline]
+                 pub fn [<try_ $field>](&self) -> $crate::result::Result<$field_ty> {
+                    let max_width = core::mem::size_of_val(&self.bits) * 8;
+
+                     if $field_end < max_width && $field_start < $field_end {
+                         let value = $crate::bits::bf_extract(
+                             self.bits,
+                             $field_start,
+                             $field_end - $field_start + 1,
+                         );
+
+                         $field_ty::from_usize(value).ok_or($crate::result::Error::InvalidFieldVariant {
+                             field: stringify!($field),
+                             value,
+                         })
+                     } else {
+                        Err($crate::result::Error::IndexOutOfBounds {
+                            index: $field_start,
+                            min: $field_start,
+                            max: $field_end,
+                        })
+                     }
+                 }
+             }
+         }
+    };
+}


Then let's cool this down until we resolve other design decisions. I can say that, as of now, I like how it looks

riscv/src/register/macros.rs

romancardenas · 2024-09-18T06:37:24Z

Hi @rmsyn ! I left you a few comments. Let's first figure out how we want to use the macros and then we can explore using the paste crate (which looks great, by the way).

rmsyn · 2024-09-18T23:16:25Z

Looking at this specific example, the macro calls other macros (csr, read_csr_as, and read_only_csr_field a few times). So what is the benefit of having a single macro with many pattern branches that just propagate to other macro calls? Why don't we keep it simple and call in the user code to the read_only_csr_field all the times we want? In this way, we keep all macros simpler and users call them as many times as required.

Ah, I see what you mean now. Yes, the code could be simplified by not trying to include field definitions in the type-creation macro.

The main instance where including field definitions in the type-creation macro is when all fields are the same type (bit, range, field-mask, etc.). Otherwise, users will need to invoke the field macros anyway.

I agree simplifying is the way to go here.

rmsyn · 2024-09-19T00:09:08Z

Let's first figure out how we want to use the macros and then we can explore using the paste crate (which looks great, by the way).

I've moved the paste work into riscv/csr-macro-paste branch.

When we're settled on the other design work, I can bring that work into this branch.

romancardenas

I have been without an Internet connection, but I had some time to review this PR. Check my comments.

riscv/src/register/macros.rs

romancardenas

Ok so... Looks almost there!

What do you think about trying the paste thing so we can shrink even more the macros? The idea would be:

For getters of field $field: $field (I think there is no fallible getter?)
For setters: try_$field (fallible) and set_$field (infallible)

If you prefer, we can address this in another PR and move on.

romancardenas · 2024-09-24T06:30:20Z

riscv/src/register/tests/read_only_csr.rs

+    assert_eq!(mtest.bits(), 0);
+
+    // check that single bit field getter/setters work.
+    assert_eq!(mtest.single(), false);


Clippy complains at my machine about these kinds of assertions. Should look like:

Suggested change

assert_eq!(mtest.single(), false);

assert!(!mtest.single());

romancardenas

I noticed that we may need to re-export paste. @rmsyn , can you confirm this? If not, I think this PR is ready.

riscv/src/register/macros.rs

romancardenas

LGTM

Let's leave a couple of days in case someone wants to review this and merge it

romancardenas · 2024-10-14T14:41:58Z

If nobody opposes, I will merge this PR tomorrow by the end of the weekly meeting

romancardenas · 2024-10-17T07:34:00Z

@rmsyn can you deal with the conflicts? We will merge it to master right after fixing this.

Adds helper macros for defining CSR types.

Adds the ability to define CSR types using an enum field type.

Adds corrections to CSR creation macros for rebase on commit `a4d69614`. Includes: - additions of `try_*` function variants for fallible functions. - changing doc string macro arguments to idiomatic `///` comments

Replaces repeated field bit and range arguments with a single `bit` or `range` argument. Intended to reduce copy-paste, prevent typo errors, and clarify intent.

Adds `try_*` variants for fallible getter functions on read-only CSR macros.

Adds `try_*` variants for fallible getter functions on write-only CSR macros.

Modularizes the `read_only_csr_field` and `write_only_csr_field` macro helpers for re-use in `read_write_only_csr_field`.

Adds the explicit `mask:` field name to CSR helper macros to clarify intent.

Adds the `field` macro argument label to CSR helper macros to clarify intent.

Removes field arguments from CSR definition macros.

Removes the unused `csr` number argument from the `csr` macro.

Uses the crate result type for converting a CSR enum type from a `usize`.

Applies compile-time checks in read-only CSR macros. Authored-by: rmsyn <github@weathered-steel.dev> Co-authored-by: romancardenas <rcardenas.rod@gmail.com>

Applies compile-time checks in write-only CSR macros. Authored-by: rmsyn <github@weathered-steel.dev> Co-authored-by: romancardenas <rcardenas.rod@gmail.com>

Removes the equality check for range branches in CSR helper macros. Users who want a single-bit field, i.e. `start == end`, should use the single-bit branch variants. Authored-by: rmsyn <github@weathered-steel.dev> Co-authored-by: romancardenas <rcardenas.rod@gmail.com>

Removes CSR helper macro branches that include field enum type creation. Leaves the creation of the field enum to the separate `csr_field_enum` macro, or manual definition of the field enum. Authored-by: rmsyn <github@weathered-steel.dev> Co-authored-by: romancardenas <rcardenas.rod@gmail.com>

Uses the [`paste`](https://docs.rs/paste) dependency for concatenating idents in CSR helper macros.

Re-exports the `paste` macro to allow using CSR helper macros in external crates. Authored-by: rmsyn <github@weathered-steel.dev> Co-authored-by: romancardenas <rcardenas.rod@gmail.com>

rmsyn requested a review from a team as a code owner May 30, 2024 03:59

rmsyn force-pushed the riscv/csr-macro branch from df39d4e to fd163b1 Compare May 30, 2024 04:07

rmsyn commented May 30, 2024

View reviewed changes

romancardenas added the work in progress label May 30, 2024

romancardenas reviewed Jun 4, 2024

View reviewed changes

riscv/src/register/macros.rs Outdated Show resolved Hide resolved

rmsyn commented Jun 4, 2024

View reviewed changes

rmsyn force-pushed the riscv/csr-macro branch from fd163b1 to 37b9403 Compare June 6, 2024 01:23

rmsyn commented Jun 7, 2024

View reviewed changes

riscv/src/register/macros.rs Outdated Show resolved Hide resolved

rmsyn commented Jun 8, 2024

View reviewed changes

rmsyn force-pushed the riscv/csr-macro branch from 31959be to e4b5b12 Compare September 14, 2024 05:29

rmsyn commented Sep 18, 2024

View reviewed changes

romancardenas reviewed Sep 18, 2024

View reviewed changes

rmsyn force-pushed the riscv/csr-macro branch from 629e6a9 to 7ad499a Compare September 19, 2024 00:17

romancardenas reviewed Sep 23, 2024

View reviewed changes

rmsyn commented Sep 24, 2024

View reviewed changes

riscv/src/register/macros.rs Show resolved Hide resolved

romancardenas reviewed Sep 24, 2024

View reviewed changes

romancardenas reviewed Sep 26, 2024

View reviewed changes

riscv/src/register/macros.rs Outdated Show resolved Hide resolved

rmsyn force-pushed the riscv/csr-macro branch from fe000ea to c9cab75 Compare September 27, 2024 00:48

romancardenas previously approved these changes Sep 27, 2024

View reviewed changes

rmsyn and others added 19 commits October 17, 2024 23:58

riscv: add CSR-defining macros

a169176

Adds helper macros for defining CSR types.

riscv: add enum branch to CSR macro

9c431a7

Adds the ability to define CSR types using an enum field type.

fixup: fixes for rebase on latest HEAD

7f4f1f6

Adds corrections to CSR creation macros for rebase on commit `a4d69614`. Includes: - additions of `try_*` function variants for fallible functions. - changing doc string macro arguments to idiomatic `///` comments

fixup: riscv: do not repeat macro field arguments

a1c8119

Replaces repeated field bit and range arguments with a single `bit` or `range` argument. Intended to reduce copy-paste, prevent typo errors, and clarify intent.

riscv: add try variants to read-only CSR macros

49682f0

Adds `try_*` variants for fallible getter functions on read-only CSR macros.

riscv: add try variants to write-only CSR macros

6cf867d

Adds `try_*` variants for fallible getter functions on write-only CSR macros.

riscv: modularize CSR macro field helpers

3084b6c

Modularizes the `read_only_csr_field` and `write_only_csr_field` macro helpers for re-use in `read_write_only_csr_field`.

riscv: add explicit mask field to CSR macros

0367601

Adds the explicit `mask:` field name to CSR helper macros to clarify intent.

riscv: add field label to CSR macros

db53e90

Adds the `field` macro argument label to CSR helper macros to clarify intent.

fixup: riscv: remove field args from CSR macro

6e41d3a

Removes field arguments from CSR definition macros.

fixup: riscv: remove CSR number from csr macro

ec6ef2b

Removes the unused `csr` number argument from the `csr` macro.

fixup: riscv: use result for CSR enum conversion

ac99986

Uses the crate result type for converting a CSR enum type from a `usize`.

fixup: riscv: read-only CSR compile-time asserts

cc061bd

Applies compile-time checks in read-only CSR macros. Authored-by: rmsyn <github@weathered-steel.dev> Co-authored-by: romancardenas <rcardenas.rod@gmail.com>

fixup: riscv: write-only CSR compile-time asserts

cb9310b

Applies compile-time checks in write-only CSR macros. Authored-by: rmsyn <github@weathered-steel.dev> Co-authored-by: romancardenas <rcardenas.rod@gmail.com>

riscv: use paste for CSR helper macros

c7f6e75

Uses the [`paste`](https://docs.rs/paste) dependency for concatenating idents in CSR helper macros.

fixup: riscv: clippy fixes for CSR test assertions

00dc02b

riscv: re-export paste macro

b568220

Re-exports the `paste` macro to allow using CSR helper macros in external crates. Authored-by: rmsyn <github@weathered-steel.dev> Co-authored-by: romancardenas <rcardenas.rod@gmail.com>

rmsyn dismissed romancardenas’s stale review via b568220 October 18, 2024 00:00

rmsyn force-pushed the riscv/csr-macro branch from c9cab75 to b568220 Compare October 18, 2024 00:00

romancardenas approved these changes Oct 18, 2024

View reviewed changes

romancardenas added this pull request to the merge queue Oct 18, 2024

Merged via the queue into rust-embedded:master with commit a35fde8 Oct 18, 2024
101 checks passed

	($doc:expr, $ty:ident: $csr:literal, $mask:literal) => {
	($doc:expr, $ty:ident, $mask:literal) => {

riscv: add CSR-defining macros #219

riscv: add CSR-defining macros #219

Conversation

rmsyn commented May 30, 2024

Choose a reason for hiding this comment

romancardenas commented May 30, 2024

romancardenas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmsyn Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

rmsyn commented Jun 4, 2024

rmsyn commented Jun 6, 2024

romancardenas commented Jun 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmsyn commented Sep 14, 2024

rmsyn commented Sep 15, 2024

romancardenas commented Sep 16, 2024

romancardenas commented Sep 17, 2024

rmsyn commented Sep 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romancardenas commented Sep 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmsyn Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romancardenas commented Sep 18, 2024

rmsyn commented Sep 18, 2024

rmsyn commented Sep 19, 2024

romancardenas left a comment

Choose a reason for hiding this comment

romancardenas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romancardenas left a comment

Choose a reason for hiding this comment

romancardenas left a comment

Choose a reason for hiding this comment

romancardenas commented Oct 14, 2024

romancardenas commented Oct 17, 2024

rmsyn Jun 6, 2024 •

edited

Loading

rmsyn Sep 19, 2024 •

edited

Loading