ConstProp attached wires if there is also a port (#818)

This enables the pattern of attaching "through" a wire to give better
Verilog that also works in Verilator

Use WrappedExpression when combining attaches in ExpandWhens
to ensure no duplication of references in resulting, combined attaches

3 files changed, 34 insertions, 10 deletions

diff --git a/src/main/scala/firrtl/passes/ExpandWhens.scala b/src/main/scala/firrtl/passes/ExpandWhens.scala
index 4abae636..3665590f 100644
--- a/src/main/scala/firrtl/passes/ExpandWhens.scala
+++ b/src/main/scala/firrtl/passes/ExpandWhens.scala
@@ -235,11 +235,12 @@ object ExpandWhens extends Pass {
     */
   private def combineAttaches(attaches: Seq[Attach]): Seq[Attach] = {
     // Helper type to add an ordering index to attached Expressions
-    case class AttachAcc(exprs: Seq[Expression], idx: Int)
+    case class AttachAcc(exprs: Seq[WrappedExpression], idx: Int)
     // Map from every attached expression to its corresponding AttachAcc
     //   (many keys will point to same value)
     val attachMap = mutable.HashMap.empty[WrappedExpression, AttachAcc]
-    for (Attach(_, exprs) <- attaches) {
+    for (Attach(_, es) <- attaches) {
+      val exprs = es.map(we(_))
       val acc = exprs.map(attachMap.get(_)).flatten match {
         case Seq() => // None of these expressions is present in the attachMap
           AttachAcc(exprs, attachMap.size)
@@ -247,9 +248,9 @@ object ExpandWhens extends Pass {
           val sorted = accs sortBy (_.idx)
           AttachAcc((sorted.map(_.exprs) :+ exprs).flatten.distinct, sorted.head.idx)
       }
-      attachMap ++= acc.exprs.map(e => (we(e) -> acc))
+      attachMap ++= acc.exprs.map(_ -> acc)
     }
-    attachMap.values.toList.distinct.map(acc => Attach(NoInfo, acc.exprs))
+    attachMap.values.toList.distinct.map(acc => Attach(NoInfo, acc.exprs.map(_.e1)))
   }
   // Searches nested scopes of defaults for lvalue
   // defaults uses mutable Map because we are searching LinkedHashMaps and conversion to immutable is VERY slow
diff --git a/src/main/scala/firrtl/transforms/ConstantPropagation.scala b/src/main/scala/firrtl/transforms/ConstantPropagation.scala
index 5e9a7850..4a4f41d1 100644
--- a/src/main/scala/firrtl/transforms/ConstantPropagation.scala
+++ b/src/main/scala/firrtl/transforms/ConstantPropagation.scala
@@ -396,6 +396,9 @@ class ConstantPropagation extends Transform {
         // Propagate connections to references
         case Connect(info, lhs, rref @ WRef(rname, _, NodeKind, _)) if !dontTouches.contains(rname) =>
           Connect(info, lhs, nodeMap(rname))
+        // If an Attach has at least 1 port, any wires are redundant and can be removed
+        case Attach(info, exprs) if exprs.exists(kind(_) == PortKind) =>
+          Attach(info, exprs.filterNot(kind(_) == WireKind))
         case other => other
       }
     }
diff --git a/src/test/scala/firrtlTests/AttachSpec.scala b/src/test/scala/firrtlTests/AttachSpec.scala
index c9c609df..9bf5fefd 100644
--- a/src/test/scala/firrtlTests/AttachSpec.scala
+++ b/src/test/scala/firrtlTests/AttachSpec.scala
@@ -107,6 +107,31 @@ class InoutVerilogSpec extends FirrtlFlatSpec {
     executeTest(input, check, compiler, Seq(dontTouch("Attaching.x")))
   }
 
+  it should "attach port to submodule port through a wire" in {
+    val compiler = new VerilogCompiler
+    val input =
+      """circuit Attaching :
+         |  module Attaching :
+         |    input an: Analog<3>
+         |    wire x: Analog
+         |    inst a of A
+         |    attach (x, a.an)
+         |    attach (x, an)
+         |  module A:
+         |    input an: Analog<3> """.stripMargin
+    val check =
+      """module Attaching(
+        |  inout [2:0] an
+        |);
+        |  A a (
+        |    .an(an)
+        |  );
+        |endmodule
+        |""".stripMargin.split("\n") map normalized
+    executeTest(input, check, compiler, Seq(dontTouch("Attaching.x")))
+  }
+
+
   it should "attach multiple sources" in {
     val compiler = new VerilogCompiler
     val input =
@@ -121,18 +146,13 @@ class InoutVerilogSpec extends FirrtlFlatSpec {
         |  inout  [2:0] a1,
         |  inout  [2:0] a2
         |);
-        |  wire [2:0] x;
         |  `ifdef SYNTHESIS
-        |    assign x = a1;
-        |    assign a1 = x;
-        |    assign x = a2;
-        |    assign a2 = x;
         |    assign a1 = a2;
         |    assign a2 = a1;
         |  `elsif verilator
         |    `error "Verilator does not support alias and thus cannot arbirarily connect bidirectional wires and ports"
         |  `else
-        |    alias x = a1 = a2;
+        |    alias a1 = a2;
         |  `endif
         |endmodule
         |""".stripMargin.split("\n") map normalized