Mux1H: don't special-case (sel: UInt, in: UInt)

[ingallsj]

Related issue: #1314

Type of change: bug report

Impact: API modification

Development Phase: proposal

Release Notes
if instantiating a Mux1H(sel: UInt, in: UInt) when in.getWidth==1, do the same behavior as instantiating a Mux1H(sel.asBools, in.asBools): ignore the select and connect the input directly to the output

[claassistantio]

All committers have signed the CLA.

[aswaterman]

Bringing this Mux1H API in line with the others seems like a good thing to do, but it raises some subtle issues:

• This new version calls asBools, so it'll bail out if the width isn't known at elaboration time, whereas the previous version would just work.
• Even though this brings the API in line with the others, and even though both the old and new behaviors are within the bounds of a one-hot API, it is a change that some users could theoretically notice. I guess we need a release note or something.
• The original @note is incomplete, as it fails to mention the zero-hot case. It should say "results unspecified unless exactly one select signal is high". Then the new note you added isn't necessary.

It is possible to get rid of the dependence on asBools by doing something esoteric like

val isLessThanTwoBitsWide = (sel + 1.U) === (sel - 1.U)
Mux(isLessThanTwoBitsWide, in, (sel & in).orR)

where the first statement is a way to check whether sel is one bit wide, but to postpone the check til Firrtl time. (There's got to be a better way of doing that...)

[albert-magyar]

@aswaterman, I think the idea for your example is that the Mux will be optimized away via const-prop after the select case is coerced to a constant?

If so, I imagine that implicitly depends on the idea that (sel + 1.U) and (sel - 1.U) will be strength-reduced to matching expressions for 1-bit sel. This currently does not happen in FIRRTL but seems reasonable to add.

It also in turn depends on FIRRTL const-propping a == a, which I had assumed might already be the case but is actually not currently done. I just PRed those matching-argument degeneracies to const prop in chipsalliance/firrtl#1361.

[aswaterman]

Yeah, we would obviously want some support from Firrtl. Synthesis will get rid of this for sure, but the Verilog is still unreadable.

…

On Thu, Feb 6, 2020 at 5:48 AM Albert Magyar ***@***.***> wrote: @aswaterman <https://github.com/aswaterman>, I think the idea for your example is that the Mux will be optimized away via const-prop after the select case is coerced to a constant? If so, I imagine that implicitly depends on the idea that (sel + 1.U) and (sel - 1.U) will be strength-reduced to matching expressions for 1-bit sel. This currently does not happen in FIRRTL but seems reasonable to add. It also in turn depends on FIRRTL const-propping a == a, which I had assumed might already be the case but is actually not currently done. I just PRed those matching-argument degeneracies to const prop in chipsalliance/firrtl#1361 <chipsalliance/firrtl#1361>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1320>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAH3XQVK5XA3R2664VYCOL3RBQILTANCNFSM4KQXMTGA> .

[ingallsj]

#1314 (comment) :

I definitely don’t want to change the behavior of the 1-wide case for the other ones. If you want the and-or thing even in the 1-wide case, it’s a one-liner in Scala, or you can make your own library method for it rather than using the Chisel one.

will do