freechipsproject / rfcs

RFCs for changes to Chisel, FIRRTL, and related projects
Apache License 2.0
8 stars 2 forks source link

Port rocket-chips :<>, :<=, :=> operators #11

Open mwachs5 opened 4 years ago

mwachs5 commented 4 years ago

Type of issue: feature request

Impact: API addition (no impact on existing code)

Development Phase: proposal

Other information

If the current behavior is a bug, please provide the steps to reproduce the problem:

Users are suprised and sometimes frustrated that operations like the following don't work (just an example) :

class Bar extends Bundle {
 val bar = Bool()
}

class FooBar extends Bar {
 val foo = Bool()
}

...

class FooBarModule extends Module {
  val io = IO( new Bundle {
    val fooIn = Input(new Foo())
    val fooOut = Output(new Foo())
    val barIn = Input(new Bar())
    val barOut = Output(new Foo())
}

io.fooOut := io.barIn // Doesn't compile because there is no foo defined in bar
io.fooOut.foo := false.B

What is the current behavior?

The above doesn't compile and there is no way to tell it to compile without doing stuff like

fooOut.bar := barIn.bar

What is the expected behavior?

There should be some way to say "I know what I am doing, compile this."

Please tell us about your environment:

What is the use case for changing the behavior?

There are already operators defined here, but it would be nice to standardize and clearly document them:

https://github.com/chipsalliance/rocket-chip/blob/cca6d83d21f1f75c025f23e2dcd83f2acc3d1931/src/main/scala/util/BundleMap.scala#L69

aswaterman commented 4 years ago

Just to be clear, most of us rocket-chip folks don't endorse the incendiary FixChisel3 name, but agree the chisel3 behavior is occasionally quite frustrating in this respect.

mwachs5 commented 4 years ago

Also, I would say that most-of-us-rocket-chip-folks also don't FULLY understand what any of these operators do (nor have I fully internalized and trusted myself to understand what <> and := do in chisel3)

terpstra commented 4 years ago

These operators :<>, :<=, and :=> do NOT allow for connecting bundles with elements whose names do not match. Or, rather, that was not the intent. In fact, I think it might make sense to require the scala types to exactly match on both sides of these operators to prevent messing things up with field names.

terpstra commented 4 years ago

These operators are useful for working with bundles that have wires in both directions. Forget about Input/Output; think about it in terms of Flipped, like firrtl (which does this correctly).

a :<= b // means drive all unflipped fields of 'a' from 'b' (ie: valid/bits) a :=> b // means drive all flipped fields of 'b' from 'a' (ie: ready) a :<> b // do both of the above

In chisel3, the operators := and <> became much less powerful: 'a := b' only works if there are no directions on fields. 'a <> b' only works if one of those is an IO (not a wire).

Contrast this with 'a :<> b' which will connect a read-valid producer 'b' to a consumer 'a'. If you flip this to 'b :<> a', it works the way you would expect (flipping the role of producer/consumer). This is how chisel2-compat and firrtl work. Also, I find ':<>' has superior readability (even if the direction can be inferred from an IO), because it clearly states the intended producer/consumer relationship. Plus, you will get an appropriate error if you connected it the wrong way (usually because you got the IO direction wrong) instead of silently succeeding.

What if you want to connect ready/valid/bits from 'b' to 'a'? ie: you don't care about producer/consumer relationship. Perhaps you are tapping the connection to monitor traffic on an existing connection. In that case you can do 'a :<= b' + 'b :=> a'. In some ways, this combination is a generalization of 'a := b'.

seldridge commented 4 years ago

Dev meeting resolution: this is really an RFC about "what are the 'correct' connection semantics for Chisel"?

There are a number of things that this touches on:

  1. Types of allowable connections: Scala types vs. Structural types
    • There are reasons for this (making it easier to interface external libraries), but are there other solutions like making it known to users that they can write implicit classes to define new connection semantics as opposed to relying on structural equivalence
  2. What FIRRTL operators are actually emitted? <= vs. <-
  3. Where the error is emitted, Chisel3 vs. FIRRTL and how this interacts with the FIRRTL operator (e.g., traversing the bundle for equivalence checking doesn't preclude emitting <-).
  4. Prevention of yet-another connection semantic (Compatibiliy mode, non-compatibility mode, Fix Chisel3, etc.) vs. allowing users to define their connection semantics (which is also totally allowable).
  5. "Getting the connection semantics Right (:tm:)"

There are a number of important stakeholders that have valuable opinions/viewpoints and it would be good to keep them in the loop and solicit their feedback on the RFC: @terpstra, @sdtwigg come to mind.

I'll migrate this and close this issue once an RFC is written. Tentatively, @jackkoenig will start the discussion on the RFC repo.