Legalize neg: -x becomes 0 - x (#2128)

This fixes an error with negating a negative SInt literal and a
[debatable] lint warning in Verilator when negating any value.

This behavior matches that of Chisel (which directly emits the 0 - x
already).

3 files changed, 74 insertions, 1 deletions

diff --git a/src/test/scala/firrtl/testutils/FirrtlSpec.scala b/src/test/scala/firrtl/testutils/FirrtlSpec.scala
index 63def26a..4dc2d642 100644
--- a/src/test/scala/firrtl/testutils/FirrtlSpec.scala
+++ b/src/test/scala/firrtl/testutils/FirrtlSpec.scala
@@ -4,6 +4,7 @@ package firrtl.testutils
 
 import java.io._
 import java.security.Permission
+import scala.sys.process._
 
 import logger.{LazyLogging, LogLevel, LogLevelAnnotation}
 
@@ -175,6 +176,22 @@ trait FirrtlRunners extends BackendCompilationUtilities {
     compiler.compileAndEmit(CircuitState(circuit, HighForm, annotations), extraCheckTransforms)
   }
 
+  /** Run Verilator lint on some Verilog text
+    *
+    * @param inputVerilog Verilog to pass to `verilator --lint-only`
+    * @return Verilator return 0
+    */
+  def lintVerilog(inputVerilog: String): Unit = {
+    val testDir = createTestDirectory(s"${this.getClass.getSimpleName}_lint")
+    val filename = new File(testDir, "test.v")
+    val w = new FileWriter(filename)
+    w.write(inputVerilog)
+    w.close()
+
+    val cmd = Seq("verilator", "--lint-only", filename.toString)
+    assert(cmd.!(loggingProcessLogger) == 0, "Lint must pass")
+  }
+
   /** Compile a Firrtl file
     *
     * @param prefix is the name of the Firrtl file without path or file extension
@@ -413,6 +430,14 @@ abstract class ExecutionTest(
   }
 }
 
+/** Super class for execution driven Firrtl tests compiled without optimizations */
+abstract class ExecutionTestNoOpt(
+  name:        String,
+  dir:         String,
+  vFiles:      Seq[String] = Seq.empty,
+  annotations: AnnotationSeq = Seq.empty)
+    extends ExecutionTest(name, dir, vFiles, RunFirrtlTransformAnnotation(new MinimumVerilogEmitter) +: annotations)
+
 /** Super class for compilation driven Firrtl tests */
 abstract class CompilationTest(name: String, dir: String) extends FirrtlPropSpec {
   property(s"$name should compile correctly") {
diff --git a/src/test/scala/firrtlTests/LegalizeSpec.scala b/src/test/scala/firrtlTests/LegalizeSpec.scala
index 905d578e..ad85668e 100644
--- a/src/test/scala/firrtlTests/LegalizeSpec.scala
+++ b/src/test/scala/firrtlTests/LegalizeSpec.scala
@@ -2,6 +2,8 @@
 
 package firrtlTests
 
-import firrtl.testutils.ExecutionTest
+import firrtl.testutils.{ExecutionTest, ExecutionTestNoOpt}
 
 class LegalizeExecutionTest extends ExecutionTest("Legalize", "/passes/Legalize")
+// Legalize also needs to work when optimizations are turned off
+class LegalizeExecutionTestNoOpt extends ExecutionTestNoOpt("Legalize", "/passes/Legalize")
diff --git a/src/test/scala/firrtlTests/NegSpec.scala b/src/test/scala/firrtlTests/NegSpec.scala
new file mode 100644
index 00000000..c60294e3
--- /dev/null
+++ b/src/test/scala/firrtlTests/NegSpec.scala
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package firrtlTests
+
+import firrtl.testutils._
+
+class NegSpec extends FirrtlFlatSpec {
+  "unsigned neg" should "be correct and lint-clean" in {
+    val input =
+      """|circuit UnsignedNeg :
+         |  module UnsignedNeg  :
+         |    input in : UInt<8>
+         |    output out : SInt
+         |    out <= neg(in)
+         |""".stripMargin
+    val expected =
+      """|module UnsignedNegRef(
+         |  input [7:0] in,
+         |  output [8:0] out
+         |);
+         |  assign out = 8'd0 - in;
+         |endmodule""".stripMargin
+    firrtlEquivalenceWithVerilog(input, expected)
+    lintVerilog(compileToVerilog(input))
+  }
+
+  "signed neg" should "be correct and lint-clean" in {
+    val input =
+      """|circuit SignedNeg :
+         |  module SignedNeg  :
+         |    input in : SInt<8>
+         |    output out : SInt
+         |    out <= neg(in)
+         |""".stripMargin
+    // -$signed(in) is a lint warning in Verilator but is functionally correct
+    val expected =
+      """|module SignedNegRef(
+         |  input [7:0] in,
+         |  output [8:0] out
+         |);
+         |  assign out = -$signed(in);
+         |endmodule""".stripMargin
+    firrtlEquivalenceWithVerilog(input, expected)
+    lintVerilog(compileToVerilog(input))
+  }
+}