[proposal] Bundle literals implementation

[ducky64]

Potential implementation for Bundle literals, that doesn't use macro annotations and has passable syntax.

Example syntax:

(new MyBundle).Lit(_.a -> 42.U, _.b -> true.B)

For nested Bundles:

// specify an inner bundle with another bundle lit
(new MyOuterBundle).Lit(
  _.a -> (new MyBundle).Lit(_.a -> 42.U, _.b -> true.B)
)

// specify inner bundle fields directly
(new MyOuterBundle).Lit(
  _.a.a -> 42.U, _.a.b -> true.B,
  _.b.c -> false.B, _.b.d -> 255.U
)

and more weirdly for anonymous inner Bundles (so probably not recommended syntax?):

(new MyOuterBundle).Lit(
  b => b.b -> b.b.Lit(_.c -> false.B, _.d -> 255.U)
)

The Lit constructor takes in a list of pairs (->) from fields to values, each each element being a function from a self-type object (the clone is used internally) to a field. This gives reasonable syntax (basically you have the additional _. prefix over each field over what we could do with macro annotations) while being able to compile-time typecheck field accesses (so if you renamed fields, the typechecker would complain about trying to access an nonexistent field).

Drawbacks compared to a macro annotation approach:

• the value type can't be checked at compile time, it accepts any value of type Data. With macro annotations, we can (maybe?) generate a function signature based off the field type. Note that compile-time Chisel type checks are not perfect, the dynamic typeEquivalent is much more accurate.
• you need the additional magic _. prefix for each field.
• _ in anonymous functions is a somewhat-flakey operator. This seems to work though. Originally, Lit has the argument signature (this.type => Data, Data)* but the _ doesn't work there.
• technically you could use the _ operator in the value part (since the argument signature is (this.type => (Data, Data))*), but that will exception out inside the Lit constructor, since the Lit constructor calls the function on the newly cloned object (which obviously can't be referenced before).

But not needing a macro annotation (and requiring annotated Bundles, or the dev time to develop the annotation that we haven't had yet) is a nice property...

If we like this API, these improvements are needed:

• more test cases, particularly for error cases
• refactoring of the Lit constructor, while it's not terrible right not it's also not great
• perhaps thoughts on supporting Vec literals, or also Bundle wire initializers ([RFC] Bundle and Vec .Lit and .Wire #1005) - but might be a follow-on PR that might need a deeper refactoring

---

Related issue: Resolves #805

Type of change: other enhancement

Impact: API addition (no impact on existing code)

Development Phase: proposal

Release Notes

[chick]

This seems like a pretty reasonable proposal to me. I think getting them out there with a minimum of boilerplate required of the Bundle definer makes the _. a small price to pay. Macro's are nice but make for tough learning of internals. I'd say we should put this on the Friday agenda.

[edwardcwang]

Can this work together/is this interoperable with a future @bundle annotation?

[ducky64]

@edwardcwang potentially: this expects a varargs function of Data -> Data, and so it should have a different type signature than what we would generate with a @bundle annotation, even if it's also named Lit (though we'd want to be really careful to see if there could be potential nasty interactions).

That being said, I think if (and that's a reasonably big if) this goes through, the need for a @bundle annotation becomes much less clear. It's a lot of development effort for a small syntactic gain, especially since we already have autoclonetype. The main argument against this PR would be that it's really a bit of a hack, but Chisel has its fair share of syntactic hacks (eg .elsewhen, switch, the Module(...) wrapper).

[ducky64]

Resolution from today's meeting was to document partial-initialization and DontCare semantics, and productionize this. The syntax seemed fine, and this is much preferable to the current way of constructing Bundle literals by defining a very low-level Lit function.

There was also discussion on naming (Lit vs lit) and invocation structure (this is very different than existing literal constructors, eg 0.U - though those may be more converters more like mySeq.toList).

[ducky64]

This API appears to work in scastie

class BundleLitTest {
  val fieldA = 0
  val fieldB = 1
  
  def Lit(elems: (this.type => (Int, Int))*): this.type = {
    for (elem <- elems) {
      println(elem(this))
    }
    this
  }
}

val a = new BundleLitTest
a.Lit(_.fieldA -> 10, _.fieldB -> 11)

(0,10)
(1,11)

[chick]

Tested with 2.13 and dotty
scastie.scala-lang.org

[ducky64]

Ok, I've cleaned up the code and add test cases. Please review now.

Some things that might need more discussion:

• What should isLit do for Bundle literals? It currently returns false, partially for backwards compatibility because Bundle literals behave differently than other literals. Should there be a different recommended API?
• Can we have a typeEquivalent definition that the Bundle literal constructor can use to check field assignment? It works for aggregate types. but not for Bits types since the check compares width too.
• Currently, partially initialized Bundle literals have uninitialized fields set to DontCare, and if assigned to another wire, any value on the wire would be overwritten with DontCare. (side note: is there a way to write a test case that a wire is actually DontCare?)
• What should the recommended syntax be for initializing sub-bundles? Deep field specification (potentially breaks abstraction)? Sub-bundle specification (keeps abstraction, but syntax for anonymous inner bundle literals is weird)? Should we support both? Or just one?

Future work would include:

• Vec literals, which also needs discussion on system architecture and API.

[aswaterman]

My recommendation is that isLit = elements.forall(_.isLit)--yes, technically an API change, so should consult users. Same for eventual Vec lits.

The current DontCare scheme sounds like the right way to go. Assigning a literal to another wire/reg should assign the entire thing--even if some fields are don't-cares.

[chick]

This looks good to me.
Fix compile problem and go

[ducky64]

retest this please

[ducky64]

Not sure what's going on with the compile error, it compiles fine for me and I don't see any new style violations. Potentially a transient bug in the build system, or at least that's what the Jenkins log indicates.

[ucbjrl]

We're adding some new machines to Jenkins slave network and some of the jobs impose incorrect constraints on their environment. This should be sorted out in the next day or two.

[chick]

retest this please

[chick]

@ducky64 Looks like this finally made it through the testing. Time to get it merged

[ducky64]

One additional thing we can do is to require an experimental import, eg import chisel3.experimental.BundleLiteralConstructor._ as we've done with the recent testers2 experimental features. Thoughts?

[ducky64]

Changed to:

• Require import chisel3.experimental.BundleLiteralConstructor._ to access Bundle .Lit, with the Aggregate class literal constructor changed to private[chisel3] def _makeLit (it relies on protected features like bind so I can't move the entire thing into an implicit class).
• Remove Data.selfBind, which was originally used to support DIY Bundle Literals. Any existing DIY Bundle Literal code will break, but that was never really supported anyways, and I really hope no one else is using the selfBind API.

[jackkoenig]

Suggestion to change chisel3.experimental.BundleLiteralConstructor to chisel3.experimental.BundleLiteral or chisel3.experimental.BundleLiterals

thoughts?

[edwardcwang]

I like the idea of chisel3.experimental.BundleLiterals