chisel icon indicating copy to clipboard operation
chisel copied to clipboard

Add Incoming/Outgoing as alternatives to IO

Open azidar opened this issue 1 year ago • 11 comments

Motivation

First step in implementing what was discussed in this issue:

https://github.com/chipsalliance/chisel/issues/2643

In short, Incoming and Outgoing provide a way to create input/output ports that does not "strip flips" like Input/Output do. The alternative way to do this was to create IO(Flipped(..)) and IO(..), which requires the implicit knowledge that IO creates an output port.

I'm in general curious for feedback on this API, so I'm marking as a draft for now. Thanks!

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 request a desired merge strategy?
  • [ ] Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • Feature (or new API)
  • API modification
  • API deprecation
  • Backend code generation
  • Performance improvement
  • Bugfix
  • Documentation or website-related
  • Dependency update
  • Internal or build-related (includes code refactoring/cleanup)

Desired Merge Strategy

  • Squash: The PR will be squashed and merged (choose this if you have no preference).
  • Rebase: You will rebase the PR onto master and it will be merged with a merge commit.

Release Notes

Reviewer Checklist (only modified by reviewer)

  • [ ] Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
  • [ ] Did you mark the proper milestone (Bug fix: 3.5.x, 3.6.x, or 5.x depending on impact, API modification or big change: 6.0)?
  • [ ] Did you review?
  • [ ] Did you check whether all relevant Contributor checkboxes have been checked?
  • [ ] Did you do one of the following when ready to merge:
    • [ ] Squash: You/ the contributor Enable auto-merge (squash), clean up the commit message, and label with Please Merge.
    • [ ] Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

azidar avatar Nov 06 '23 19:11 azidar

Bikeshed-level: I'd opt for the terser: In and Out vs. Incoming and Outgoing. This has the coincidence that in/out were the compromise names between favoring terseness and explicitness that were adopted for dialects in CIRCT.

Adding to the bikeshed i like that:

val foo = Incoming(Bool()) val bar = Outgoing(Bool())

line up 🤷

mwachs5 avatar Nov 09 '23 17:11 mwachs5

And another required thing might be providing a Decoupled and Valid equivalent type, and even deprecate the coerced one.

sequencer avatar Nov 09 '23 17:11 sequencer

What would an explicit Aligned do? Isn't that a no-op?

seldridge avatar Nov 10 '23 05:11 seldridge

What would an explicit Aligned do? Isn't that a no-op? How about provide a semantic:

Aligned(Flipped(MyThing()))
Aligned(Aligned(MyThing()))

Both equal to

Aligned(MyThing())

Aligned will ignore the inner Flipped.

sequencer avatar Nov 14 '23 17:11 sequencer

No @sequencer that is not what I mean. I am agreeing for a noop operator that does nothing, simply for the alignment (no pun intended) of the words.

mwachs5 avatar Nov 14 '23 18:11 mwachs5

Is it the case that the following are indistinguishable from a connection/flow perspective? (modulo directly inspecting the type) val a = Incoming(Bundle(flip a : UInt)) val b = Outgoing(Bundle(a : UInt))

darthscsi avatar Nov 27 '23 17:11 darthscsi

As @seldridge mentioned, Incoming and Outgoing are unusual names. VHDL uses simply in and out. Verilog uses input and output. Both should be fine.

schoeberl avatar Nov 27 '23 17:11 schoeberl

Is it the case that the following are indistinguishable from a connection/flow perspective? (modulo directly inspecting the type) val a = Incoming(Bundle(flip a : UInt)) val b = Outgoing(Bundle(a : UInt))

Written in actual user Chisel not firrtly-psuedocode:

val a = Incoming(new Bundle{ val a = Flipped(UInt())})
val b = Outgoing(new Bundle{ val a = UInt()})

val a_wire = Wire(new Bundle{ val a = Flipped(UInt())})
val b_wire = Wire(new Bundle{ val a = UInt()})

a :<>= a_wire
b :<>= b_wire

Not the same: https://scastie.scala-lang.org/exwHPt8vSEWpyd7N2SsKmQ

...

connect a_wire.a, a.a @[src/main/scala/main.scala 13:3]
connect b.a, b_wire.a @[src/main/scala/main.scala 14:3]

We can tell the users to write something different, but the above is the common thing people are doing intuitively.

What if the wires have no flips to match:

val a = Incoming(new Bundle{ val a = Flipped(UInt())})
val b = Outgoing(new Bundle{ val a = UInt()})

val a_wire = Wire(new Bundle{ val a = UInt()})
val b_wire = Wire(new Bundle{ val a = UInt()})

a :<>= a_wire
b :<>= b_wire

They get an error: (https://scastie.scala-lang.org/xnj0D741RsqWhoAvSk4hZA)

 src/main/scala/main.scala 13:3: inversely oriented fields Example.a.a: IO[UInt] and Example.a_wire.a: Wire[UInt]
[error] a :<>= a_wire

mwachs5 avatar Nov 27 '23 17:11 mwachs5

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: azidar / name: Adam Izraelevitz (c8171fe12d55d491d20825a904b372b63b079e00, 62a2a4ccd2a4546ce58e1a752b5eb2fde6a1b4d3, 5397547403864701ce14a036ec4684a2304e409c)

What is the story supposed to be for the enduser?

This kind of feature is technically good. But for this kind of change, what are we going to ask people to do with this new feature? When the designer "needs to add a port", are we advocating the use of Outgoing and Incoming over Input and Output?

mmaloney-sf avatar Nov 30 '23 21:11 mmaloney-sf

This kind of feature is technically good. But for this kind of change, what are we going to ask people to do with this new feature? When the designer "needs to add a port", are we advocating the use of Outgoing and Incoming over Input and Output?

IO(...) and IO(Flipped(...)) would be replaced with Outgoing(...) and Incoming(...)

The enduser doesn't write Input or Output today to make a port. they have to write IO(...) and through some complicated relations to whatever is in the ... they get an input or output. In an ideal world we could repurpose the terms Input and Output for this and completely eliminate them from their current usage which is more like "Flip-and-or-Strip" and doesn't really mean Input or Output until you couple it with IO.

mwachs5 avatar Nov 30 '23 21:11 mwachs5