chipsalliance
RFC: Deprecate either Decoupled or DecoupledIO?
Type of issue: feature request
Impact: API modification
Development Phase: proposal
Other information
If the current behavior is a bug, please provide the steps to reproduce the problem:
What is the current behavior?
User does something like:
val foo: DecoupledIO[Bar] = Decoupled(bar)
There is a mental burden to remember which one is DecoupledIO and which one is Decoupled, for no good reason.
What is the expected behavior?
User would write:
val foo: Decoupled[Bar] = Decoupled(bar)
OR
val foo: DecoupledIO[Bar] = DecoupledIO(bar)
We'd deprecate the other one for a version or so.
Similar for ReadyValids and such, I didn't do a comprehensive survey
Please tell us about your environment:
What is the use case for changing the behavior?
Less mental load for users.
Similar for ReadyValids and such, I didn't do a comprehensive survey
It would be awesome if you could make a list of our bundles in the standard library and how they are named. Having some consistency there would also help make this a more polished experience with users. You are making a great case for having the type and the constructor have the same name. Now the next question is whether all bundles should end in IO.
It would be awesome if you could make a list of our bundles in the standard library and how they are named.
Except for Valid, the ...IO version seems strongly favored:
util/Valid.scala:class Valid[+T <: Data](gen: T) extends Bundle {
util/Valid.scala: class PipeIO extends Bundle {
util/Decoupled.scala:abstract class ReadyValidIO[+T <: Data](gen: T) extends Bundle {
util/Decoupled.scala: QueueIO ... extends Bundle { // See github.com/freechipsproject/chisel3/issues/765 for why gen is a private val and proposed replacement APIs.
class DecoupledIO[+T <: Data](gen: T) extends ReadyValidIO[T](gen)
util/Arbiter.scala:class ArbiterIO[T <: Data](private val gen: T, val n: Int) extends Bundle {
util/random/PRNG.scala:class PRNGIO(val n: Int) extends Bundle {
sh-4.4$ grep -r 'extends ReadyValidIO' *
util/Decoupled.scala:class DecoupledIO[+T <: Data](gen: T) extends ReadyValidIO[T](gen)
util/Decoupled.scala:class IrrevocableIO[+T <: Data](gen: T) extends ReadyValidIO[T](gen)
The Valid not having IO in its name is confusing since it does ascribe some direction, Output, to the bit it is adding.
Unless we get in the habit of suffixing the class names of all Records and Bundles with "IO" i think we should drop it.
Unless we get in the habit of suffixing the class names of all Records and Bundles with "IO" i think we should drop it.
Personally that is what I generally do. Another option that I have seen used a lot in rocket chip is to use Bundle as a suffix which is a little more accurate (not all bundles are used as I/Os) but also makes names a bit longer.
not all bundles are used as I/Os
True, but wonder if it is useful to capture bi-directionality in the name IO, vs a Bundle which to me implies no directionality?
True, but wonder if it is useful to capture bi-directionality in the name
IO, vs aBundlewhich to me implies no directionality?
So would the guideline be: if you specify a directional in your bundle (i.e. if you use Input, Output or Flipped) then you should suffix with IO? That seems like an interesting idea.
So would the guideline be:
I guess, but not one that is widely adopted, so I'd also be OK with just dropping the suffixes.
Another better guideline might be "if your thing is of type NounIO it should be like that, because it's certainly not an Noun (e.g. ArbiterIO, PRNGIO).
But "Decoupled" is describing the IO itself, not the noun it the IO of"
But "Decoupled" is describing the IO itself, not the noun it the IO of"
I feel like Objects or Types (not including traits/interfaces) should always be named with a noun (and optional qualifying adjectives). So DecoupledIO would be an IO that is decoupled.
Another option that I have seen used a lot in rocket chip is to use Bundle as a suffix which is a little more accurate (not all bundles are used as I/Os) but also makes names a bit longer.
I agree -- that bundle does not imply (module) IO is why i think we should move away from it. Maybe that's too pedantic...
True, but wonder if it is useful to capture bi-directionality in the name IO, vs a Bundle which to me implies no directionality?
See it doesn't for me -- but it don't think it would be hard to define a new type that imposed unidirectionality of fields (and so would statically enforce that association).
I am in favor of Bundles that are used as IO should have IO in their name. Therefore, I would like DecoupledIO. Or, as in #1350, better call it ReadyValidIO. ready/valid is used in textbooks and in AXI.