From 651fbe9339aca5fcb562715d00b1f87cf66296ee Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Thu, 28 Jan 2021 18:39:57 -0800 Subject: Stop padding multiply and divide ops (#2058) Fixes bug with mul or div followed by cat. Also fixes some Verilog lint issues.--- src/main/scala/firrtl/passes/PadWidths.scala | 2 +- src/test/scala/firrtl/testutils/FirrtlSpec.scala | 42 +++++++ .../scala/firrtlTests/VerilogEmitterTests.scala | 24 ++++ .../scala/firrtlTests/VerilogEquivalenceSpec.scala | 123 +++++++++++++++++++++ 4 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 src/test/scala/firrtlTests/VerilogEquivalenceSpec.scala (limited to 'src') diff --git a/src/main/scala/firrtl/passes/PadWidths.scala b/src/main/scala/firrtl/passes/PadWidths.scala index 875e80ae..1a430778 100644 --- a/src/main/scala/firrtl/passes/PadWidths.scala +++ b/src/main/scala/firrtl/passes/PadWidths.scala @@ -58,7 +58,7 @@ object PadWidths extends Pass { case ex: ValidIf => ex.copy(value = fixup(width(ex.tpe))(ex.value)) case ex: DoPrim => ex.op match { - case Lt | Leq | Gt | Geq | Eq | Neq | Not | And | Or | Xor | Add | Sub | Mul | Div | Rem | Shr => + case Lt | Leq | Gt | Geq | Eq | Neq | Not | And | Or | Xor | Add | Sub | Rem | Shr => // sensitive ops ex.map(fixup((ex.args.map(width).foldLeft(0))(math.max))) case Dshl => diff --git a/src/test/scala/firrtl/testutils/FirrtlSpec.scala b/src/test/scala/firrtl/testutils/FirrtlSpec.scala index 3a6f9372..6de2af1e 100644 --- a/src/test/scala/firrtl/testutils/FirrtlSpec.scala +++ b/src/test/scala/firrtl/testutils/FirrtlSpec.scala @@ -122,6 +122,48 @@ trait FirrtlRunners extends BackendCompilationUtilities { assert(BackendCompilationUtilities.yosysExpectSuccess(customName, refName, testDir, timesteps)) } + /** Check equivalence of Firrtl with reference Verilog + * + * @note the name of the reference Verilog module is grabbed via regex + * @param inputFirrtl string containing Firrtl source + * @param referenceVerilog Verilog that will be used as reference for LEC + * @param timesteps the maximum number of timesteps to consider + */ + def firrtlEquivalenceWithVerilog( + inputFirrtl: String, + referenceVerilog: String, + timesteps: Int = 1 + ): Unit = { + val VerilogModule = """(?s).*module\s(\w+).*""".r + val refName = referenceVerilog match { + case VerilogModule(name) => name + case _ => throw new Exception(s"Reference Verilog must match simple regex! $VerilogModule") + } + val circuit = Parser.parse(inputFirrtl.split("\n").toIterator) + val inputName = circuit.main + require(refName != inputName, s"Name of reference Verilog must not match name of input FIRRTL: $refName") + + val testDir = createTestDirectory(inputName + "_equivalence_test") + + val annos = List( + TargetDirAnnotation(testDir.toString), + InfoModeAnnotation("ignore"), + stage.FirrtlCircuitAnnotation(circuit), + stage.RunFirrtlTransformAnnotation.stringToEmitter("verilog"), + stage.OutputFileAnnotation(inputName) + ) + + (new firrtl.stage.FirrtlStage).execute(Array(), annos) + + // Write reference + val w = new FileWriter(new File(testDir, s"$refName.v")) + w.write(referenceVerilog) + w.close() + + assert(BackendCompilationUtilities.yosysExpectSuccess(inputName, refName, testDir, timesteps)) + } + + /** Compiles input Firrtl to Verilog */ def compileToVerilog(input: String, annotations: AnnotationSeq = Seq.empty): String = { val circuit = Parser.parse(input.split("\n").toIterator) diff --git a/src/test/scala/firrtlTests/VerilogEmitterTests.scala b/src/test/scala/firrtlTests/VerilogEmitterTests.scala index 7704a0a2..ec30e55c 100644 --- a/src/test/scala/firrtlTests/VerilogEmitterTests.scala +++ b/src/test/scala/firrtlTests/VerilogEmitterTests.scala @@ -754,6 +754,30 @@ class VerilogEmitterSpec extends FirrtlFlatSpec { result should containLine("assign z = _GEN_0[1:0];") } + it should "not pad multiplication" in { + val compiler = new VerilogCompiler + val result = compileBody( + """input x : UInt<2> + |input y: UInt<4> + |output z : UInt<6> + |z <= mul(x, y) + |""".stripMargin + ) + result should containLine("assign z = x * y;") + } + + it should "not pad division" in { + val compiler = new VerilogCompiler + val result = compileBody( + """input x : UInt<4> + |input y: UInt<2> + |output z : UInt<4> + |z <= div(x, y) + |""".stripMargin + ) + result should containLine("assign z = x / y;") + } + it should "correctly emit addition with a negative literal with width > 32" in { val result = compileBody( """input x : SInt<34> diff --git a/src/test/scala/firrtlTests/VerilogEquivalenceSpec.scala b/src/test/scala/firrtlTests/VerilogEquivalenceSpec.scala new file mode 100644 index 00000000..d88309ce --- /dev/null +++ b/src/test/scala/firrtlTests/VerilogEquivalenceSpec.scala @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: Apache-2.0 + +package firrtlTests + +import firrtl.testutils._ + +class VerilogEquivalenceSpec extends FirrtlFlatSpec { + "mul followed by cat" should "be correct" in { + val header = s""" + |circuit Multiply : + | module Multiply : + | input x : UInt<4> + | input y : UInt<2> + | input z : UInt<2> + | output out : UInt<8> + |""".stripMargin + val input1 = header + """ + | out <= cat(z, mul(x, y))""".stripMargin + val input2 = header + """ + | node n = mul(x, y) + | node m = cat(z, n) + | out <= m""".stripMargin + val expected = s""" + |module MultiplyRef( + | input [3:0] x, + | input [1:0] y, + | input [1:0] z, + | output [7:0] out + |); + | wire [5:0] w = x * y; + | assign out = {z, w}; + |endmodule""".stripMargin + firrtlEquivalenceWithVerilog(input1, expected) + firrtlEquivalenceWithVerilog(input2, expected) + } + + "div followed by cat" should "be correct" in { + val header = s""" + |circuit Divide : + | module Divide : + | input x : UInt<4> + | input y : UInt<2> + | input z : UInt<2> + | output out : UInt<6> + |""".stripMargin + val input1 = header + """ + | out <= cat(z, div(x, y))""".stripMargin + val input2 = header + """ + | node n = div(x, y) + | node m = cat(z, n) + | out <= m""".stripMargin + val expected = s""" + |module DivideRef( + | input [3:0] x, + | input [1:0] y, + | input [1:0] z, + | output [5:0] out + |); + | wire [3:0] w = x / y; + | assign out = {z, w}; + |endmodule""".stripMargin + firrtlEquivalenceWithVerilog(input1, expected) + firrtlEquivalenceWithVerilog(input2, expected) + } + + "signed mul followed by cat" should "be correct" in { + val header = s""" + |circuit SignedMultiply : + | module SignedMultiply : + | input x : SInt<4> + | input y : SInt<2> + | input z : SInt<2> + | output out : UInt<8> + |""".stripMargin + val input1 = header + """ + | out <= cat(z, mul(x, y))""".stripMargin + val input2 = header + """ + | node n = mul(x, y) + | node m = cat(z, n) + | out <= m""".stripMargin + val expected = s""" + |module SignedMultiplyRef( + | input signed [3:0] x, + | input signed [1:0] y, + | input signed [1:0] z, + | output [7:0] out + |); + | wire [5:0] w = x * y; + | assign out = {z, w}; + |endmodule""".stripMargin + firrtlEquivalenceWithVerilog(input1, expected) + firrtlEquivalenceWithVerilog(input2, expected) + } + + "signed div followed by cat" should "be correct" in { + val header = s""" + |circuit SignedDivide : + | module SignedDivide : + | input x : SInt<4> + | input y : SInt<2> + | input z : SInt<2> + | output out : UInt<7> + |""".stripMargin + val input1 = header + """ + | out <= cat(z, div(x, y))""".stripMargin + val input2 = header + """ + | node n = div(x, y) + | node m = cat(z, n) + | out <= m""".stripMargin + val expected = s""" + |module SignedDivideRef( + | input signed [3:0] x, + | input signed [1:0] y, + | input signed [1:0] z, + | output [6:0] out + |); + | wire [4:0] w = x / y; + | assign out = {z, w}; + |endmodule""".stripMargin + firrtlEquivalenceWithVerilog(input1, expected) + firrtlEquivalenceWithVerilog(input2, expected) + } +} -- cgit v1.2.3