Add SparseVec

[seldridge]

Add a new utility, SparseVec. This exposes a normal Vec-like dynamic access API. However, the dynamic access is implemented one of three ways:

1. Using a binary decoder
2. Using a one-hot decoder
3. Using when statements

This is added to better support a design pattern ("vec--decoder pattern") where a large, sparse vector is used to describe a decoder. This pattern, if unchecked and used in a parametric generator, can cause the vector to grow extremely large and cause backend tools to complain.

Specifically, Cadence tooling is known to set limits on the addressable range of a "memory" and will error if these are exceeded. This vec--decoder pattern can easily exceed this if designers are not careful.

Release Notes

Add SparseVec API.

[sequencer]

Some nitpicking ;p

[sequencer]

Default to N? What if input address is not in the Sparse Vector? Should we assert it out?

[seldridge]

The default value should probably be configurable. This SparseVec is intended to replace this pattern for something like sourceStall: https://github.com/chipsalliance/rocket-chip/blob/f11fb1aea0033cf87aebcc9e5d3961810d1dcf57/src/main/scala/tilelink/ToAXI4.scala#L106 We've run into issues where a Vec like this gets comically large for certain parameterizations.

The test in this PR is modeled off of this example. Roughly:

1. You create a Vec Wire.
2. You set everything in the Vec to zero.
3. You set a few values in the Vec. This can be either constants (see: sourceTable) or expressions (see: sourceStall).
4. You dynamically index out of the Vec.

This is basically describing a decoder which should be optimized by synthesis tools. However, it looks really bad and can trip limits on dynamic indexes for "memories" for some tools. Specifically, Cadence Genus will warn if you try to dynamically index something larger that 16Ki.

There are some subtleties here where the dynamic index has both a default value and an out-of-bounds value. I'll put a comment later about this.

[seldridge]

The right thing to do here is probably return BitPat.dontCare(addrWidth) though this should be configurable.

[seldridge]

Note that this has some interesting interactions when trying to use this to replace a dynamic index into a Vec. The Scala-based FIRRTL Compiler (SFC) has always treated an out-of-bounds index into a Vec as returning the zero'th element. The CIRCT developers have tried to change this but we have discovered that designers have intentionally or unintentionally written designs which rely on this behavior. 😢 The current version of this PR differs in behavior from a dynamic index into a Vec. Commit 7b5d6fc includes a test of this behavior that shows a difference in the results between a dynamic index and a SparseVec.

Normally, you would leave this behavior up to a FIRRTL compiler to sort out (as the FIRRTL spec states that an out-of-bounds access can return a different indeterminate value for each out-of-bounds element). However, with SparseVec, Chisel needs to eagerly choose what the out-of-bounds behavior is. Eventually, this behavior will likely diverge between SparseVec and dynamic out-of-bounds index. I will update this PR to use the current behavior of dynamic index out-of-bounds behavior (possibly under an option).

[jackkoenig]

I'm a little reluctant to give up the class name SparseVec to a non-Data (since it would be a breaking change to try to make it a Record after the fact). I'm not saying we need to do the Data API right this moment, but maybe we adopt a slightly different API which is just factory methods that take the mapping, addr, and Type and just go ahead and do the dynamic lookup there?

[seldridge]

Moved this to extend Record.

[jackkoenig]

Looks awesome, some nits, a maybe bug, and a request

[jackkoenig]

I think this is wrong for one-hot, I think you need to use just size, maybe this should be a method defined on the SparseVec.type?

[seldridge]

This is the number of bits necessary to represent the size of the vec, not the number of bits necessary for the internal representation.

The lookup is roughly: UInt(addrWidth.W) -> chiselTypeOf(encoding(_, encodedSize)) -> Mux.

The encodedSize (number of unique entries) is passed to the encoding to let it determine the size. This looks to be correct for the one-hot, but looks overprovisioned for the binary. But, yes, the internal encoding may get much larger which is what the encoding method is trying to handle.

I think I got that right. Does that logic make sense?

[seldridge]

I fixed the sizing of the binary case here: e4e0760 This also fixed the warnings which I incorrectly thought was due to valid use of out-of-bounds indexing. My bad.

[jackkoenig]

I don't think this logic is correct when you have SparseVec.OutOfBoundsBehavior.First combined with no element at index 0. In order to be equivalent to a dense Vec, we need to give a "fake" zero-th value which effectively is the default value. Basically, I think this can be fixed as follows:

Suggested change

	(outOfBoundsValue, zeroValue) match {
	case (SparseVec.OutOfBoundsBehavior.Indeterminate, _) | (_, None) =>
	case (SparseVec.OutOfBoundsBehavior.First, Some(data)) =>
	when(addr >= size.U) {
	result := data
	}
	}
	outOfBoundsValue match {
	case SparseVec.OutOfBoundsBehavior.Indeterminate =>
	case SparseVec.OutOfBoundsBehavior.First, Some(data)) =>
	val oob = zeroValue.getOrElse(defaultValue.getValue(lookupType)) // assign result of this expression to a val up above
	when(addr >= size.U) {
	result := oob
	}
	}

This could use a test. This case I'm describing here came up in the real design when I deployed this same basic idea (but in a much less reusable way, this PR is awesome).

[seldridge]

I added a test case. However, this seems to pass. I think this logic is correct for the reason that the result is already set, outside of any when, to defaultValue.getValue(lookupType). Hence, the PR should only need to set the value to the zeroValue and just let it fallback to the default.

Does that make sense?

[jackkoenig]

Doesn't the size need to be at least 4 for this to have out-of-bounds access?

[seldridge]

The access needs to be padded up to a power of 2, yes. However, the size of the vec should be smaller. The test checks all values up to the next power of 2.

This test is doing is constructing a SparseVec and a dense Vec of size 3. The dense Vec has all values initialized to DontCare (elements [0, 1, 2]). The user-provided indices in both the Vec and SparseVec are then set ([1, 2]). The test then checks that all indices, including out-of-bounds indices match. I.e., all of [0, 1, 2, 3] match.

There's a debug flag to the test that shows the index/output behavior for this test. Check if this looks right to you, specifically, 0 -> 0 and 3 -> 0:

index, dense, binary, onehot, ifelse
0: 0, 0, 0, 0
1: 2, 2, 2, 2
2: 3, 3, 3, 3
3: 0, 0, 0, 0

Edit: the debug flag (true above) was accidentally already on for this test and the output doesn't include the headers shown. This was updated in the latest commit to not be on and to add the headers.

[seldridge]

I added some additional comments to the tests to indicate what is going on here.

[jackkoenig]

Awesome work dude, especially on the tests. This PR should serve as an example on how to test things.