Add scanLeftOr and scanRightOr utilies

[CircuitCoder]

Contributor Checklist

• Did you add Scaladoc to every public function/method?
• Did you add at least one test demonstrating the PR?
• Did you delete any extraneous printlns/debugging code?
• Did you specify the type of improvement?
• Did you add appropriate documentation in docs/src?
• Did you state the API impact?
• Did you specify the code generation impact?
• Did you request a desired merge strategy?
• Did you add text to be included in the Release Notes for this change?

Type of Improvement

new feature/API

API Impact

Add utility LSBOr and MSBOr similar to rocket-chip's leftOr and rightOr. These functions are generally very useful and can be used to implementing other useful utilities such as bitfield arbitration

Backend Code Generation Impact

N/A

Desired Merge Strategy

Rebase

Release Notes

Add experimental utility LSBOr and MSBOr.

Reviewer Checklist (only modified by reviewer)

• Did you add the appropriate labels?
• Did you mark the proper milestone (Bug fix: 3.4.x, [small] API extension: 3.5.x, API modification or big change: 3.6.0)?
• Did you review?
• Did you check whether all relevant Contributor checkboxes have been checked?
• Did you mark as Please Merge?

[CLAassistant]

All committers have signed the CLA.

[sequencer]

Generally LGTM, name of this: LSBOr or lsbOr or LsbOr should have an alignment to chisel standard library.

[sequencer]

LGTM.

[sequencer]

I think we need to discuss the package name on dev-meeting, hold it till next Monday

[sequencer]

feedback form dev-meeting.

[jackkoenig]

Okay, done bike shedding. We should

1. Name them scanLeftOr and scanRightOr to use consistent terminology with sequence traversals.
2. As far as the implementation, we should use the original from rocket-chip which is O(lg(n)) in node count vs. O(n^2) for this current implementation (note my implementation using scanLeft is O(n))[1].

Per QoR concerns, if someone provides compelling physical design results we can revisit. That being said, SiFive has been building high performance designs using the rocket-chip utility so I doubt there's an issue in proprietary ASIC tools but I will grant that there may be problems using open-source.

[1] Table of node count:

Implementation	Width	Chirrtl Lines	Verilog Lines
rocket-chip	8	18	13
	16	21	15
	32	24	17
	64	27	19
this	8	168	25
	16	584	41
	32	2184	89
	64	8456	241
scanLeft	8	40	17
	16	72	29
	32	136	52
	64	264	104

[mwachs5]

Ok coming in just at the end of this, I'm confused by:

Name them scanLeftOr and scanRightOr to use consistent terminology with sequence traversals.

So is the "Left" relative to a UInt w/ value 0xA written out in binary: b1010 the "leftmost" bit is a 1 or in the 0 in the to-bools vec version of it Seq(0, 1, 0, 1)? I don't really care and think it's fine as long as we document it very heavily.

Reading the above I think we're saying it's the second one. Can we just make it you can't actually call this on a UInt, only on a Vec[Bool]? Then the confusion basically goes away. And I liked the suggestion of calling it scanUpOr when applied to a UInt. Anyway, I'm late to the bikeshed painting party so feel free to ignore my input :)

[jackkoenig]

So is the "Left" relative to a UInt w/ value 0xA written out in binary: b1010 the "leftmost" bit is a 1 or in the 0 in the to-bools vec version of it Seq(0, 1, 0, 1)? I don't really care and think it's fine as long as we document it very heavily.

Reading the above I think we're saying it's the second one.

"Left" means low index to high index in sequences (in every language with traversals, think foldLeft), so the analogy is from low-order bit to high-order bit (think about what the 0th index is on b1010)*. Waterman's suggestion of Up and Down is interesting and I kind of like it, but there's an argument of having additional terminology to learn rather than just learning Left/Right. If we add Up/Down for use on UInts, it suggests we should have consistent terminology on Vecs, but Vec extends Seq so via inheritance it is stuck with Left/Right terminology.

Thus my argument is to stick with Left/Right and just explain it. It's consistent and we're not making up new terminology.

Can we just make it you can't actually call this on a UInt, only on a Vec[Bool]? Then the confusion basically goes away.

No, the whole purpose of this API is that it's useful on UInt and can be done using word-level logic that is more efficient than doing it on a Vec, see the table I made in #2407 (review). Word-level is O(lg(n)) lines of chirrtl/verilog while the best bit-level implementation is O(n) where n is the width of your bitvector.

*I want to note that this sort of ambiguity exists in basically every language. The main difference here is that most languages don't have iteration on bits or digits of numbers. This does suggest that it would be reasonable to invent new terminology (and I think Up/Down is pretty good). The main counterargument is consistency between UInts and Vecs (given that a hypothetical UInt.scanUpOr is equivalent to a hypothetical UInt.asBools.scanLeftOr). We can of course document this, but there is a strong argument to be consistent and have it just be UInt.scanLeftOr and UInt.asBools.scanLeftOr.

[mwachs5]

Thus my argument is to stick with Left/Right and just explain it. It's consistent and we're not making up new terminology.

SGTM. Let me know when I can review the scaladoc :)

[CircuitCoder]

"Left" means low index to high index in sequences

Actually that's a great way to explain the usage or "left" and "right" here. I've added it to the documentation.

[sequencer]

I do think we need a CI to reformat on pulls:(

[mwachs5]

can we update the title of this PR to match the new names?

stale review

[jackkoenig]

LGTM. I deleted the comparison to "shift left" because I think it's more confusing than helpful. I would argue that one could think about "shift left" as shifting from left to right where left is low-order bits, but no one actually does.

I also changed the package to chisel3.util. I'm confident enough in this API that there's no need for experimental.

[jackkoenig]

Thank you @CircuitCoder for your contribution and perseverance at getting this very useful API merged!