Constraint Mux1H to provide deactivated output when given a zero-hot selector.

[nicmcd]

https://github.com/chipsalliance/chisel3/blob/1c5d1b5317a0c9fe7ef9d15138065a817380a1e4/src/main/scala/chisel3/util/Mux.scala#L24

The documentation for Mux1H says "If zero or multiple selectors are set the behavior is undefined", however, the methodology for implementing 1-hot muxes automatically makes the output all zeros when none of the inputs are selected by the selector bits (aka 0-hot selection). I've tested this with Vec[Bool] and Vec[UInt] and the current code is operating as I described. I propose the documentation support this case officially.

[aswaterman]

If Mux1H is used with single-element vectors, this isn’t true: the lone data is returned. “Unspecified” captures both behaviors.

But I suppose there’s no harm in documenting both the 1-input and (n>1)-input cases explicitly, rather than leaving it unspecified, since it seems extremely unlikely we’d be tempted to change to an alternate unspecified behavior at this point.

[aswaterman]

Unfortunately, one of the four apply methods has different behavior than the others for the 1-input case: https://github.com/chipsalliance/chisel3/blob/1c5d1b5317a0c9fe7ef9d15138065a817380a1e4/src/main/scala/chisel3/util/Mux.scala#L30

This one arguably shouldn't exist: it's actually less code to inline the expression (sel & in).orR in this case anyway. So, my proposal is to deprecate this one, and after it's gone, we can document the unspecified behavior.

[nicmcd]

If Mux1H is used with single-element vectors, this isn’t true: the lone data is returned.

Is there a reason this is preferred? Seems to me that a 0-hot input is currently considered invalid. The single element vectors appears to be handled by a special case in oneHotMux (correct me if I'm wrong). Could that case be made to return zeros then the functionality for 0-hot select would be specified and accurate for all cases?

This one arguably shouldn't exist: it's actually less code to inline the expression.

I don't agree with this. Using the name Mux1H describes intent and makes the code readable. If one wrote (sel & in).orR instead, another reader would find it hard to understand what is happening. Not everyone knows that 1-hot muxes are and/or trees, but everyone should know what a Mux is.

[aswaterman]

Is there a reason this is preferred?

Yes, it reduces area and latency.

[aswaterman]

I don't agree with this. Using the name Mux1H describes intent and makes the code readable. If one wrote (sel & in).orR instead, another reader would find it hard to understand what is happening. Not everyone knows that 1-hot muxes are and/or trees, but everyone should know what a Mux is.

I think that’s a defensible POV, but if we retain this method, we’re probably going to leave the documentation as-is and close this ticket.

[nicmcd]

That is probably the best solution. For my purposes, I have unit tests that verify the "undefined" behavior for my project and I'll rely on it. If the implementation underneath changes in a future causing the 0-hot case behavior to change, my tests should catch that.