Cannot use Seq in Bundle construction?

[edwardcwang]

It seems like Chisel just doesn't like it when we use Seq to construct Bundles. Two test cases (which may or may not be caused by the same code paths):

1. Using Seq to construct Input(UInt()):

class MyTest extends Bundle {
    val el = Seq(1, 4, 9).map(i => Input(UInt(i.W)))
    // val el = Seq(Input(UInt(1.W)), Input(UInt(4.W)), Input(UInt(9.W))) // equivalent
    override def cloneType = (new MyTest).asInstanceOf[this.type]
}

class MyTestMod extends Module {
  val io = IO(new Bundle {
    val test = new MyTest()
    val out = Output(UInt(12.W))
  })
  
  io.out := io.test.el(0) + io.test.el(1) + io.test.el(2)
}

Results in Chisel not being able to figure out the direction of MyTest:

chisel3.core.Binding$MixedDirectionAggregateException: Aggregate 'MyTestMod' can't have elements that are both directioned and undirectioned: Vector((chisel3.core.UInt@e,Output), (MyTest@6,Unspecified))

2. Using Seq to construct UInt() and then specifying the direction in the bundle above it (e.g. Input(new MyTest())):

class MyTest extends Bundle {
    val el = Seq(1, 4, 9).map(i => UInt(i.W))
    override def cloneType = (new MyTest).asInstanceOf[this.type]
}

class MyTestMod extends Module {
  val io = IO(new Bundle {
    val test = Input(new MyTest())
    val out = Output(UInt(12.W))
  })
  
  io.out := io.test.el(0) + io.test.el(1) + io.test.el(2)
}

Results in Chisel misclassifying the elements of el as bare Chisel types as opposed to hardware.

chisel3.core.Binding$ExpectedHardwareException: bits operated on 'chisel3.core.UInt@b' must be hardware, not a bare Chisel type

Type of issue: bug report

Impact: no functional change

Development Phase: request

What is the current behavior? See above

What is the expected behavior? The examples should construct hardware and elaborate properly as expected, without any error messages, or Chisel should give the user a clear error message if using Seq to construct Bundles is unsupported.

Please tell us about your environment: Chisel 3.1

What is the use case for changing the behavior? The error message is confusing to users (directions were specified from a user's perspective) and it is not immediately clear why this is causes a problem to begin with.

[jackkoenig]

This is expected behavior, Seqs are Scala types, not Chisel types, so we can't use them to define Chisel Types. When you're defining a Chisel type you need to use Vec, not Seq. In this case it looks like you want to have different widths for the elements of the Seq, so you'll need to use a custom Record type like HeterogeneousBag in rocket-chip.

Since this is not an uncommon thing that people try to do, we should have a more specialized error message.

[edwardcwang]

I see - thanks for the fast response.

Seems like there are a few avenues forward:

1. More specialized error message to help users (as you mentioned)
2. Do you think it would be reasonable to port HeterogeneousBag into chisel3.util and document it with ScalaDoc to help users? Perhaps it could even go in the error message of (1).
3. Seems like we could also rename HeterogeneousBag to HeterogeneousVec since it behaves a bit like a Vec from the user's perspective? It's more consistent for the user as well since I as a user don't have to think/guess about what a "Bag" is.

[edwardcwang]

@jackkoenig If you think this is worthwhile, I can go ahead and do some PRs. :)

[jackkoenig]

More specialized error message is a big one I think, I'm not exactly sure how to do it but using reflection on the Bundle we can look for Seqs in public vals and then upon someone attempting to connect to one we could suggest that as a possible cause?

I think a variant of HeterogeneousBag makes sense in chisel3.util. One subtle issue is that HeterogeneousBag does not call cloneType on the arguments passed to it. A version in chisel3.util definitely needs to call cloneType on its inputs. HeterogeneousVec is better but still a really long name. Perhaps HVec? My only fear with that is it might confuse proper functional programmers in that it might sound similar to HList despite being quite different. Still, probably worth being much less annoying to read.

If you have time, I think these are great things to work on!

[shunshou]

Funny that a HetVec still hasn’t made it into Chisel in like two years :p I also have something similar that works off of Record. It might even be sitting somewhere in barstools

[edwardcwang]

@jackkoenig Great! I PR'ed a proposal along the lines of what we discussed in #850.

[edwardcwang]

Implemented in #850