From 2d197c841c5400c6deaa1592525be6a1d81dc1e2 Mon Sep 17 00:00:00 2001 From: Kevin Laeufer Date: Tue, 21 Dec 2021 12:07:30 -0500 Subject: [PATCH 1/5] smt: deal correctly with negative SInt literals (#2447) --- .../smt/FirrtlExpressionSemantics.scala | 4 ++- .../smt/FirrtlExpressionSemanticsSpec.scala | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/main/scala/firrtl/backends/experimental/smt/FirrtlExpressionSemantics.scala b/src/main/scala/firrtl/backends/experimental/smt/FirrtlExpressionSemantics.scala index 6f454a22ad..865382c9e8 100644 --- a/src/main/scala/firrtl/backends/experimental/smt/FirrtlExpressionSemantics.scala +++ b/src/main/scala/firrtl/backends/experimental/smt/FirrtlExpressionSemantics.scala @@ -13,7 +13,9 @@ private object FirrtlExpressionSemantics { case ir.DoPrim(op, args, consts, _) => onPrim(op, args, consts) case r: ir.RefLikeExpression => BVSymbol(r.serialize, getWidth(r)) case ir.UIntLiteral(value, ir.IntWidth(width)) => BVLiteral(value, width.toInt) - case ir.SIntLiteral(value, ir.IntWidth(width)) => BVLiteral(value, width.toInt) + case ir.SIntLiteral(value, ir.IntWidth(width)) => + val twosComplementValue = value & ((BigInt(1) << width.toInt) - 1) + BVLiteral(twosComplementValue, width.toInt) case ir.Mux(cond, tval, fval, _) => val width = List(tval, fval).map(getWidth).max BVIte(toSMT(cond), toSMT(tval, width), toSMT(fval, width)) diff --git a/src/test/scala/firrtl/backends/experimental/smt/FirrtlExpressionSemanticsSpec.scala b/src/test/scala/firrtl/backends/experimental/smt/FirrtlExpressionSemanticsSpec.scala index 7476b20ca7..5202162514 100644 --- a/src/test/scala/firrtl/backends/experimental/smt/FirrtlExpressionSemanticsSpec.scala +++ b/src/test/scala/firrtl/backends/experimental/smt/FirrtlExpressionSemanticsSpec.scala @@ -278,4 +278,35 @@ class FirrtlExpressionSemanticsSpec extends AnyFlatSpec { assert(primop(false, "tail", 4, List(5), List(1)) == "i0[3:0]") assert(primop(false, "tail", 2, List(5), List(3)) == "i0[1:0]") } + + private def literalSource(resTpe: String, lit: String) = + s"""circuit m: + | module m: + | output res: $resTpe + | res <= $lit + | + |""".stripMargin + private def literalExpr(resTpe: String, lit: String) = { + val src = literalSource(resTpe, lit) + val sys = SMTBackendHelpers.toSys(src, modelUndef = true) + sys.signals.last.e.toString + } + + private def uIntLit(lit: String) = literalExpr("UInt", lit) + it should "correctly translate unsigned integer literals" in { + assert(uIntLit("UInt(5)") == "3'b101") + assert(uIntLit("UInt<4>(5)") == "4'b101") + assert(uIntLit("UInt(0)") == "1'b0") + } + + private def sIntLit(lit: String) = literalExpr("SInt", lit) + it should "correctly translate signed integer literals" in { + assert(sIntLit("SInt(5)") == "4'b101") + assert(sIntLit("SInt<4>(5)") == "4'b101") + assert(sIntLit("SInt(0)") == "1'b0") + assert(sIntLit("SInt(-1)") == "1'b1") + assert(sIntLit("SInt(-2)") == "2'b10") + assert(sIntLit("SInt(-5)") == "4'b1011") + assert(sIntLit("SInt<4>(-5)") == "4'b1011") + } } From 4f3d1003811aa38d10e32b347c8607414d9be034 Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Tue, 21 Dec 2021 18:47:18 -0800 Subject: [PATCH 2/5] Remove some warnings (#2448) * Fix unreachable code warning by changing match order Simulation Statements did not previously extend IsDeclaration, but now they do so their match blocks need to be above IsDeclaration. * Handle MemoryNoInit case in RtlilEmitter * Remove use of deprecated logToFile * Fix uses of LegalizeClocksTransform Replaced all uses of LegalizeClocksTransform with LegalizeClocksAndAsyncResetsTransform. * Remove use of CircuitForm in ZeroWidth --- src/main/scala/firrtl/AddDescriptionNodes.scala | 2 +- .../backends/experimental/rtlil/RtlilEmitter.scala | 2 ++ .../firrtl/passes/VerilogModulusCleanup.scala | 2 +- src/main/scala/firrtl/passes/VerilogPrep.scala | 2 +- src/main/scala/firrtl/passes/ZeroWidth.scala | 2 +- src/main/scala/firrtl/stage/Forms.scala | 2 +- .../scala/firrtl/transforms/FlattenRegUpdate.scala | 2 +- .../scala/firrtl/transforms/GroupComponents.scala | 14 +++++++------- .../transforms/InlineAcrossCastsTransform.scala | 2 +- .../transforms/RemoveKeywordCollisions.scala | 2 +- src/main/scala/logger/LoggerOptions.scala | 2 +- 11 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/main/scala/firrtl/AddDescriptionNodes.scala b/src/main/scala/firrtl/AddDescriptionNodes.scala index 90e38beb1e..de9ff523aa 100644 --- a/src/main/scala/firrtl/AddDescriptionNodes.scala +++ b/src/main/scala/firrtl/AddDescriptionNodes.scala @@ -151,7 +151,7 @@ class AddDescriptionNodes extends Transform with DependencyAPIMigration { Dependency[firrtl.transforms.InlineBitExtractionsTransform], Dependency[firrtl.transforms.PropagatePresetAnnotations], Dependency[firrtl.transforms.InlineAcrossCastsTransform], - Dependency[firrtl.transforms.LegalizeClocksTransform], + Dependency[firrtl.transforms.LegalizeClocksAndAsyncResetsTransform], Dependency[firrtl.transforms.FlattenRegUpdate], Dependency(passes.VerilogModulusCleanup), Dependency[firrtl.transforms.VerilogRename], diff --git a/src/main/scala/firrtl/backends/experimental/rtlil/RtlilEmitter.scala b/src/main/scala/firrtl/backends/experimental/rtlil/RtlilEmitter.scala index a5f7f81fdc..6c6c0b6981 100644 --- a/src/main/scala/firrtl/backends/experimental/rtlil/RtlilEmitter.scala +++ b/src/main/scala/firrtl/backends/experimental/rtlil/RtlilEmitter.scala @@ -880,6 +880,8 @@ private[firrtl] class RtlilEmitter extends SeqTransform with Emitter with Depend println("Leaving memory uninitialized.") case MemoryFileInlineInit(_, _) => throw EmitterException(s"Memory $name cannot be initialized from a file, RTLIL cannot express this.") + case MemoryNoInit => + // No initialization to emit } for (r <- rd) { val data = memPortField(x, r, "data") diff --git a/src/main/scala/firrtl/passes/VerilogModulusCleanup.scala b/src/main/scala/firrtl/passes/VerilogModulusCleanup.scala index 03dcf0a397..3ca862b95b 100644 --- a/src/main/scala/firrtl/passes/VerilogModulusCleanup.scala +++ b/src/main/scala/firrtl/passes/VerilogModulusCleanup.scala @@ -33,7 +33,7 @@ object VerilogModulusCleanup extends Pass { Dependency[firrtl.transforms.ReplaceTruncatingArithmetic], Dependency[firrtl.transforms.InlineBitExtractionsTransform], Dependency[firrtl.transforms.InlineAcrossCastsTransform], - Dependency[firrtl.transforms.LegalizeClocksTransform], + Dependency[firrtl.transforms.LegalizeClocksAndAsyncResetsTransform], Dependency[firrtl.transforms.FlattenRegUpdate] ) diff --git a/src/main/scala/firrtl/passes/VerilogPrep.scala b/src/main/scala/firrtl/passes/VerilogPrep.scala index 9499889a19..2bd17519f3 100644 --- a/src/main/scala/firrtl/passes/VerilogPrep.scala +++ b/src/main/scala/firrtl/passes/VerilogPrep.scala @@ -29,7 +29,7 @@ object VerilogPrep extends Pass { Dependency[firrtl.transforms.ReplaceTruncatingArithmetic], Dependency[firrtl.transforms.InlineBitExtractionsTransform], Dependency[firrtl.transforms.InlineAcrossCastsTransform], - Dependency[firrtl.transforms.LegalizeClocksTransform], + Dependency[firrtl.transforms.LegalizeClocksAndAsyncResetsTransform], Dependency[firrtl.transforms.FlattenRegUpdate], Dependency(passes.VerilogModulusCleanup), Dependency[firrtl.transforms.VerilogRename] diff --git a/src/main/scala/firrtl/passes/ZeroWidth.scala b/src/main/scala/firrtl/passes/ZeroWidth.scala index ab1cf7bbd3..057f85a677 100644 --- a/src/main/scala/firrtl/passes/ZeroWidth.scala +++ b/src/main/scala/firrtl/passes/ZeroWidth.scala @@ -204,6 +204,6 @@ object ZeroWidth extends Transform with DependencyAPIMigration { val renames = MutableRenameMap() renames.setCircuit(c.main) val result = c.copy(modules = c.modules.map(onModule(renames))) - CircuitState(result, outputForm, state.annotations, Some(renames)) + state.copy(circuit = result, renames = Some(renames)) } } diff --git a/src/main/scala/firrtl/stage/Forms.scala b/src/main/scala/firrtl/stage/Forms.scala index 44ad25cdca..f36e8f879b 100644 --- a/src/main/scala/firrtl/stage/Forms.scala +++ b/src/main/scala/firrtl/stage/Forms.scala @@ -112,7 +112,7 @@ object Forms { Dependency[firrtl.transforms.ReplaceTruncatingArithmetic], Dependency[firrtl.transforms.InlineBitExtractionsTransform], Dependency[firrtl.transforms.InlineAcrossCastsTransform], - Dependency[firrtl.transforms.LegalizeClocksTransform], + Dependency[firrtl.transforms.LegalizeClocksAndAsyncResetsTransform], Dependency[firrtl.transforms.FlattenRegUpdate], Dependency(passes.VerilogModulusCleanup), Dependency[firrtl.transforms.VerilogRename], diff --git a/src/main/scala/firrtl/transforms/FlattenRegUpdate.scala b/src/main/scala/firrtl/transforms/FlattenRegUpdate.scala index 3f497c91cc..f2dffc4c5b 100644 --- a/src/main/scala/firrtl/transforms/FlattenRegUpdate.scala +++ b/src/main/scala/firrtl/transforms/FlattenRegUpdate.scala @@ -171,7 +171,7 @@ class FlattenRegUpdate extends Transform with DependencyAPIMigration { Dependency[ReplaceTruncatingArithmetic], Dependency[InlineBitExtractionsTransform], Dependency[InlineAcrossCastsTransform], - Dependency[LegalizeClocksTransform] + Dependency[LegalizeClocksAndAsyncResetsTransform] ) override def optionalPrerequisites = firrtl.stage.Forms.LowFormOptimized diff --git a/src/main/scala/firrtl/transforms/GroupComponents.scala b/src/main/scala/firrtl/transforms/GroupComponents.scala index ae1104140d..c2a79d536b 100644 --- a/src/main/scala/firrtl/transforms/GroupComponents.scala +++ b/src/main/scala/firrtl/transforms/GroupComponents.scala @@ -336,13 +336,6 @@ class GroupComponents extends Transform with DependencyAPIMigration { } def onStmt(stmt: Statement): Unit = stmt match { case w: WDefInstance => - case h: IsDeclaration => - bidirGraph.addVertex(h.name) - h.map(onExpr(WRef(h.name))) - case Attach(_, exprs) => // Add edge between each expression - exprs.tail.map(onExpr(getWRef(exprs.head))) - case Connect(_, loc, expr) => - onExpr(getWRef(loc))(expr) case q @ Stop(_, _, clk, en) => val simName = simNamespace.newTemp simulations(simName) = q @@ -351,6 +344,13 @@ class GroupComponents extends Transform with DependencyAPIMigration { val simName = simNamespace.newTemp simulations(simName) = q (args :+ clk :+ en).map(onExpr(WRef(simName))) + case h: IsDeclaration => + bidirGraph.addVertex(h.name) + h.map(onExpr(WRef(h.name))) + case Attach(_, exprs) => // Add edge between each expression + exprs.tail.map(onExpr(getWRef(exprs.head))) + case Connect(_, loc, expr) => + onExpr(getWRef(loc))(expr) case Block(stmts) => stmts.foreach(onStmt) case ignore @ (_: IsInvalid | EmptyStmt) => // do nothing case other => throw new Exception(s"Unexpected Statement $other") diff --git a/src/main/scala/firrtl/transforms/InlineAcrossCastsTransform.scala b/src/main/scala/firrtl/transforms/InlineAcrossCastsTransform.scala index 24c792d7b3..16e148ca64 100644 --- a/src/main/scala/firrtl/transforms/InlineAcrossCastsTransform.scala +++ b/src/main/scala/firrtl/transforms/InlineAcrossCastsTransform.scala @@ -97,7 +97,7 @@ class InlineAcrossCastsTransform extends Transform with DependencyAPIMigration { override def optionalPrerequisiteOf = Seq.empty override def invalidates(a: Transform): Boolean = a match { - case _: LegalizeClocksTransform => true + case _: LegalizeClocksAndAsyncResetsTransform => true case _ => false } diff --git a/src/main/scala/firrtl/transforms/RemoveKeywordCollisions.scala b/src/main/scala/firrtl/transforms/RemoveKeywordCollisions.scala index 69d4aa8ddc..6e2c9a4a07 100644 --- a/src/main/scala/firrtl/transforms/RemoveKeywordCollisions.scala +++ b/src/main/scala/firrtl/transforms/RemoveKeywordCollisions.scala @@ -38,7 +38,7 @@ class VerilogRename extends RemoveKeywordCollisions(v_keywords) { Dependency[ReplaceTruncatingArithmetic], Dependency[InlineBitExtractionsTransform], Dependency[InlineAcrossCastsTransform], - Dependency[LegalizeClocksTransform], + Dependency[LegalizeClocksAndAsyncResetsTransform], Dependency[FlattenRegUpdate], Dependency(passes.VerilogModulusCleanup) ) diff --git a/src/main/scala/logger/LoggerOptions.scala b/src/main/scala/logger/LoggerOptions.scala index bfd072df24..e493a25251 100644 --- a/src/main/scala/logger/LoggerOptions.scala +++ b/src/main/scala/logger/LoggerOptions.scala @@ -32,7 +32,7 @@ class LoggerOptions private[logger] ( } /** Return the name of the log file, defaults to `a.log` if unspecified */ - def getLogFileName(): Option[String] = if (!logToFile()) None else logFileName.orElse(Some("a.log")) + def getLogFileName(): Option[String] = logFileName.orElse(Some("a.log")) /** True if a [[Logger]] should be writing to a file */ @deprecated("logToFile was removed, use logFileName.nonEmpty", "FIRRTL 1.2") From ebad877af9d9fe43a92f64f8b63ca8904834cc4d Mon Sep 17 00:00:00 2001 From: Jiuyang Liu Date: Tue, 28 Dec 2021 03:13:05 +0800 Subject: [PATCH 3/5] disable random should not contain useInitAsPreset. (#2449) --- src/main/scala/firrtl/Emitter.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/firrtl/Emitter.scala b/src/main/scala/firrtl/Emitter.scala index 98bb412787..82b033b658 100644 --- a/src/main/scala/firrtl/Emitter.scala +++ b/src/main/scala/firrtl/Emitter.scala @@ -200,7 +200,7 @@ object EmitAllModulesAnnotation extends HasShellOptions { s.split(",") .map { case "disableMemRandomization" => - CustomDefaultRegisterEmission(useInitAsPreset = true, disableRandomization = true) + CustomDefaultRegisterEmission(useInitAsPreset = false, disableRandomization = true) case "disableRegisterRandomization" => CustomDefaultMemoryEmission(MemoryNoInit) case a => throw new PhaseException(s"Unknown emission options '$a'! (Did you misspell it?)") } From 3e494b5cceda14a73f288a293dd007a82be18bb8 Mon Sep 17 00:00:00 2001 From: sinofp Date: Thu, 6 Jan 2022 04:17:52 +0000 Subject: [PATCH 4/5] Add FileInfo to asyncResetAlwaysBlocks (#2451) * Add FileInfo to asyncResetAlwaysBlocks Always blocks need three FileInfo (if, true, false) to show line numbers, but initially, every always blocks only have one FileInfo (false). RemoveReset adds the extra two FileInfo to sync always blocks, so sync always blocks can have line numbers. Async always blocks don't provide their only FileInfo, so there are no line numbers. This commit gives async always block the extra FileInfo to show line numbers for them. This code: ```scala import chisel3._ import chisel3.stage._ import firrtl.CustomDefaultRegisterEmission class Test extends Module with RequireAsyncReset { val io = IO(new Bundle { val in = Input(Bool()) val out = Output(Bool()) }) val valid = RegInit(false.B) valid := io.in io.out := valid } object Test extends App { new ChiselStage().execute(Array(), Seq( ChiselGeneratorAnnotation(() => new Test()), CustomDefaultRegisterEmission(useInitAsPreset = false, disableRandomization = true) )) } ``` will generate this Verilog: ```verilog module Test( input clock, input reset, input io_in, output io_out ); reg valid; // @[Playground.scala 10:22] assign io_out = valid; // @[Playground.scala 12:10] always @(posedge clock or posedge reset) begin if (reset) begin // @[Playground.scala 10:22] valid <= 1'h0; // @[Playground.scala 10:22] end else begin valid <= io_in; // @[Playground.scala 11:9] end end endmodule ``` they have correct line numbers (10, 10, 11). * Add test for async always block line numbers * Add comment for review --- .../firrtl/backends/verilog/VerilogEmitter.scala | 5 ++--- .../scala/firrtl/transforms/RemoveReset.scala | 10 ++++++++++ .../scala/firrtlTests/VerilogEmitterTests.scala | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala b/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala index af8996ebfe..f48e4846a6 100644 --- a/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala +++ b/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala @@ -787,13 +787,12 @@ class VerilogEmitter extends SeqTransform with Emitter { } else { // Asynchronous Reset assert(reset.tpe == AsyncResetType, "Error! Synchronous reset should have been removed!") val tv = init - val InfoExpr(finfo, fv) = netlist(r) - // TODO add register info argument and build a MultiInfo to pass + val InfoExpr(info, fv) = netlist(r) asyncResetAlwaysBlocks += ( ( clk, reset, - addUpdate(NoInfo, Mux(reset, tv, fv, mux_type_and_widths(tv, fv)), Seq.empty) + addUpdate(info, Mux(reset, tv, fv, mux_type_and_widths(tv, fv)), Seq.empty) ) ) } diff --git a/src/main/scala/firrtl/transforms/RemoveReset.scala b/src/main/scala/firrtl/transforms/RemoveReset.scala index 62b341cdf7..f1434ad297 100644 --- a/src/main/scala/firrtl/transforms/RemoveReset.scala +++ b/src/main/scala/firrtl/transforms/RemoveReset.scala @@ -50,6 +50,7 @@ object RemoveReset extends Transform with DependencyAPIMigration { private def onModule(m: DefModule, isPreset: String => Boolean): DefModule = { val resets = mutable.HashMap.empty[String, Reset] + val asyncResets = mutable.HashMap.empty[String, Reset] val invalids = computeInvalids(m) def onStmt(stmt: Statement): Statement = { stmt match { @@ -77,12 +78,21 @@ object RemoveReset extends Transform with DependencyAPIMigration { // Add register reset to map resets(rname) = Reset(reset, init, info) reg.copy(reset = Utils.zero, init = WRef(reg)) + case reg @ DefRegister(info, rname, _, _, reset, init) if reset.tpe == AsyncResetType => + asyncResets(rname) = Reset(reset, init, info) + reg case Connect(info, ref @ WRef(rname, _, RegKind, _), expr) if resets.contains(rname) => val reset = resets(rname) val muxType = Utils.mux_type_and_widths(reset.value, expr) // Use reg source locator for mux enable and true value since that's where they're defined val infox = MultiInfo(reset.info, reset.info, info) Connect(infox, ref, Mux(reset.cond, reset.value, expr, muxType)) + case Connect(info, ref @ WRef(rname, _, RegKind, _), expr) if asyncResets.contains(rname) => + val reset = asyncResets(rname) + // The `muxType` for async always blocks is located in [[VerilogEmitter.VerilogRender.regUpdate]]: + // addUpdate(info, Mux(reset, tv, fv, mux_type_and_widths(tv, fv)), Seq.empty) + val infox = MultiInfo(reset.info, reset.info, info) + Connect(infox, ref, expr) case other => other.map(onStmt) } } diff --git a/src/test/scala/firrtlTests/VerilogEmitterTests.scala b/src/test/scala/firrtlTests/VerilogEmitterTests.scala index 4c09c083c9..1a40cc8fcb 100644 --- a/src/test/scala/firrtlTests/VerilogEmitterTests.scala +++ b/src/test/scala/firrtlTests/VerilogEmitterTests.scala @@ -717,6 +717,22 @@ class VerilogEmitterSpec extends FirrtlFlatSpec { result should containLine("assign z = x == y;") } + it should "show line numbers for AsyncReset regUpdate" in { + val result = compileBody( + """input clock : Clock + |input reset : AsyncReset + |output io : { flip in : UInt<1>, out : UInt<1>} + | + |reg valid : UInt<1>, clock with : + | reset => (reset, UInt<1>("h0")) @[Playground.scala 11:22] + |valid <= io.in @[Playground.scala 12:9] + |io.out <= valid @[Playground.scala 13:10]""".stripMargin + ) + result should containLine("if (reset) begin // @[Playground.scala 11:22]") + result should containLine("valid <= 1'h0; // @[Playground.scala 11:22]") + result should containLine("valid <= io_in; // @[Playground.scala 12:9]") + } + it should "subtract positive literals instead of adding negative literals" in { val compiler = new VerilogCompiler val result = compileBody( From d1ba3b0c23886e4f7e11625f1458fb90b405effc Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 11 Jan 2022 18:23:46 -0800 Subject: [PATCH 5/5] Update .mergify.yml (#2454) --- .mergify.yml | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/.mergify.yml b/.mergify.yml index 5e77c8d945..1d6998658e 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -1,3 +1,7 @@ +queue_rules: +- name: default + conditions: + - status-success=all tests passed pull_request_rules: - name: automatic squash-and-merge on CI success and review conditions: @@ -9,10 +13,10 @@ pull_request_rules: - label!="DO NOT MERGE" - label!="bp-conflict" actions: - merge: + queue: + name: default method: squash - strict: smart - strict_method: merge + update_method: merge - name: backport to 1.4.x conditions: - merged @@ -73,10 +77,10 @@ pull_request_rules: - label!="DO NOT MERGE" - label!="bp-conflict" actions: - merge: + queue: + name: default method: squash - strict: smart - strict_method: merge + update_method: merge - name: automatic squash-and-mege of 1.3.x backport PRs conditions: - status-success=all tests passed @@ -86,10 +90,10 @@ pull_request_rules: - label!="DO NOT MERGE" - label!="bp-conflict" actions: - merge: + queue: + name: default method: squash - strict: smart - strict_method: merge + update_method: merge - name: automatic squash-and-mege of 1.4.x backport PRs conditions: - status-success=all tests passed @@ -99,8 +103,8 @@ pull_request_rules: - label!="DO NOT MERGE" - label!="bp-conflict" actions: - merge: + queue: + name: default method: squash - strict: smart - strict_method: merge + update_method: merge