Add Clash numeric types plus basic `impl`s by rslawson · Pull Request #1176 · bittide/bittide-hardware

rslawson · 2026-02-02T18:28:26Z

This implements basic functionality for Index<N, T>, Unsigned<N, T>, Signed<N, T>, and BitVec<N, M>. This PR is currently marked as draft since I still need to:

Add documentation to the types themselves and to the size check traits, not just their methods
Create macros as aliases for the types
Create instantiation macros
Add tests?

Anywho, at least an initial review pass and/or suggestions on things I should add in this PR that I didn't (and that I don't have WIP stuff for already from before I did a lot of downscoping, like operator impls).

hydrolarus · 2026-02-03T07:08:00Z

Have you done any tests to see if generated code does not need any trait bounds? To me it seems like it it shouldn't be required, but there's no usage examples, just making sure since I'm afraid of trait bounds 😱

rslawson · 2026-02-03T08:38:18Z

Have you done any tests to see if generated code does not need any trait bounds? To me it seems like it it shouldn't be required, but there's no usage examples, just making sure since I'm afraid of trait bounds 😱

I haven't done that yet, no. That's what I'll be working on today, so hopefully it's something I can complete today (^:

hydrolarus · 2026-02-27T10:21:40Z

firmware-support/bittide-macros/src/lib.rs

+            let input_str = input.to_string();
+            if !(input_str.starts_with("0x") || input_str.starts_with("0X"))
+                || input_str
+                    .chars()
+                    .skip(2)
+                    .any(|c| !(c.is_ascii_hexdigit() || c == ' ' || c == '_'))
+            {
+                return syn::Error::new(
+                    input.span(),
+                    concat!(
+                        "Expected a hexadecimal literal! Must start with `0x` or `0X` and contain ",
+                        "only hexdigits, spaces, and underscores",
+                    )
+                    .to_string(),
+                )
+                .into_compile_error()
+                .into();
+            }


Wouldn't it make sense to also support binary literals and maybe even base 10 literals?

I could add support for binary literals pretty easily, just the issue is that for decimal literals there's the issue of not having a way to turn that into binary very easily past a certain size.

firmware-support/bittide-hal/src/manual_additions/bitvector.rs

firmware-support/bittide-hal/src/manual_additions/index.rs

firmware-support/bittide-hal/src/manual_additions/signed.rs

firmware-support/bittide-hal/src/manual_additions/calendar.rs

hydrolarus · 2026-03-03T07:35:29Z

firmware-support/bittide-macros/src/lib.rs

+            let mut char_iter = input_str.chars().filter(char::is_ascii_hexdigit).rev();
+            let mut lower = true;
+            let mut idx = 0;
+            loop {


I've been thinking if there is a good way to use from_str_radix for this instead of hand rolling this, but I guess you want arbitrarily large literals too, so this wouldn't work 😔 Maybe this could be a comment here though? Otherwise I'll wonder the same thing in two months when I look at this again haha

hydrolarus · 2026-03-03T08:06:14Z

firmware-support/bittide-macros/src/lib.rs

+                array_str.push('[');
+                for byte in buf {
+                    write!(array_str, "0x{byte:02x}, ").unwrap();
+                }
+                array_str.pop();
+                array_str.pop();
+                array_str.push(']');


This could check for the first element to have less manual shuffling of the characters in the string

Suggested change

array_str.push('[');

for byte in buf {

write!(array_str, "0x{byte:02x}, ").unwrap();

}

array_str.pop();

array_str.pop();

array_str.push(']');

array_str.push('[');

for (i, byte) in buf.iter().enumerate() {

if i > 0 {

write!(array_str, ", ").unwrap();

}

write!(array_str, "0x{byte:02x}").unwrap();

}

array_str.push(']');

rslawson · 2026-03-18T13:52:36Z

Marked as ready for review. I know there's one more thing I need to do (sign extension for Signed<N, T> when converting from BitVector<M, N>) before it's actually ready to go, but what with all the other things I want to do done plus all the tests passing I'm happy to call it ready for review.

NOTE: WILL NOT PASS CI

hydrolarus

Overall looking good, but some questions regarding macro hygiene, style of type macros and usage of expr-macros

hydrolarus · 2026-03-23T09:49:13Z

firmware-support/memmap-generate/src/backends/rust/mod.rs

+                let n = n as usize;
+                let len = n.div_ceil(8);
+                quote! { BitVector<#n, #len> }


This leaves suffixes everywhere, right? So BitVector<24usize, 3usize>. Wouldn't it make sense to use unsuffixed literals since usually the types of the literals aren't so important on the type level?

Same goes for the other types below

hydrolarus · 2026-03-23T09:53:33Z

firmware-support/bittide-macros/src/lib.rs

+    fn get_correct_size(n: u128) -> u32 {
+        let clog2 = (u128::BITS - (n - 1).leading_zeros()).next_power_of_two();
+        clog2.max(8)
    }


The name is a bit non-descriptive. Maybe native_size_in_bits or something? Same for the other local functions below

hydrolarus · 2026-03-23T09:56:55Z

firmware-support/bittide-macros/src/lib.rs

-        bittide_hal::manual_additions::index::IndexTy<#n_lit, #ty>
+        Index<#n_lit, #ty>


Sadly proc macros don't have the same hygiene as declarative macros... I guess it's because of using the macros in the bittide_hal? Otherwise I feel like the fully qualified version would be more robust?
Maaaybe it's possible to check if the macro is being invoked in bittide_hal and then change the prefix?

hydrolarus · 2026-03-23T09:58:00Z

firmware-support/bittide-macros/src/lib.rs

+    quote! {
+        crate::manual_additions::signed::Signed<#n_lit, #ty>
+    }
+    .into()


Here it's using crate again, so this wouldn't work outside of bittide_hal?

hydrolarus · 2026-03-23T10:01:44Z

firmware-support/bittide-hal/src/manual_additions/scatter_gather_pe/scatter_gather.rs

    pub fn write_slice(&self, src: &[[u8; 8]], offset: usize) {
        assert!(src.len() + offset <= Self::GATHER_MEMORY_LEN);
        let mut off = offset;
        for &val in src {
            unsafe {
-                self.set_gather_memory_unchecked(off, val);
+                self.set_gather_memory_unchecked(off, BitVector::new_unchecked(val));


Haven't checked usage sites of this function yet, but with the changed code generator, wouldn't src: &[BitVector![64]] make this easier? Same for the ScatterUnit below and the other scatter_gather impls in the other files

hydrolarus · 2026-03-23T10:44:17Z

firmware-binaries/sim-tests/elastic_buffer_wb_test/src/main.rs


        // Set margin and enable
-        elastic_buffer.set_auto_center_margin(margin);
+        elastic_buffer.set_auto_center_margin(Unsigned::new(margin).unwrap());


Shouldn't unsigned!(margin, n = N) work here too?

hydrolarus · 2026-03-23T10:45:25Z

firmware-binaries/sim-tests/elastic_buffer_wb_test/src/main.rs


        // Manually adjust buffer off-center
-        elastic_buffer.set_adjustment(perturbation);
+        elastic_buffer.set_adjustment(Signed::new(perturbation).unwrap());


Same here, signed!(perturbation, n = N)

hydrolarus · 2026-03-23T10:45:45Z

firmware-binaries/sim-tests/nested_interconnect_test/src/main.rs

+        peripheral.set_status(Unsigned::new(*status_val).unwrap());
        write!(name_buf, "peripheral{}.status.readback", i).unwrap();
-        expect(&name_buf, *status_val, peripheral.status());
+        expect(&name_buf, *status_val, peripheral.status().into_inner());
        name_buf.clear();

        // Write and read back control
-        peripheral.set_control(*control_val);
+        peripheral.set_control(Unsigned::new(*control_val).unwrap());


unsigned!(<expr>, n = N)

hydrolarus · 2026-03-23T10:46:58Z

firmware-binaries/sim-tests/switch_calendar_test/src/main.rs

 fn main() -> ! {
-    let active_entry0: [u8; 16] = core::array::from_fn(|i| i as u8);
-    let mut active_entry1: [u8; 16] = active_entry0;
+    let active_entry0 = core::array::from_fn(|i| Index::new(i as u8).unwrap());


index!(i as u8, n = _)

hydrolarus · 2026-03-23T10:47:29Z

firmware-binaries/sim-tests/switch_calendar_test/src/main.rs

+            ve_repeat: Unsigned::new(8).unwrap(),
        },
        ValidEntry_12 {
            ve_entry: active_entry1,
-            ve_repeat: 16,
+            ve_repeat: Unsigned::new(16).unwrap(),
        },
    ];

    let cal_shadow: [CalendarEntryType<Calendar>; 16] = core::array::from_fn(|i| ValidEntry_12 {
-        ve_entry: core::array::from_fn(|_j| i as u8),
-        ve_repeat: i as u16,
+        ve_entry: core::array::from_fn(|_j| Index::new(i as u8).unwrap()),
+        ve_repeat: Unsigned::new(i as u16).unwrap(),


Same here, contruction macros should probably be preferred unless there's a good reason to not use them

rslawson force-pushed the rs/numeric-types branch from 85ac603 to fed0aeb Compare February 25, 2026 15:51

martijnbastiaan requested a review from hydrolarus February 26, 2026 08:25

hydrolarus reviewed Feb 27, 2026

View reviewed changes

rslawson force-pushed the rs/numeric-types branch 5 times, most recently from 3c4d3da to 5da7507 Compare March 2, 2026 15:46

rslawson requested a review from hydrolarus March 2, 2026 18:09

hydrolarus reviewed Mar 3, 2026

View reviewed changes

rslawson force-pushed the rs/numeric-types branch 2 times, most recently from dbbb82e to f2ca52c Compare March 3, 2026 12:27

rslawson force-pushed the rs/numeric-types branch 3 times, most recently from 2bf627f to cdd3069 Compare March 18, 2026 12:31

rslawson marked this pull request as ready for review March 18, 2026 13:50

rslawson added 5 commits March 19, 2026 15:53

Add Clash numeric types plus basic impls

0e8f9d7

NOTE: WILL NOT PASS CI

Create macros for the new numeric types

b29bfe0

NOTE: WILL NOT PASS CI

Make codegen use new numeric types

6860335

NOTE: WILL NOT PASS CI

Adapt firmware-support/ code to the new numeric types

ecf65f4

NOTE: WILL NOT PASS CI

Adapt firmware-binaries/ code to new numeric types

5dc442f

rslawson force-pushed the rs/numeric-types branch from cdd3069 to d00584e Compare March 19, 2026 14:53

Add Rust tests into CI

95fa3d0

rslawson force-pushed the rs/numeric-types branch from d00584e to 95fa3d0 Compare March 19, 2026 15:10

rslawson requested a review from hydrolarus March 19, 2026 15:16

hydrolarus reviewed Mar 23, 2026

View reviewed changes

		bittide_hal::manual_additions::index::IndexTy<#n_lit, #ty>
		Index<#n_lit, #ty>

Conversation

rslawson commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hydrolarus commented Feb 3, 2026

Uh oh!

rslawson commented Feb 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rslawson commented Mar 18, 2026

Uh oh!

hydrolarus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rslawson commented Feb 2, 2026 •

edited

Loading