MixedVec Doesn't Work with SuggestName

[seldridge]

As reported by @oharboe, a MixedVec doesn't seem to work when using the .suggestName to change the name of the numeric identifiers created by the MixedVec.

Consider:

import chisel3._
import chisel3.util.MixedVec
import circt.stage.ChiselStage

class Foo extends Module {
  val a = IO(Input(MixedVec(Bool())))

  a(0).suggestName("a")
}

println(ChiselStage.emitCHIRRTL(new Foo))

This produces the following CHIRRTL:

circuit Foo :
  module Foo :
    input clock : Clock
    input reset : UInt<1>
    input a : { 0 : UInt<1>} @[src/main/scala/main.scala 8:13]

    skip

I'd expect to see:

    input a : { a : UInt<1>}

[jackkoenig]

This isn't a bug, the purpose of MixedVec isn't to let you name the fields, it's just that you want something Vec-like that allows different types. It is also not possible in Chisel (and probably shouldn't be) to use .suggestName to influence the name of any element of any Aggregate (and it is a bug that we don't error when you try).

That being said, the desire to have programmatic naming control over ports in an Aggregate makes sense. This is something you can do with DataView, or with FlatIO + a Record where you set the names of the elements. It really depends on the specific use case though so if we can get the use cases, we can add them to the Cookbook (and there might be related utilities we should add).

[jackkoenig]

Perhaps an alternative phrasing might be that MixedVec (or something similar) should let you provide names when defining the type.

[oharboe]

This isn't a bug, the purpose of MixedVec isn't to let you name the fields, it's just that you want something Vec-like that allows different types. It is also not possible in Chisel (and probably shouldn't be) to use .suggestName to influence the name of any element of any Aggregate (and it is a bug that we don't error when you try).

It isn't possible to name fields of a Vec() either.

That being said, the desire to have programmatic naming control over ports in an Aggregate makes sense. This is something you can do with DataView, or with FlatIO + a Record where you set the names of the elements. It really depends on the specific use case though so if we can get the use cases, we can add them to the Cookbook (and there might be related utilities we should add).

I have a Bundle where I want to suggest names to each of the entries in a Vec() or MixedVec() as the entries in these Mixed/Vec()s correspond to an enum.

E.g. an enum can have up, down, left, right and I want the Vec/MixedVec() entries to have the names up, down, left, right.

Ideally, I could suggest these names as part of the Bundle definition.

[jackkoenig]

I have a Bundle where I want to suggest names to each of the entries in a Vec() or MixedVec() as the entries in these Mixed/Vec()s correspond to an enum.

E.g. an enum can have up, down, left, right and I want the Vec/MixedVec() entries to have the names up, down, left, right.

Ideally, I could suggest these names as part of the Bundle definition.

Why not just put those in a Bundle?

class MyBundle extends Bundle {
  val up = Bool()
  val down = Bool()
  val left = Bool()
  val right = Bool()
}

Is the problem that the set of names you need here are programmatic?

[oharboe]

I have a Bundle where I want to suggest names to each of the entries in a Vec() or MixedVec() as the entries in these Mixed/Vec()s correspond to an enum. E.g. an enum can have up, down, left, right and I want the Vec/MixedVec() entries to have the names up, down, left, right. Ideally, I could suggest these names as part of the Bundle definition.

Why not just put those in a Bundle?

class MyBundle extends Bundle {
  val up = Bool()
  val down = Bool()
  val left = Bool()
  val right = Bool()
}

Is the problem that the set of names you need here programmatic?

Yes.

The elements in this enum that are actually used in the implementation is a configuration matter, so something like:

val foo = Input(Vec(actualenumsused, Bool()))

(enumsfilteredbyused zip foo).foreach{case (a, b) => foo.suggestName(foo.toString())

I actually use the Vec() as a sequence, not really as a hardware lookup. Perhaps I do in some cases... Though: I can't use a Seq in a bundle.

class Foo extends Bundle {
val foo Seq.fill(5)(Bool())
}

[jackkoenig]

Thanks for the clarification!

If I'm understanding correctly, this is the sort of thing that custom Records are for, eg. I can make a Record that sets the name of its fields based on a Seq of Strings you pass:

// Note that in Chisel 3.5.5 and 3.5.6 you'll need to mixin chisel3.experimental.AutoCloneType
class MyEnumRecord(names: Seq[String]) extends Record {
  lazy val elements = SeqMap(names.map(_ -> Bool()): _*)

  // You can then add whatever utility methods you want
  def asSeq: Seq[Bool] = elements.map(_._2).toSeq
}

Here's a complete example using it: https://scastie.scala-lang.org/GkNGoc3TR0y1MpWmRfy5kw

This particular Record may not be exactly what you want, but the point is that you can do whatever you want for the number, names, and types of the fields of a Record.

[oharboe]

@jackkoenig @yupferris @seldridge Thanks! Will try on our codebase when I have a new version of LLVM CIRCT that fixes various other nits. My plan is that I then have everything I need to upgrade to Chisel 3.6.0.

One of the most effective ways I have of dispelling of Chisel FUD is to say show me the Verilog and I will make Chisel that generates it.

To be able to exactly control the module interfaces is the most critical part of this promise.

[oharboe]

I messed around a bit with your example.

https://scastie.scala-lang.org/TCWELv9PTguV4Eipgfttvw

Interesting: suggestName() doesnt work for Record.

I can see scenarious where the naming of the inputs and outputs in Verilog is a separate concern for Verilog and Chisel.

Use case: I add metainformation to Verilog names that I can use in set_io_pin_constraint regex in OpenROAD.

e.g. I may care that some pins are on the left, right, top and bottom of the macro, but the Chisel code is topological in nature, it doesnt care where io pins go.

However, sometimes I can do some IO constraint calculations in Chisel, just because it is a typed statically checked language and a million times more pleasant than .tcl.

[mwachs5]

.suggestName is never supposed to work for the fields of aggregates (Bundle, Record, or Vec).

We should be erroring on that rather than silently letting people try, reported in #2920.