google / google/xls

XLS should codegen explicitly-sized sub-expressions to avoid self-determined widths

Open
#2,048 0 comments 0 reactions 0 assignees View on GitHub
codegen enhancement 🧦 sox
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)

XLS generates expressions relying on implicitly sized bitwidths according to LRM Table 11-21. In general, for hardware design, this is problematic because the bitlengths of self-determined expressions can be counterintuitive. Examples:

* `a * b` and `a + b`, etc. produces a result of length `max($bits(a), $bits(b))`. `2'b10 + 2'b10` as a subexpression produces a result of `2'b00`.
* In `logic [7:0] foo; logic bar; logic baz; assign foo = {(bar << 3), (baz << 3)};`, `bar << 3` has length 1 (`$bits(bar)`), and `foo` evaluates to zero.

Due to these types of bugs that can be quite hard to detect in DV, chip projects commonly have lint rules in place to flag self-determined bitlengths in arithmetic expressions that may not produce the intended result. The Google Verilog style guide specifies avoiding these cases by either:

* Casting the sub-expression to a specific width
* Breaking down compound expressions into separate intermediate variables.

XLS knows the bitwidths of everything so should actually do the right thing, but generated code can still hit these lint errors. For example,

https://github.com/google/xls/blob/a69abccc21ece6e734ae8ee6ab3db4c5bdc545e5/xls/dslx/stdlib/apfloat.x#L414-L421

results in something including:

`'23'h7f_ffff << fltirst_lost_bit_idx__1`

where `fltirst_lost_bit_idx__1` is 32 bits, so the lint tool thinks you might expect a larger result. In actuality, we know that the bit index is something in range, and we still expect a 23 bit result.

### Current best alternative workaround (limit 100 words)

We can globally waive this type of lint error for an XLS generated file.

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

Ideally, XLS generated RTL should be lint clean when reasonable and follow established best style practices.

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.