google / google/xls

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

Open
#1,811 0 comments 0 reactions 0 assignees View on GitHub
documentation enhancement
Dominant language
C++
Stars
1.9k
Forks
283
Avg merge
2d 10h
Merged PRs (30d)
135

Description

### 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.
```rust
%%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](https://github.com/user-attachments/assets/16da99e7-191d-4541-ae08-59c814ffcebe)

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.
```rust
%%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](https://github.com/user-attachments/assets/6fc610ce-0d43-46d7-ae2e-e2cdf1adab22)

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

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

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.