Bugfix: Dedup aggressively (ignore comments) (#375)

FileInfo is merged

5 files changed, 134 insertions, 10 deletions

diff --git a/src/main/scala/firrtl/ExecutionOptionsManager.scala b/src/main/scala/firrtl/ExecutionOptionsManager.scala
index f91f45c5..977ef9bb 100644
--- a/src/main/scala/firrtl/ExecutionOptionsManager.scala
+++ b/src/main/scala/firrtl/ExecutionOptionsManager.scala
@@ -260,7 +260,7 @@ trait HasFirrtlOptions {
       else parser.failure(s"$x bad value must be one of ignore|use|gen|append")
     }
     .text {
-      s"specifies the source info handling, default is ${firrtlOptions.infoMode}"
+      s"specifies the source info handling, default is ${firrtlOptions.infoModeName}"
     }
 
   parser.opt[Seq[String]]("inline")
diff --git a/src/main/scala/firrtl/ir/IR.scala b/src/main/scala/firrtl/ir/IR.scala
index b26eec4c..6eac592c 100644
--- a/src/main/scala/firrtl/ir/IR.scala
+++ b/src/main/scala/firrtl/ir/IR.scala
@@ -13,12 +13,18 @@ abstract class FirrtlNode {
 abstract class Info extends FirrtlNode {
   // default implementation
   def serialize: String = this.toString
+  def ++(that: Info): Info
 }
 case object NoInfo extends Info {
   override def toString: String = ""
+  def ++(that: Info): Info = that
 }
 case class FileInfo(info: StringLit) extends Info {
   override def toString: String = " @[" + info.serialize + "]"
+  def ++(that: Info): Info = that match {
+    case NoInfo => this
+    case FileInfo(otherInfo) => FileInfo(FIRRTLStringLitHandler.unescape(info.serialize + " " + otherInfo.serialize))
+  }
 }
 
 trait HasName {
diff --git a/src/main/scala/firrtl/transforms/Dedup.scala b/src/main/scala/firrtl/transforms/Dedup.scala
index 0d9be831..60a5cfc8 100644
--- a/src/main/scala/firrtl/transforms/Dedup.scala
+++ b/src/main/scala/firrtl/transforms/Dedup.scala
@@ -53,15 +53,39 @@ class DedupModules extends Transform {
     val dedupModules = mutable.HashMap.empty[String, String]
     // Old module name -> dup module name
     val dedupMap = mutable.HashMap.empty[String, String]
-    def onModule(m: DefModule): Option[DefModule] = {
+    // Dup module name -> all old module names
+    val oldModuleMap = mutable.HashMap.empty[String, Seq[DefModule]]
+
+    def onModule(m: DefModule): Unit = {
       def fixInstance(s: Statement): Statement = s map fixInstance match {
         case DefInstance(i, n, m) => DefInstance(i, n, dedupMap.getOrElse(m, m))
         case WDefInstance(i, n, m, t) => WDefInstance(i, n, dedupMap.getOrElse(m, m), t)
         case x => x
       }
+      def removeInfo(stmt: Statement): Statement = stmt map removeInfo match {
+        case sx: HasInfo => sx match {
+          case s: DefWire => s.copy(info = NoInfo)
+          case s: DefNode => s.copy(info = NoInfo)
+          case s: DefRegister => s.copy(info = NoInfo)
+          case s: DefInstance => s.copy(info = NoInfo)
+          case s: WDefInstance => s.copy(info = NoInfo)
+          case s: DefMemory => s.copy(info = NoInfo)
+          case s: Connect => s.copy(info = NoInfo)
+          case s: PartialConnect => s.copy(info = NoInfo)
+          case s: IsInvalid => s.copy(info = NoInfo)
+          case s: Attach => s.copy(info = NoInfo)
+          case s: Stop => s.copy(info = NoInfo)
+          case s: Print => s.copy(info = NoInfo)
+          case s: Conditionally => s.copy(info = NoInfo)
+        }
+        case sx => sx
+      }
+      def removePortInfo(p: Port): Port = p.copy(info = NoInfo)
+
 
       val mx = m map fixInstance
-      val string = mx match {
+      val mxx = (mx map removeInfo) map removePortInfo
+      val string = mxx match {
         case Module(i, n, ps, b) =>
           ps.map(_.serialize).mkString + b.serialize
         case ExtModule(i, n, ps, dn, p) =>
@@ -70,14 +94,70 @@ class DedupModules extends Transform {
       dedupModules.get(string) match {
         case Some(dupname) =>
           dedupMap(mx.name) = dupname
-          None
+          oldModuleMap(dupname) = oldModuleMap(dupname) :+ mx
         case None =>
           dedupModules(string) = mx.name
-          Some(mx)
+          oldModuleMap(mx.name) = Seq(mx)
+      }
+    }
+    def mergeModules(ms: Seq[DefModule]) = {
+      def mergeStatements(ss: Seq[Statement]): Statement = ss.head match {
+        case Block(stmts) =>
+          val inverted = invertSeqs(ss.map { case Block(s) => s })
+          val finalStmts = inverted.map { jStmts => mergeStatements(jStmts) }
+          Block(finalStmts.toSeq)
+        case Conditionally(info, pred, conseq, alt) =>
+          val finalConseq = mergeStatements(ss.map { case Conditionally(_, _, c, _) => c })
+          val finalAlt = mergeStatements(ss.map { case Conditionally(_, _, _, a) => a })
+          val finalInfo = ss.map { case Conditionally(i, _, _, _) => i }.reduce (_ ++ _)
+          Conditionally(finalInfo, pred, finalConseq, finalAlt)
+        case sx: HasInfo => sx match {
+          case s: DefWire => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: DefNode => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: DefRegister => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: DefInstance => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: WDefInstance => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: DefMemory => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: Connect => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: PartialConnect => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: IsInvalid => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: Attach => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: Stop => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+          case s: Print => s.copy(info = ss.map(getInfo).reduce(_ ++ _))
+        }
+        case s => s
+      }
+      def getInfo(s: Any): Info = s match {
+        case sx: HasInfo => sx.info
+        case _ => NoInfo
+      }
+      def invertSeqs[A](seq: Seq[Seq[A]]): Seq[Seq[A]] = {
+        val finalSeq = collection.mutable.ArrayBuffer[Seq[A]]()
+        for(j <- 0 until seq.head.size) {
+          finalSeq += seq.map(s => s(j))
+        }
+        finalSeq.toSeq
+      }
+      val finalPorts = invertSeqs(ms.map(_.ports)).map { jPorts => 
+        jPorts.tail.foldLeft(jPorts.head) { (p1, p2) =>
+          Port(p1.info ++ p2.info, p1.name, p1.direction, p1.tpe)
+        }
+      }
+      val finalInfo = ms.map(getInfo).reduce(_ ++ _)
+      ms.head match {
+        case e: ExtModule => ExtModule(finalInfo, e.name, finalPorts, e.defname, e.params)
+        case e: Module => Module(finalInfo, e.name, finalPorts, mergeStatements(ms.collect { case m: Module => m.body}))
+      }
+    }
+    moduleOrder.foreach(n => onModule(moduleMap(n)))
+
+    // Use old module list to preserve ordering
+    val dedupedModules = c.modules.flatMap { m => 
+      oldModuleMap.get(m.name) match {
+        case Some(modules) => Some(mergeModules(modules))
+        case None => None
       }
     }
-    val modulesx = moduleOrder.flatMap(n => onModule(moduleMap(n)))
-    val modulesxMap = modulesx.map(m => m.name -> m).toMap
-    c.copy(modules = c.modules.flatMap(m => modulesxMap.get(m.name)))
+    c.copy(modules = dedupedModules)
   }
 }
diff --git a/src/test/scala/firrtlTests/PassTests.scala b/src/test/scala/firrtlTests/PassTests.scala
index 7fa67153..44bd0382 100644
--- a/src/test/scala/firrtlTests/PassTests.scala
+++ b/src/test/scala/firrtlTests/PassTests.scala
@@ -7,7 +7,7 @@ import java.io.{StringWriter,Writer}
 import org.scalatest.{FlatSpec, Matchers}
 import org.scalatest.junit.JUnitRunner
 import firrtl.ir.Circuit
-import firrtl.Parser.IgnoreInfo
+import firrtl.Parser.UseInfo
 import firrtl.passes.{Pass, PassExceptions, RemoveEmpty}
 import firrtl.{
    Transform,
@@ -37,7 +37,7 @@ abstract class SimpleTransformSpec extends FlatSpec with Matchers with Compiler
    def emitter = new FirrtlEmitter
 
    // Utility function
-   def parse(s: String): Circuit = Parser.parse(s.split("\n").toIterator, infoMode = IgnoreInfo)
+   def parse(s: String): Circuit = Parser.parse(s.split("\n").toIterator, infoMode = UseInfo)
    def squash(c: Circuit): Circuit = RemoveEmpty.run(c)
 
    // Executes the test. Call in tests.
diff --git a/src/test/scala/firrtlTests/transforms/DedupTests.scala b/src/test/scala/firrtlTests/transforms/DedupTests.scala
index 1bcd711c..a17e6ae1 100644
--- a/src/test/scala/firrtlTests/transforms/DedupTests.scala
+++ b/src/test/scala/firrtlTests/transforms/DedupTests.scala
@@ -89,6 +89,44 @@ class DedupModuleTests extends HighTransformSpec {
       val aMap = new AnnotationMap(Nil)
       execute(writer, aMap, input, check)
    }
+   "The module A and B with comments" should "be deduped" in {
+      val input =
+         """circuit Top :
+           |  module Top :
+           |    inst a1 of A
+           |    inst a2 of A_
+           |  module A : @[yy 2:2]
+           |    output x: UInt<1> @[yy 2:2]
+           |    inst b of B @[yy 2:2]
+           |    x <= b.x @[yy 2:2]
+           |  module A_ : @[xx 1:1]
+           |    output x: UInt<1> @[xx 1:1]
+           |    inst b of B_ @[xx 1:1]
+           |    x <= b.x @[xx 1:1]
+           |  module B :
+           |    output x: UInt<1>
+           |    x <= UInt(1)
+           |  module B_ :
+           |    output x: UInt<1>
+           |    x <= UInt(1)
+           """.stripMargin
+      val check =
+         """circuit Top :
+           |  module Top :
+           |    inst a1 of A
+           |    inst a2 of A
+           |  module A : @[yy 2:2 xx 1:1]
+           |    output x: UInt<1> @[yy 2:2 xx 1:1]
+           |    inst b of B @[yy 2:2 xx 1:1]
+           |    x <= b.x @[yy 2:2 xx 1:1]
+           |  module B :
+           |    output x: UInt<1>
+           |    x <= UInt(1)
+           """.stripMargin
+      val writer = new StringWriter()
+      val aMap = new AnnotationMap(Nil)
+      execute(writer, aMap, input, check)
+   }
 }
 
 // Execution driven tests for inlining modules