From 8187318e7aef42d541ce307f93d9fc946ed4c38d Mon Sep 17 00:00:00 2001
From: Jack Koenig <koenig@sifive.com>
Date: Thu, 5 Nov 2020 15:54:14 -0800
Subject: [PATCH] For HasId.setRef, have first set win (with force override)
 (#1655)

This is a refinement of the assertion added in #1616 then removed in
 #1654. Because Records now set the refs of children upon binding,
later, unbound Records could incorrectly override the refs. The first
set should win.
---
 core/src/main/scala/chisel3/BlackBox.scala         |  4 ++--
 core/src/main/scala/chisel3/internal/Builder.scala |  7 +++++--
 src/test/scala/chiselTests/BundleSpec.scala        | 13 +++++++++++++
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala
index 8f83104495a..1b093ee1e80 100644
--- a/core/src/main/scala/chisel3/BlackBox.scala
+++ b/core/src/main/scala/chisel3/BlackBox.scala
@@ -165,8 +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) {
-      // Override the _ref because it was set during IO binding
-      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/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index 57a0f968066..56a85fb6153 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -198,8 +198,11 @@ 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): Unit = setRef(imm, false)
+  private[chisel3] def setRef(imm: Arg, force: Boolean): Unit = {
+    if (_ref.isEmpty || force) {
+      _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)))
diff --git a/src/test/scala/chiselTests/BundleSpec.scala b/src/test/scala/chiselTests/BundleSpec.scala
index df02aa20af8..5d3f23ecbbf 100644
--- a/src/test/scala/chiselTests/BundleSpec.scala
+++ b/src/test/scala/chiselTests/BundleSpec.scala
@@ -141,4 +141,17 @@ class BundleSpec extends ChiselFlatSpec with BundleSpecUtils with Utils {
       }
     }
   }
+
+  "Bound Data" should "have priority in setting ref over unbound Data" in {
+    class MyModule extends RawModule {
+      val foo = IO(new Bundle {
+        val x = Output(UInt(8.W))
+      })
+      foo.x := 0.U // getRef on foo.x is None.get without fix
+      val bar = new Bundle {
+        val y = foo.x
+      }
+    }
+    ChiselStage.emitChirrtl(new MyModule)
+  }
 }