Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mwachs5 opened this issue Jan 30, 2020 · 5 comments
Open

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

mwachs5 opened this issue Jan 30, 2020 · 5 comments
Assignees

Comments

@mwachs5
Copy link

mwachs5 commented Jan 30, 2020

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
Copy link
Member

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
Copy link
Author

mwachs5 commented Jan 30, 2020

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)

@mwachs5 mwachs5 changed the title Port rocket-chips "FixChisel3" operators Port rocket-chips :<> :<= :=> operators Jan 30, 2020
@mwachs5 mwachs5 changed the title Port rocket-chips :<> :<= :=> operators Port rocket-chips :<>, :<=, :=> operators Jan 31, 2020
@terpstra
Copy link

terpstra commented Feb 1, 2020

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
Copy link

terpstra commented Feb 2, 2020

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
Copy link

seldridge commented Mar 16, 2020

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.

@seldridge seldridge transferred this issue from chipsalliance/chisel Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants