Merge pull request #275 from ucb-bar/fix-printable

Fix bug in Printable FullName of submodule port

3 files changed, 42 insertions, 15 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Printable.scala b/chiselFrontend/src/main/scala/chisel3/core/Printable.scala
index 13d5d74e..f6e63936 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Printable.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Printable.scala
@@ -15,6 +15,9 @@ import java.util.{
 /** Superclass of things that can be printed in the resulting circuit
   *
   * Usually created using the custom string interpolator p"..."
+  * TODO Add support for names of Modules
+  *   Currently impossible because unpack is called before the name is selected
+  *   Could be implemented by adding a new format specifier to Firrtl (eg. %m)
   * TODO Should we provide more functions like map and mkPrintable?
   */
 sealed abstract class Printable {
@@ -22,7 +25,7 @@ sealed abstract class Printable {
     * @note This must be called after elaboration when Chisel nodes actually
     *   have names
     */
-  def unpack: (String, Iterable[String])
+  def unpack(ctx: Component): (String, Iterable[String])
   /** Allow for appending Printables like Strings */
   final def +(that: Printable) = Printables(List(this, that))
   /** Allow for appending Strings to Printables */
@@ -87,22 +90,21 @@ object Printable {
 
 case class Printables(pables: Iterable[Printable]) extends Printable {
   require(pables.hasDefiniteSize, "Infinite-sized iterables are not supported!")
-  final def unpack: (String, Iterable[String]) = {
-    val (fmts, args) = pables.map(_.unpack).unzip
+  final def unpack(ctx: Component): (String, Iterable[String]) = {
+    val (fmts, args) = pables.map(_ unpack ctx).unzip
     (fmts.mkString, args.flatten)
   }
 }
 /** Wrapper for printing Scala Strings */
 case class PString(str: String) extends Printable {
-  final def unpack: (String, Iterable[String]) =
+  final def unpack(ctx: Component): (String, Iterable[String]) =
     (str replaceAll ("%", "%%"), List.empty)
 }
 /** Superclass for Firrtl format specifiers for Bits */
 sealed abstract class FirrtlFormat(specifier: Char) extends Printable {
   def bits: Bits
-  def unpack: (String, Iterable[String]) = {
-    val id = if (bits.isLit) bits.ref.name else bits.instanceName
-    (s"%$specifier", List(id))
+  def unpack(ctx: Component): (String, Iterable[String]) = {
+    (s"%$specifier", List(bits.ref.fullName(ctx)))
   }
 }
 object FirrtlFormat {
@@ -138,13 +140,13 @@ case class Binary(bits: Bits) extends FirrtlFormat('b')
 case class Character(bits: Bits) extends FirrtlFormat('c')
 /** Put innermost name (eg. field of bundle) */
 case class Name(data: Data) extends Printable {
-  final def unpack: (String, Iterable[String]) = (data.ref.name, List.empty)
+  final def unpack(ctx: Component): (String, Iterable[String]) = (data.ref.name, List.empty)
 }
 /** Put full name within parent namespace (eg. bundleName.field) */
-case class FullName(hasId: HasId) extends Printable {
-  final def unpack: (String, Iterable[String]) = (hasId.instanceName, List.empty)
+case class FullName(data: Data) extends Printable {
+  final def unpack(ctx: Component): (String, Iterable[String]) = (data.ref.fullName(ctx), List.empty)
 }
 /** Represents escaped percents */
 case object Percent extends Printable {
-  final def unpack: (String, Iterable[String]) = ("%%", List.empty)
+  final def unpack(ctx: Component): (String, Iterable[String]) = ("%%", List.empty)
 }
diff --git a/src/main/scala/chisel3/internal/firrtl/Emitter.scala b/src/main/scala/chisel3/internal/firrtl/Emitter.scala
index 8ace27f9..8849077d 100644
--- a/src/main/scala/chisel3/internal/firrtl/Emitter.scala
+++ b/src/main/scala/chisel3/internal/firrtl/Emitter.scala
@@ -26,7 +26,7 @@ private class Emitter(circuit: Circuit) {
       case e: BulkConnect => s"${e.loc1.fullName(ctx)} <- ${e.loc2.fullName(ctx)}"
       case e: Stop => s"stop(${e.clk.fullName(ctx)}, UInt<1>(1), ${e.ret})"
       case e: Printf =>
-        val (fmt, args) = e.pable.unpack
+        val (fmt, args) = e.pable.unpack(ctx)
         val printfArgs = Seq(e.clk.fullName(ctx), "UInt<1>(1)",
           "\"" + printf.format(fmt) + "\"") ++ args
         printfArgs mkString ("printf(", ", ", ")")
diff --git a/src/test/scala/chiselTests/PrintableSpec.scala b/src/test/scala/chiselTests/PrintableSpec.scala
index a2c8c62a..12564a40 100644
--- a/src/test/scala/chiselTests/PrintableSpec.scala
+++ b/src/test/scala/chiselTests/PrintableSpec.scala
@@ -97,7 +97,14 @@ class PrintableSpec extends FlatSpec with Matchers {
       case e => fail()
     }
   }
-  it should "support names of circuit elements and the current module" in {
+  it should "support names of circuit elements including submodule IO" in {
+    // Submodule IO is a subtle issue because the Chisel element has a different
+    // parent module
+    class MySubModule extends Module {
+      val io = new Bundle {
+        val fizz = UInt(width = 32)
+      }
+    }
     class MyBundle extends Bundle {
       val foo = UInt(width = 32)
       override def cloneType = (new MyBundle).asInstanceOf[this.type]
@@ -105,15 +112,33 @@ class PrintableSpec extends FlatSpec with Matchers {
     class MyModule extends BasicTester {
       override def desiredName = "MyModule"
       val myWire = Wire(new MyBundle)
+      val myInst = Module(new MySubModule)
       printf(p"${Name(myWire.foo)}")
       printf(p"${FullName(myWire.foo)}")
-      printf(p"${FullName(this)}")
+      printf(p"${FullName(myInst.io.fizz)}")
     }
     val firrtl = Driver.emit(() => new MyModule)
+    println(firrtl)
     getPrintfs(firrtl) match {
       case Seq(Printf("foo", Seq()),
                Printf("myWire.foo", Seq()),
-               Printf("MyModule", Seq())) =>
+               Printf("myInst.io.fizz", Seq())) =>
+      case e => fail()
+    }
+  }
+  it should "handle printing ports of submodules" in {
+    class MySubModule extends Module {
+      val io = new Bundle {
+        val fizz = UInt(width = 32)
+      }
+    }
+    class MyModule extends BasicTester {
+      val myInst = Module(new MySubModule)
+      printf(p"${myInst.io.fizz}")
+    }
+    val firrtl = Driver.emit(() => new MyModule)
+    getPrintfs(firrtl) match {
+      case Seq(Printf("%d", Seq("myInst.io.fizz"))) =>
       case e => fail()
     }
   }