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

Can't have a Vec(new Bundle{}) ? #1953

Closed
mwachs5 opened this issue Jun 7, 2021 · 4 comments · Fixed by #2543
Closed

Can't have a Vec(new Bundle{}) ? #1953

mwachs5 opened this issue Jun 7, 2021 · 4 comments · Fixed by #2543

Comments

@mwachs5
Copy link
Contributor

mwachs5 commented Jun 7, 2021

Type of issue: bug report

Impact: unknown

Development Phase: request

Other information

If the current behavior is a bug, please provide the steps to reproduce the problem:
See this Scastie:
https://scastie.scala-lang.org/2qc4i466TRS9gvWIAnLmRQ

Note this is using Chisel 3.4.3 with the naming plugin enabled (same behavior seen without plugin enabled: https://scastie.scala-lang.org/XlNASbYNT6WDa641zN9kNQ)

Same code here:

import chisel3._
import chisel3.stage.ChiselStage

class Example extends MultiIOModule {
  val in = IO(Input(Bool()))
  val out= IO(Output(Bool()))

  val thisWorks = Reg(new Bundle{})
  val thisDoesNotWork = Reg(Vec(4, new Bundle{})) //I get a None.get error message
    
  out := in
}

What is the current behavior?

If I try to build a Vec(x, Bundle{}) I get a None.get error.

What is the expected behavior?

Either I get a good error message about something I am doing wrong (unclear whether Vec-of-empty-bundle is allowed, when a Reg-of-empty-bundle works fine), or it works fine and I get registers-of-empty-bundles.

Please tell us about your environment:

Chisel 3.4.3, with naming plugin enabled
What is the use case for changing the behavior?

It is nice to be able to build up Bundles of optional fields or sequential fields which might end up that the Bundle itself has nothing in it and have things just work without special casing

@mwachs5
Copy link
Contributor Author

mwachs5 commented Jun 7, 2021

This error is stemming from:

https://github.com/chipsalliance/chisel3/blob/master/core/src/main/scala/chisel3/Aggregate.scala#L235

    // Since all children are the same, we can just use the sample_element rather than all children    
    // .get is safe because None means mixed directions, we only pass 1 so that's not possible 
 direction = ActualDirection.fromChildren(Set(sample_element.direction), resolvedDirection).get

@mwachs5
Copy link
Contributor Author

mwachs5 commented Jun 7, 2021

rewriting the code to make the direction clear works to have an empty bundle in a Vec:
https://scastie.scala-lang.org/OlWvps2cRRa19sFOjdzUsg

@schoeberl
Copy link
Contributor

What is the meaning of a Reg or Vec of an empty Bundle. Maybe Reg((new Bundle{}) should err.

@jackkoenig
Copy link
Contributor

Example showing this in v3.4.4: https://scastie.scala-lang.org/Up44My1rRiOkIlyWjpoMLw

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

Successfully merging a pull request may close this issue.

3 participants