Skip to content

Commit

Permalink
Warn user that using Seq for hardware construction in Bundle is not s…
Browse files Browse the repository at this point in the history
…upported
  • Loading branch information
edwardcwang committed Jul 13, 2018
1 parent 4853c67 commit 6de6733
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 7 deletions.
36 changes: 29 additions & 7 deletions chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -539,18 +539,40 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
val nameMap = LinkedHashMap[String, Data]()
val seen = HashSet[Data]()
for (m <- getPublicFields(classOf[Bundle])) {
getBundleField(m) foreach { d =>
if (nameMap contains m.getName) {
require(nameMap(m.getName) eq d)
} else if (!seen(d)) {
nameMap(m.getName) = d
seen += d
}
getBundleField(m) match {
case Some(d: Data) =>
if (nameMap contains m.getName) {
require(nameMap(m.getName) eq d)
} else if (!seen(d)) {
nameMap(m.getName) = d
seen += d
}
case None =>
if (!ignoreSeq) {
m.invoke(this) match {
case s: scala.collection.Seq[Any] if s.nonEmpty => s.head match {
// Ignore empty Seq()
case d: Data => throwException("Public Seq members cannot be used to define Bundle elements " +
s"(found public Seq member '${m.getName}'). " +
"Either use a Vec() if all elements are of the same type, or HeterogeneousVec() if the elements " +
"are of different types. If this Seq member is not intended to construct RTL, override ignoreSeq " +
"to be true.")
case _ => // don't care about non-Data Seq
}
case _ => // not a Seq
}
}
}
}
ListMap(nameMap.toSeq sortWith { case ((an, a), (bn, b)) => (a._id > b._id) || ((a eq b) && (an > bn)) }: _*)
}

/**
* Override this to be true to avoid raising an error/exception when a Seq is a public member of the bundle.
* This is useful if you have public Seq fields in the Bundle that are unrelated to hardware construction.
*/
def ignoreSeq: Boolean = false

/** Returns a field's contained user-defined Bundle element if it appears to
* be one, otherwise returns None.
*/
Expand Down
48 changes: 48 additions & 0 deletions src/test/scala/chiselTests/BundleSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ trait BundleSpecUtils {
override def cloneType = (new BundleBar).asInstanceOf[this.type]
}

class BadSeqBundle extends Bundle {
val bar = Seq(UInt(16.W), UInt(8.W), UInt(4.W))
override def cloneType = (new BadSeqBundle).asInstanceOf[this.type]
}

class MyModule(output: Bundle, input: Bundle) extends Module {
val io = IO(new Bundle {
val in = Input(input)
Expand Down Expand Up @@ -68,4 +73,47 @@ class BundleSpec extends ChiselFlatSpec with BundleSpecUtils {
elaborate { new MyModule(new BundleFoo, new BundleFooBar) }
}).getMessage should include ("Left Record missing field")
}

"Bundles" should "not be able to use Seq for constructing hardware" in {
(the[ChiselException] thrownBy {
elaborate {
new Module {
val io = IO(new Bundle {
val b = new BadSeqBundle
})
}
}
}).getMessage should include("Public Seq members cannot be used to define Bundle elements")
}

"Bundles" should "be allowed to have Seq if need be" in {
assertTesterPasses {
new BasicTester {
val m = Module(new Module {
val io = IO(new Bundle {
val b = new BadSeqBundle {
override def ignoreSeq = true
}
})
})
stop()
}
}
}

"Bundles" should "be allowed to have non-Chisel Seqs" in {
assertTesterPasses {
new BasicTester {
val m = Module(new Module {
val io = IO(new Bundle {
val f = Output(UInt(8.W))
val unrelated = (0 to 10).toSeq
val unrelated2 = Seq("Hello", "World", "Chisel")
})
io.f := 0.U
})
stop()
}
}
}
}

0 comments on commit 6de6733

Please sign in to comment.