Consolidate apfloat "round with ties to nearest even" logic
- Dominant language
- C++
- Stars
- 1.9k
- Forks
- 283
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 135
Description
During code review of https://github.com/google/xls/commit/f8853882164c629c39aa3fb75336d4d7615d1729, we discussed that there would now be multiple (different) implementations of "should we round up (with ties to nearest even)" for apfloats. Specifically, we have:
https://github.com/google/xls/blob/53f30d068f9bb9e7888ecbe4c34273f5c3223e27/xls/dslx/stdlib/apfloat.x#L280-L298
https://github.com/google/xls/blob/53f30d068f9bb9e7888ecbe4c34273f5c3223e27/xls/dslx/stdlib/apfloat.x#L794-L806
We left it as is because of a few considerations:
1. The implementations are different, and it would be nice to compare QoR.
2. Neither implementation is part of the public API.
3. `rne` (unintentionally?) adds behavior for `lsb_idx` overflowing the input fraction size (https://github.com/google/xls/blob/53f30d068f9bb9e7888ecbe4c34273f5c3223e27/xls/dslx/stdlib/apfloat.x#L311), while the latter knows this will never happen by construction.
4. `rne` may benefit from renaming since it doesn't do any "rounding" as implied by the name.
Contributor guide
Assessment
This issue has not been assessed yet.