Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[enhancement] Please clearly state that std::umul and umulp work differently. #1811

Open
nezumi-tech opened this issue Dec 20, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@nezumi-tech
Copy link

What's hard to do? (limit 100 words)

For std::umul, the number of bits in the output increases with respect to the input. However, umulp(), which has a similar name to std::umul, does not change the number of bits on input and output.
I'm a newbie to Rust, and the difference is counter-intuitive, so I struggled with it for about 30 minutes.

Following is my Google Colab code to test pipelined multiplication and adder.

%%dslx --top=muladd --clock_period_ps=1000 --flop_inputs=false --flop_outputs=false

import std;

fn muladd(a: u8, b: u8, c: u8) -> u16 {
    // let product = std::umul(a, b); // Output is u16
    // product + c as u16

    let (p, s) = umulp(a, b); // Output is u8 because inputs are u8.
    (p + s) as u16 + c as u16
}

#[test]
fn muladd_test() {
  assert_eq(muladd(u8:2, u8:2, u8:2), u16:6);
  assert_eq(muladd(u8:3, u8:2, u8:2), u16:8);
  assert_eq(muladd(u8:2, u8:4, u8:1), u16:9);
  assert_eq(muladd(u8:16, u8:16, u8:16), u16:272);
  assert_eq(muladd(u8:255, u8:255, u8:255), u16:65280);
}

Error output is as below.
Image

Since umulp() outputs p + s = 0 not 256, 0 + 16 = 16 is the result.

Current best alternative workaround (limit 100 words)

For multiplication between 8-bit numbers, the output should have 16 bits. Therefore, casting the input to type u16 solved the problem.

%%dslx --top=muladd --clock_period_ps=1000 --flop_inputs=false --flop_outputs=false

import std;

fn muladd(a: u8, b: u8, c: u8) -> u16 {
    // let product = std::umul(a, b);
    // product + c as u16
    let (p, s) = umulp(a as u16, b as u16);  // Output is u16 because inputs are u16.
    (p + s) as u16 + c as u16
}

#[test]
fn muladd_test() {
  assert_eq(muladd(u8:2, u8:2, u8:2), u16:6);
  assert_eq(muladd(u8:3, u8:2, u8:2), u16:8);
  assert_eq(muladd(u8:2, u8:4, u8:1), u16:9);
  assert_eq(muladd(u8:16, u8:16, u8:16), u16:272);
  assert_eq(muladd(u8:255, u8:255, u8:255), u16:65280);
}

Output is good.
Image

Your view of the "best case XLS enhancement" (limit 100 words)

Please clearly state in the documentation that std::umul and umulp work differently.

@nezumi-tech nezumi-tech added the enhancement New feature or request label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant