Define leading '_' as API for creating temporaries (backport #2580) (#2581)

* Define leading '_' as API for creating temporaries

Chisel and FIRRTL have long used signals with names beginning with an
underscore as an API to specify that the name does not really matter.
Tools like Verilator follow a similar convention and exclude signals
with underscore names from waveform dumps by default. With the
introduction of compiler-plugin prefixing in Chisel 3.4, the convention
remained but was hard for users to use unless the unnnamed signal
existed outside of any prefix domain. Notably, unnamed signals are most
useful when creating wires inside of utility methods which almost always
results in the signal ending up with a prefix.

With this commit, Chisel explicitly recognizes signals whos val names
start with an underscore and preserve that underscore regardless of any
prefixing. Chisel will also ignore such underscores when generating
prefixes based on the temporary signal, preventing accidental double
underscores in the names of signals that are prefixed by the temporary.

(cherry picked from commit bd94366290886f3489d58f88b9768c7c11fa2cb6)

* Remove unused defaultPrefix argument from _computeName

(cherry picked from commit ec178aa20a830df2c8c756b9e569709a59073554)

# Conflicts:
#	core/src/main/scala/chisel3/Module.scala
#	core/src/main/scala/chisel3/experimental/hierarchy/ModuleClone.scala

* Resolve backport conflicts

* Waive false positive binary compatibility errors

Co-authored-by: Jack Koenig <koenig@sifive.com>

2 files changed, 107 insertions, 5 deletions

diff --git a/src/test/scala/chiselTests/naming/NamePluginSpec.scala b/src/test/scala/chiselTests/naming/NamePluginSpec.scala
index 482ef62b..a787bb80 100644
--- a/src/test/scala/chiselTests/naming/NamePluginSpec.scala
+++ b/src/test/scala/chiselTests/naming/NamePluginSpec.scala
@@ -340,4 +340,23 @@ class NamePluginSpec extends ChiselFlatSpec with Utils {
       Select.wires(top).map(_.instanceName) should be(List("a_b_c", "a_b", "a"))
     }
   }
+
+  behavior.of("Unnamed values (aka \"Temporaries\")")
+
+  they should "be declared by starting the name with '_'" in {
+    class Test extends Module {
+      {
+        val a = {
+          val b = {
+            val _c = Wire(UInt(3.W))
+            4.U // literal so there is no name
+          }
+          b
+        }
+      }
+    }
+    aspectTest(() => new Test) { top: Test =>
+      Select.wires(top).map(_.instanceName) should be(List("_a_b_c"))
+    }
+  }
 }
diff --git a/src/test/scala/chiselTests/naming/PrefixSpec.scala b/src/test/scala/chiselTests/naming/PrefixSpec.scala
index 1e628391..6d52407e 100644
--- a/src/test/scala/chiselTests/naming/PrefixSpec.scala
+++ b/src/test/scala/chiselTests/naming/PrefixSpec.scala
@@ -233,18 +233,46 @@ class PrefixSpec extends ChiselPropSpec with Utils {
     }
   }
 
-  property("Prefixing should be the prefix during the last call to autoName/suggestName") {
+  property("Prefixing should NOT be influenced by suggestName") {
     class Test extends Module {
       {
         val wire = {
-          val x = Wire(UInt(3.W)).suggestName("mywire")
-          x
+          val x = Wire(UInt(3.W)) // wire_x
+          Wire(UInt(3.W)).suggestName("foo")
+        }
+      }
+    }
+    aspectTest(() => new Test) { top: Test =>
+      Select.wires(top).map(_.instanceName) should be(List("wire_x", "foo"))
+    }
+  }
+
+  property("Prefixing should be influenced by the \"current name\" of the signal") {
+    class Test extends Module {
+      {
+        val wire = {
+          val y = Wire(UInt(3.W)).suggestName("foo")
+          val x = Wire(UInt(3.W)) // wire_x
+          y
+        }
+
+        val wire2 = Wire(UInt(3.W))
+        wire2 := {
+          val x = Wire(UInt(3.W)) // wire2_x
+          x + 1.U
+        }
+        wire2.suggestName("bar")
+
+        val wire3 = Wire(UInt(3.W))
+        wire3.suggestName("fizz")
+        wire3 := {
+          val x = Wire(UInt(3.W)) // fizz_x
+          x + 1.U
         }
       }
     }
     aspectTest(() => new Test) { top: Test =>
-      Select.wires(top).map(_.instanceName) should be(List("mywire"))
-      Select.wires(top).map(_.instanceName) shouldNot be(List("wire_mywire"))
+      Select.wires(top).map(_.instanceName) should be(List("foo", "wire_x", "bar", "wire2_x", "fizz", "fizz_x"))
     }
   }
 
@@ -414,4 +442,59 @@ class PrefixSpec extends ChiselPropSpec with Utils {
     (chirrtl should include).regex("assume.*: x5_x3")
     (chirrtl should include).regex("printf.*: x5_x4")
   }
+
+  property("Leading '_' in val names should be ignored in prefixes") {
+    class Test extends Module {
+      {
+        val a = {
+          val _b = {
+            val c = Wire(UInt(3.W))
+            4.U // literal because there is no name
+          }
+          _b
+        }
+      }
+    }
+    aspectTest(() => new Test) { top: Test =>
+      Select.wires(top).map(_.instanceName) should be(List("a_b_c"))
+    }
+  }
+
+  // This checks that we don't just blanket ignore leading _ in prefixes
+  property("User-specified prefixes with '_' should be respected") {
+    class Test extends Module {
+      {
+        val a = {
+          val _b = prefix("_b") {
+            val c = Wire(UInt(3.W))
+          }
+          4.U
+        }
+      }
+    }
+    aspectTest(() => new Test) { top: Test =>
+      Select.wires(top).map(_.instanceName) should be(List("a__b_c"))
+    }
+  }
+
+  property("Leading '_' in signal names should be ignored in prefixes from connections") {
+    class Test extends Module {
+      {
+        val a = {
+          val b = {
+            val _c = IO(Output(UInt(3.W))) // port so not selected as wire
+            _c := {
+              val d = Wire(UInt(3.W))
+              d
+            }
+            4.U // literal so there is no name
+          }
+          b
+        }
+      }
+    }
+    aspectTest(() => new Test) { top: Test =>
+      Select.wires(top).map(_.instanceName) should be(List("a_b_c_d"))
+    }
+  }
 }