summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--core/src/main/scala/chisel3/Aggregate.scala22
-rw-r--r--core/src/main/scala/chisel3/BlackBox.scala3
-rw-r--r--core/src/main/scala/chisel3/Data.scala6
-rw-r--r--core/src/main/scala/chisel3/internal/Builder.scala20
-rw-r--r--plugin/src/main/scala-2.12/chisel3/internal/plugin/ChiselPlugin.scala9
-rw-r--r--src/test/scala/chiselTests/Mem.scala53
-rw-r--r--src/test/scala/chiselTests/naming/PrefixSpec.scala4
7 files changed, 95 insertions, 22 deletions
diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala
index 9ccf7dbb..6c11b2db 100644
--- a/core/src/main/scala/chisel3/Aggregate.scala
+++ b/core/src/main/scala/chisel3/Aggregate.scala
@@ -479,9 +479,21 @@ trait VecLike[T <: Data] extends collection.IndexedSeq[T] with HasId with Source
* RTL writers should use [[Bundle]]. See [[Record#elements]] for an example.
*/
abstract class Record(private[chisel3] implicit val compileOptions: CompileOptions) extends Aggregate {
+
+ // Doing this earlier than onModuleClose allows field names to be available for prefixing the names
+ // of hardware created when connecting to one of these elements
+ private def setElementRefs(): Unit = {
+ // Since elements is a map, it is impossible for two elements to have the same
+ // identifier; however, Namespace sanitizes identifiers to make them legal for Firrtl/Verilog
+ // which can cause collisions
+ val _namespace = Namespace.empty
+ for ((name, elt) <- elements) { elt.setRef(this, _namespace.name(name, leadingDigitOk=true)) }
+ }
+
private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
try {
super.bind(target, parentDirection)
+ setElementRefs()
} catch { // nasty compatibility mode shim, where anything flies
case e: MixedDirectionAggregateException if !compileOptions.dontAssumeDirectionality =>
val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection)
@@ -633,13 +645,11 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
case _ => false
}
- // NOTE: This sets up dependent references, it can be done before closing the Module
private[chisel3] override def _onModuleClose: Unit = {
- // Since Bundle names this via reflection, it is impossible for two elements to have the same
- // identifier; however, Namespace sanitizes identifiers to make them legal for Firrtl/Verilog
- // which can cause collisions
- val _namespace = Namespace.empty
- for ((name, elt) <- elements) { elt.setRef(this, _namespace.name(name, leadingDigitOk=true)) }
+ // This is usually done during binding, but these must still be set for unbound Records
+ if (this.binding.isEmpty) {
+ setElementRefs()
+ }
}
private[chisel3] final def allElements: Seq[Element] = elements.toIndexedSeq.flatMap(_._2.allElements)
diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala
index b204e301..1b093ee1 100644
--- a/core/src/main/scala/chisel3/BlackBox.scala
+++ b/core/src/main/scala/chisel3/BlackBox.scala
@@ -165,7 +165,8 @@ abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param
// Long term solution will be to define BlackBox IO differently as part of
// it not descending from the (current) Module
for ((name, port) <- namedPorts) {
- port.setRef(ModuleIO(this, _namespace.name(name)))
+ // We have to force override the _ref because it was set during IO binding
+ port.setRef(ModuleIO(this, _namespace.name(name)), force = true)
}
// We need to call forceName and onModuleClose on all of the sub-elements
diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala
index bb45b4d8..5d398aa6 100644
--- a/core/src/main/scala/chisel3/Data.scala
+++ b/core/src/main/scala/chisel3/Data.scala
@@ -9,6 +9,8 @@ import chisel3.internal._
import chisel3.internal.firrtl._
import chisel3.internal.sourceinfo.{DeprecatedSourceInfo, SourceInfo, SourceInfoTransform, UnlocatableSourceInfo}
+import scala.util.Try
+
/** User-specified directions.
*/
sealed abstract class SpecifiedDirection
@@ -374,7 +376,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
// Provides a unhelpful fallback for literals, which should have custom rendering per
// Data-subtype.
// TODO Is this okay for sample_element? It *shouldn't* be visible to users
- protected def bindingToString: String = topBindingOpt match {
+ protected def bindingToString: String = Try(topBindingOpt match {
case None => ""
case Some(OpBinding(enclosure, _)) => s"(OpResult in ${enclosure.desiredName})"
case Some(MemoryPortBinding(enclosure, _)) => s"(MemPort in ${enclosure.desiredName})"
@@ -389,7 +391,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
case Some(DontCareBinding()) => s"(DontCare)"
case Some(ElementLitBinding(litArg)) => s"(unhandled literal)"
case Some(BundleLitBinding(litMap)) => s"(unhandled bundle literal)"
- }
+ }).getOrElse("")
// Return ALL elements at root of this type.
// Contasts with flatten, which returns just Bits
diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index ffa99913..3988ac68 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -198,7 +198,10 @@ private[chisel3] trait HasId extends InstanceId {
}
private var _ref: Option[Arg] = None
- private[chisel3] def setRef(imm: Arg): Unit = _ref = Some(imm)
+ private[chisel3] def setRef(imm: Arg, force: Boolean = false): Unit = {
+ assert(force || _ref.isEmpty, s"Internal Error, setRef for $this called twice! first ${_ref.get}, second $imm")
+ _ref = Some(imm)
+ }
private[chisel3] def setRef(parent: HasId, name: String): Unit = setRef(Slot(Node(parent), name))
private[chisel3] def setRef(parent: HasId, index: Int): Unit = setRef(Index(Node(parent), ILit(index)))
private[chisel3] def setRef(parent: HasId, index: UInt): Unit = setRef(Index(Node(parent), index.ref))
@@ -369,13 +372,12 @@ private[chisel3] object Builder {
*/
def pushPrefix(d: HasId): Boolean = {
def buildAggName(id: HasId): Option[String] = {
- // TODO This is slow, can we store this information upon binding?
- def getSubName(field: Data, parent: Data): Option[String] = parent match {
- case vec: Vec[_] =>
- val idx = vec.indexOf(field)
- if (idx >= 0) Some(idx.toString) else None
- case rec: Record => rec.elements.collectFirst { case (name, d) if d == field => name }
- case _ => Builder.exception(s"Shouldn't see non-Aggregate $parent as parent in ChildBinding!")
+ def getSubName(field: Data): Option[String] = field.getOptionRef.flatMap {
+ case Slot(_, field) => Some(field) // Record
+ case Index(_, ILit(n)) => Some(n.toString) // Vec static indexing
+ case Index(_, ULit(n, _)) => Some(n.toString) // Vec lit indexing
+ case Index(_, _: Node) => None // Vec dynamic indexing
+ case ModuleIO(_, n) => Some(n) // BlackBox port
}
def map2[A, B](a: Option[A], b: Option[A])(f: (A, A) => B): Option[B] =
a.flatMap(ax => b.map(f(ax, _)))
@@ -384,7 +386,7 @@ private[chisel3] object Builder {
case (_: WireBinding | _: RegBinding | _: MemoryPortBinding | _: OpBinding) => data.seedOpt
case ChildBinding(parent) => recData(parent).map { p =>
// And name of the field if we have one, we don't for dynamic indexing of Vecs
- getSubName(data, parent).map(p + "_" + _).getOrElse(p)
+ getSubName(data).map(p + "_" + _).getOrElse(p)
}
case SampleElementBinding(parent) => recData(parent)
case PortBinding(mod) if Builder.currentModule.contains(mod) => data.seedOpt
diff --git a/plugin/src/main/scala-2.12/chisel3/internal/plugin/ChiselPlugin.scala b/plugin/src/main/scala-2.12/chisel3/internal/plugin/ChiselPlugin.scala
index 6e4f3ece..9f90ba16 100644
--- a/plugin/src/main/scala-2.12/chisel3/internal/plugin/ChiselPlugin.scala
+++ b/plugin/src/main/scala-2.12/chisel3/internal/plugin/ChiselPlugin.scala
@@ -124,27 +124,30 @@ class ChiselComponent(val global: Global) extends PluginComponent with TypingTra
dd.symbol.logicallyEnclosingMember.thisType <:< inferType(tq"chisel3.Bundle")
}
+ private def stringFromTermName(name: TermName): String =
+ name.toString.trim() // Remove trailing space (Scalac implementation detail)
+
// Method called by the compiler to modify source tree
override def transform(tree: Tree): Tree = tree match {
// Check if a subtree is a candidate
case dd @ ValDef(mods, name, tpt, rhs) if okVal(dd) =>
// If a Data and in a Bundle, just get the name but not a prefix
if (shouldMatchData(dd) && inBundle(dd)) {
- val TermName(str: String) = name
+ val str = stringFromTermName(name)
val newRHS = transform(rhs) // chisel3.internal.plugin.autoNameRecursively
val named = q"chisel3.internal.plugin.autoNameRecursively($str, $newRHS)"
treeCopy.ValDef(dd, mods, name, tpt, localTyper typed named)
}
// If a Data or a Memory, get the name and a prefix
else if (shouldMatchDataOrMem(dd)) {
- val TermName(str: String) = name
+ val str = stringFromTermName(name)
val newRHS = transform(rhs)
val prefixed = q"chisel3.experimental.prefix.apply[$tpt](name=$str)(f=$newRHS)"
val named = q"chisel3.internal.plugin.autoNameRecursively($str, $prefixed)"
treeCopy.ValDef(dd, mods, name, tpt, localTyper typed named)
// If an instance, just get a name but no prefix
} else if (shouldMatchModule(dd)) {
- val TermName(str: String) = name
+ val str = stringFromTermName(name)
val newRHS = transform(rhs)
val named = q"chisel3.internal.plugin.autoNameRecursively($str, $newRHS)"
treeCopy.ValDef(dd, mods, name, tpt, localTyper typed named)
diff --git a/src/test/scala/chiselTests/Mem.scala b/src/test/scala/chiselTests/Mem.scala
index b66ec42f..8bcd3aac 100644
--- a/src/test/scala/chiselTests/Mem.scala
+++ b/src/test/scala/chiselTests/Mem.scala
@@ -96,6 +96,51 @@ class HugeCMemTester(size: BigInt) extends BasicTester {
}
}
+class SyncReadMemBundleTester extends BasicTester {
+ val (cnt, _) = Counter(true.B, 5)
+ val tpe = new Bundle {
+ val foo = UInt(2.W)
+ }
+ val mem = SyncReadMem(2, tpe)
+ val rdata = mem.read(cnt - 1.U, cnt =/= 0.U)
+
+ switch (cnt) {
+ is (0.U) {
+ val w = Wire(tpe)
+ w.foo := 3.U
+ mem.write(cnt, w)
+ }
+ is (1.U) {
+ val w = Wire(tpe)
+ w.foo := 2.U
+ mem.write(cnt, w)
+ }
+ is (2.U) { assert(rdata.foo === 3.U) }
+ is (3.U) { assert(rdata.foo === 2.U) }
+ is (4.U) { stop() }
+ }
+}
+
+class MemBundleTester extends BasicTester {
+ val tpe = new Bundle {
+ val foo = UInt(2.W)
+ }
+ val mem = Mem(2, tpe)
+
+ // Circuit style tester is definitely the wrong abstraction here
+ val (cnt, wrap) = Counter(true.B, 2)
+ mem(0) := {
+ val w = Wire(tpe)
+ w.foo := 1.U
+ w
+ }
+
+ when (cnt === 1.U) {
+ assert(mem.read(0.U).foo === 1.U)
+ stop()
+ }
+}
+
class MemorySpec extends ChiselPropSpec {
property("Mem of Vec should work") {
assertTesterPasses { new MemVecTester }
@@ -105,6 +150,14 @@ class MemorySpec extends ChiselPropSpec {
assertTesterPasses { new SyncReadMemTester }
}
+ property("SyncReadMems of Bundles should work") {
+ assertTesterPasses { new SyncReadMemBundleTester }
+ }
+
+ property("Mems of Bundles should work") {
+ assertTesterPasses { new MemBundleTester }
+ }
+
property("SyncReadMem write collision behaviors should work") {
assertTesterPasses { new SyncReadMemWriteCollisionTester }
}
diff --git a/src/test/scala/chiselTests/naming/PrefixSpec.scala b/src/test/scala/chiselTests/naming/PrefixSpec.scala
index 27a9fd39..83408dea 100644
--- a/src/test/scala/chiselTests/naming/PrefixSpec.scala
+++ b/src/test/scala/chiselTests/naming/PrefixSpec.scala
@@ -285,6 +285,7 @@ class PrefixSpec extends ChiselPropSpec with Utils {
wire.y := RegNext(3.U)
wire.vec(0) := RegNext(3.U)
wire.vec(wire.x) := RegNext(3.U)
+ wire.vec(1.U) := RegNext(3.U)
}
}
aspectTest(() => new Test) {
@@ -293,7 +294,8 @@ class PrefixSpec extends ChiselPropSpec with Utils {
"wire_x_REG",
"wire_y_REG",
"wire_vec_0_REG",
- "wire_vec_REG"
+ "wire_vec_REG",
+ "wire_vec_1_REG"
))
}
}