Merge branch 'modrefactor'

2 files changed, 87 insertions, 18 deletions

diff --git a/src/main/scala/Chisel/internal/firrtl/Emitter.scala b/src/main/scala/Chisel/internal/firrtl/Emitter.scala
index 13d9fa8f..c2391f97 100644
--- a/src/main/scala/Chisel/internal/firrtl/Emitter.scala
+++ b/src/main/scala/Chisel/internal/firrtl/Emitter.scala
@@ -22,7 +22,7 @@ private class Emitter(circuit: Circuit) {
     case e: Printf => s"""printf(${e.clk.fullName(ctx)}, UInt<1>(1), "${e.format}"${e.ids.map(_.fullName(ctx)).fold(""){_ + ", " + _}})"""
     case e: DefInvalid => s"${e.arg.fullName(ctx)} is invalid"
     case e: DefInstance => {
-      val modName = moduleMap.getOrElse(e.id.name, e.id.name)
+      val modName = moduleMap.get(e.id.name).get
       s"inst ${e.name} of $modName"
     }
 
@@ -33,31 +33,59 @@ private class Emitter(circuit: Circuit) {
       unindent()
       "skip"
   }
-  private def emitBody(m: Component) = {
-    val me = new StringBuilder
+
+  // Map of Module FIRRTL definition to FIRRTL name, if it has been emitted already.
+  private val defnMap = collection.mutable.HashMap[String, String]()
+  // Map of Component name to FIRRTL id.
+  private val moduleMap = collection.mutable.HashMap[String, String]()
+
+  /** Generates the FIRRTL module definition with a specified name.
+    */
+  private def moduleDefn(m: Component, name: String): String = {
+    val body = new StringBuilder
+    m.id match {
+      case _: BlackBox => body ++= newline + s"extmodule $name : "
+      case _: Module => body ++= newline + s"module $name : "
+    }
     withIndent {
       for (p <- m.ports)
-        me ++= newline + emitPort(p)
-      me ++= newline
-      for (cmd <- m.commands)
-        me ++= newline + emit(cmd, m)
-      me ++= newline
+        body ++= newline + emitPort(p)
+      body ++= newline
+
+      m.id match {
+        case _: BlackBox =>
+          // TODO: BlackBoxes should be empty, but funkiness in Module() means
+          // it's not for now. Eventually, this should assert out.
+        case _: Module => for (cmd <- m.commands) {
+          body ++= newline + emit(cmd, m)
+        }
+      }
+      body ++= newline
     }
-    me
+    body.toString()
   }
 
-  private val bodyMap = collection.mutable.HashMap[StringBuilder, String]()
-  private val moduleMap = collection.mutable.HashMap[String, String]()
-
+  /** Returns the FIRRTL declaration and body of a module, or nothing if it's a
+    * duplicate of something already emitted (on the basis of simple string
+    * matching).
+    */
   private def emit(m: Component): String = {
-    val body = emitBody(m)
-    bodyMap get body match {
-      case Some(name) =>
-        moduleMap(m.name) = name
+    // Generate the body.
+    val moduleName = m.id.getClass.getName.split('.').last
+    val defn = moduleDefn(m, moduleName)
+
+    defnMap get defn match {
+      case Some(deduplicatedName) =>
+        moduleMap(m.name) = deduplicatedName
         ""
       case None =>
-        bodyMap(body) = m.name
-        newline + s"module ${m.name} : " + body
+        require(!(moduleMap contains m.name),
+            "emitting module with same name but different contents")
+
+        moduleMap(m.name) = m.name
+        defnMap(defn) = m.name
+
+        moduleDefn(m, m.name)
     }
   }
 
diff --git a/src/test/scala/chiselTests/ParameterizedModule.scala b/src/test/scala/chiselTests/ParameterizedModule.scala
new file mode 100644
index 00000000..f356cebd
--- /dev/null
+++ b/src/test/scala/chiselTests/ParameterizedModule.scala
@@ -0,0 +1,41 @@
+// See LICENSE for license details.
+
+package chiselTests
+
+import org.scalatest._
+import Chisel._
+import Chisel.testers.BasicTester
+
+class ParameterizedModule(invert: Boolean) extends Module {
+  val io = new Bundle {
+    val in = new Bool(INPUT)
+    val out = new Bool(OUTPUT)
+  }
+  if (invert) {
+    io.out := !io.in
+  } else {
+    io.out := io.in
+  }
+}
+
+/** A simple test to check Module deduplication doesn't affect correctness (two
+  * modules with the same name but different contents aren't aliased). Doesn't
+  * check that deduplication actually happens, though.
+  */
+class ParameterizedModuleTester() extends BasicTester {
+  val invert = Module(new ParameterizedModule(true))
+  val noninvert = Module(new ParameterizedModule(false))
+
+  invert.io.in := Bool(true)
+  noninvert.io.in := Bool(true)
+  assert(invert.io.out === Bool(false))
+  assert(noninvert.io.out === Bool(true))
+
+  stop()
+}
+
+class ParameterizedModuleSpec extends ChiselFlatSpec {
+  "Different parameterized modules" should "have different behavior" in {
+    assert(execute{ new ParameterizedModuleTester() })
+  }
+}