From e45a9e03858d1879bd0783bfbf32ae70849de952 Mon Sep 17 00:00:00 2001 From: Will Dietz Date: Wed, 17 Apr 2024 10:11:11 -0500 Subject: [PATCH 1/2] Add ifElseFatal intrinsics; use it for chisel3.assert emission. Restore #3912 and #3825. Use intrinsic expression instead of intrinsic module. This reverts commit 347d82b8c1c41f8cfd26d07004491e6bc0c549fb. --- core/src/main/scala/chisel3/BlackBox.scala | 3 + core/src/main/scala/chisel3/Printable.scala | 13 +++++ .../scala/chisel3/VerificationStatement.scala | 57 +++++++------------ .../experimental/VerificationIntrinsics.scala | 27 +++++++++ .../chisel3/internal/firrtl/Converter.scala | 9 ++- .../scala/chiselTests/VerificationSpec.scala | 15 +++-- .../chiselTests/naming/NamePluginSpec.scala | 2 - .../scala/chiselTests/naming/PrefixSpec.scala | 6 +- 8 files changed, 80 insertions(+), 52 deletions(-) create mode 100644 core/src/main/scala/chisel3/experimental/VerificationIntrinsics.scala diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala index 0d3841a08c9..4fa9eea0b03 100644 --- a/core/src/main/scala/chisel3/BlackBox.scala +++ b/core/src/main/scala/chisel3/BlackBox.scala @@ -28,6 +28,9 @@ package experimental { case class DoubleParam(value: Double) extends Param case class StringParam(value: String) extends Param + /** Creates a parameter from the Printable's resulting format String */ + case class PrintableParam(value: chisel3.Printable, context: BaseModule) extends Param + /** Unquoted String */ case class RawParam(value: String) extends Param diff --git a/core/src/main/scala/chisel3/Printable.scala b/core/src/main/scala/chisel3/Printable.scala index 370055b5e35..d7646ae5bfd 100644 --- a/core/src/main/scala/chisel3/Printable.scala +++ b/core/src/main/scala/chisel3/Printable.scala @@ -51,6 +51,10 @@ sealed abstract class Printable { */ def unpack(ctx: Component): (String, Iterable[String]) + /** Unpack into a Seq of captured Bits arguments + */ + def unpackArgs: Seq[Bits] + /** Allow for appending Printables like Strings */ final def +(that: Printable): Printables = Printables(List(this, that)) @@ -155,12 +159,16 @@ case class Printables(pables: Iterable[Printable]) extends Printable { val (fmts, args) = pables.map(_.unpack(ctx)).unzip (fmts.mkString, args.flatten) } + + final def unpackArgs: Seq[Bits] = pables.view.flatMap(_.unpackArgs).toList } /** Wrapper for printing Scala Strings */ case class PString(str: String) extends Printable { final def unpack(ctx: Component): (String, Iterable[String]) = (str.replaceAll("%", "%%"), List.empty) + + final def unpackArgs: Seq[Bits] = List.empty } /** Superclass for Firrtl format specifiers for Bits */ @@ -169,6 +177,8 @@ sealed abstract class FirrtlFormat(private[chisel3] val specifier: Char) extends def unpack(ctx: Component): (String, Iterable[String]) = { (s"%$specifier", List(bits.ref.fullName(ctx))) } + + def unpackArgs: Seq[Bits] = List(bits) } object FirrtlFormat { final val legalSpecifiers = List('d', 'x', 'b', 'c') @@ -209,14 +219,17 @@ case class Character(bits: Bits) extends FirrtlFormat('c') /** Put innermost name (eg. field of bundle) */ case class Name(data: Data) extends Printable { final def unpack(ctx: Component): (String, Iterable[String]) = (data.ref.name, List.empty) + final def unpackArgs: Seq[Bits] = List.empty } /** Put full name within parent namespace (eg. bundleName.field) */ case class FullName(data: Data) extends Printable { final def unpack(ctx: Component): (String, Iterable[String]) = (data.ref.fullName(ctx), List.empty) + final def unpackArgs: Seq[Bits] = List.empty } /** Represents escaped percents */ case object Percent extends Printable { final def unpack(ctx: Component): (String, Iterable[String]) = ("%%", List.empty) + final def unpackArgs: Seq[Bits] = List.empty } diff --git a/core/src/main/scala/chisel3/VerificationStatement.scala b/core/src/main/scala/chisel3/VerificationStatement.scala index 4b1903a7dd4..96aa6e61f64 100644 --- a/core/src/main/scala/chisel3/VerificationStatement.scala +++ b/core/src/main/scala/chisel3/VerificationStatement.scala @@ -7,7 +7,8 @@ import scala.language.experimental.macros import chisel3.internal._ import chisel3.internal.Builder.pushCommand import chisel3.internal.firrtl.ir._ -import chisel3.experimental.SourceInfo +import chisel3.experimental.{BaseModule, SourceInfo} +import chisel3.util.circt.IfElseFatalIntrinsic import scala.annotation.nowarn import scala.reflect.macros.blackbox @@ -56,10 +57,6 @@ object assert extends VerifPrintMacrosDoc { * Does not fire when in reset (defined as the current implicit reset, e.g. as set by * the enclosing `withReset` or Module.reset. * - * May be called outside of a Module (like defined in a function), so - * functions using assert make the standard Module assumptions (single clock - * and single reset). - * * @param cond condition, assertion fires (simulation fails) on a rising clock edge when false and reset is not asserted * @param message optional chisel Printable type message * @@ -139,6 +136,19 @@ object assert extends VerifPrintMacrosDoc { q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.None)($sourceInfo)" } + private def emitIfElseFatalIntrinsic( + clock: Clock, + predicate: Bool, + enable: Bool, + format: Printable + )( + implicit sourceInfo: SourceInfo + ): Assert = { + val id = Builder.forcedUserModule // It should be safe since we push commands anyway. + IfElseFatalIntrinsic(id, format, "chisel3_builtin", clock, predicate, enable, format.unpackArgs: _*)(sourceInfo) + new Assert() + } + /** This will be removed in Chisel 3.6 in favor of the Printable version * * @group VerifPrintMacros @@ -151,12 +161,8 @@ object assert extends VerifPrintMacrosDoc { )( implicit sourceInfo: SourceInfo ): Assert = { - val id = new Assert() - when(!Module.reset.asBool) { - failureMessage("Assertion", line, cond, message.map(Printable.pack(_, data: _*))) - Builder.pushCommand(Verification(id, Formal.Assert, sourceInfo, Module.clock.ref, cond.ref, "")) - } - id + val pable = formatFailureMessage("Assertion", line, cond, message.map(Printable.pack(_, data: _*))) + emitIfElseFatalIntrinsic(Module.clock, cond, !Module.reset.asBool, pable) } /** @group VerifPrintMacros */ @@ -167,13 +173,9 @@ object assert extends VerifPrintMacrosDoc { )( implicit sourceInfo: SourceInfo ): Assert = { - val id = new Assert() message.foreach(Printable.checkScope(_)) - when(!Module.reset.asBool) { - failureMessage("Assertion", line, cond, message) - Builder.pushCommand(Verification(id, Formal.Assert, sourceInfo, Module.clock.ref, cond.ref, "")) - } - id + val pable = formatFailureMessage("Assertion", line, cond, message) + emitIfElseFatalIntrinsic(Module.clock, cond, !Module.reset.asBool, pable) } } @@ -214,10 +216,6 @@ object assume extends VerifPrintMacrosDoc { * reset). If your definition of reset is not the encapsulating Module's * reset, you will need to gate this externally. * - * May be called outside of a Module (like defined in a function), so - * functions using assert make the standard Module assumptions (single clock - * and single reset). - * * @param cond condition, assertion fires (simulation fails) when false * @param message optional Printable type message when the assertion fires * @@ -342,10 +340,6 @@ object cover extends VerifPrintMacrosDoc { * reset). If your definition of reset is not the encapsulating Module's * reset, you will need to gate this externally. * - * May be called outside of a Module (like defined in a function), so - * functions using assert make the standard Module assumptions (single clock - * and single reset). - * * @param cond condition that will be sampled on every clock tick * @param message a string describing the cover event */ @@ -462,17 +456,4 @@ private object VerificationStatement { case None => p"$kind failed\n at $lineMsg\n" } } - - def failureMessage( - kind: String, - lineInfo: SourceLineInfo, - cond: Bool, - message: Option[Printable] - )( - implicit sourceInfo: SourceInfo - ): Unit = { - when(!cond) { - printf.printfWithoutReset(formatFailureMessage(kind, lineInfo, cond, message)) - } - } } diff --git a/core/src/main/scala/chisel3/experimental/VerificationIntrinsics.scala b/core/src/main/scala/chisel3/experimental/VerificationIntrinsics.scala new file mode 100644 index 00000000000..fa131260672 --- /dev/null +++ b/core/src/main/scala/chisel3/experimental/VerificationIntrinsics.scala @@ -0,0 +1,27 @@ +package chisel3.util.circt + +import chisel3._ +import chisel3.internal._ +import chisel3.experimental.{BaseModule, SourceInfo} + +/** Create an `ifelsefatal` style assertion. + */ +private[chisel3] object IfElseFatalIntrinsic { + def apply( + id: BaseModule, + format: Printable, + label: String, + clock: Clock, + predicate: Bool, + enable: Bool, + data: Data* + )( + implicit sourceInfo: SourceInfo + ): Unit = { + Intrinsic( + "circt_chisel_ifelsefatal", + "format" -> chisel3.experimental.PrintableParam(format, id), + "label" -> label + )((Seq(clock, predicate, enable) ++ data): _*) + } +} diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index 09bd8471257..320148ccc99 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -11,7 +11,7 @@ import chisel3.internal.{castToInt, throwException, HasId} import chisel3.internal.firrtl.ir._ import chisel3.EnumType import scala.annotation.tailrec -import scala.collection.immutable.{Queue, VectorBuilder} +import scala.collection.immutable.{Queue, VectorBuilder, VectorMap} private[chisel3] object Converter { // TODO modeled on unpack method on Printable, refactor? @@ -463,7 +463,12 @@ private[chisel3] object Converter { case IntParam(value) => fir.IntParam(name, value) case DoubleParam(value) => fir.DoubleParam(name, value) case StringParam(value) => fir.StringParam(name, fir.StringLit(value)) - case RawParam(value) => fir.RawStringParam(name, value) + case PrintableParam(value, id) => { + val ctx = id._component.get + val (fmt, _) = unpack(value, ctx) + fir.StringParam(name, fir.StringLit(fmt)) + } + case RawParam(value) => fir.RawStringParam(name, value) } // TODO: Modify Panama CIRCT to account for type aliasing information. This is a temporary hack to diff --git a/src/test/scala/chiselTests/VerificationSpec.scala b/src/test/scala/chiselTests/VerificationSpec.scala index 1b62e74bc03..e535f8d9c05 100644 --- a/src/test/scala/chiselTests/VerificationSpec.scala +++ b/src/test/scala/chiselTests/VerificationSpec.scala @@ -17,7 +17,7 @@ class SimpleTest extends Module { cover(io.in === 3.U) when(io.in === 3.U) { assume(io.in =/= 2.U) - assert(io.out === io.in) + assert(io.out === io.in, p"${FullName(io.in)}:${io.in} is equal to ${FullName(io.out)}:${io.out}") } } @@ -30,16 +30,19 @@ class VerificationSpec extends ChiselPropSpec with Matchers { property("basic equality check should work") { val fir = ChiselStage.emitCHIRRTL(new SimpleTest) - val lines = fir.split("\n").map(_.trim).toIndexedSeq + val lines = fir.split("(\n|@)").map(_.trim).toIndexedSeq // reset guard around the verification statement - assertContains(lines, "when _T_1 : ") + assertContains(lines, "when _T_1 :") assertContains(lines, "cover(clock, _T, UInt<1>(0h1), \"\")") assertContains(lines, "assume(clock, _T_3, UInt<1>(0h1), \"Assumption failed") - - assertContains(lines, "when _T_7 : ") - assertContains(lines, "assert(clock, _T_5, UInt<1>(0h1), \"\")") + assertContains(lines, "node _T_5 = eq(io.out, io.in)") + assertContains(lines, "node _T_6 = eq(reset, UInt<1>(0h0))") + assertContains( + lines, + "intrinsic(circt_chisel_ifelsefatal, clock, _T_5, _T_6, io.in, io.out)" + ) } property("annotation of verification constructs should work") { diff --git a/src/test/scala/chiselTests/naming/NamePluginSpec.scala b/src/test/scala/chiselTests/naming/NamePluginSpec.scala index 248bab9207c..2e271c5e7c3 100644 --- a/src/test/scala/chiselTests/naming/NamePluginSpec.scala +++ b/src/test/scala/chiselTests/naming/NamePluginSpec.scala @@ -75,14 +75,12 @@ class NamePluginSpec extends ChiselFlatSpec with Utils { val foo, bar = IO(Input(UInt(8.W))) { - val x1 = chisel3.assert(1.U === 1.U) val x2 = cover(foo =/= bar) val x3 = chisel3.assume(foo =/= 123.U) val x4 = printf("foo = %d\n", foo) } } val chirrtl = ChiselStage.emitCHIRRTL(new Test) - (chirrtl should include).regex("assert.*: x1") (chirrtl should include).regex("cover.*: x2") (chirrtl should include).regex("assume.*: x3") (chirrtl should include).regex("printf.*: x4") diff --git a/src/test/scala/chiselTests/naming/PrefixSpec.scala b/src/test/scala/chiselTests/naming/PrefixSpec.scala index 5b4171f729b..bc8030a34bf 100644 --- a/src/test/scala/chiselTests/naming/PrefixSpec.scala +++ b/src/test/scala/chiselTests/naming/PrefixSpec.scala @@ -460,17 +460,15 @@ class PrefixSpec extends ChiselPropSpec with Utils { { val x5 = { - val x1 = chisel3.assert(1.U === 1.U) val x2 = cover(foo =/= bar) val x3 = chisel3.assume(foo =/= 123.U) val x4 = printf("foo = %d\n", foo) - x1 + x2 } } } val chirrtl = ChiselStage.emitCHIRRTL(new Test) - (chirrtl should include).regex("assert.*: x5") - (chirrtl should include).regex("cover.*: x5_x2") + (chirrtl should include).regex("cover.*: x5") (chirrtl should include).regex("assume.*: x5_x3") (chirrtl should include).regex("printf.*: x5_x4") } From 30b1bd43454a66fb9c69dd4812669eb286edbaf2 Mon Sep 17 00:00:00 2001 From: Will Dietz Date: Wed, 17 Apr 2024 17:17:29 -0500 Subject: [PATCH 2/2] raw-quote and less escaping --- src/test/scala/chiselTests/VerificationSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/scala/chiselTests/VerificationSpec.scala b/src/test/scala/chiselTests/VerificationSpec.scala index e535f8d9c05..edcfa37ccbc 100644 --- a/src/test/scala/chiselTests/VerificationSpec.scala +++ b/src/test/scala/chiselTests/VerificationSpec.scala @@ -41,7 +41,7 @@ class VerificationSpec extends ChiselPropSpec with Matchers { assertContains(lines, "node _T_6 = eq(reset, UInt<1>(0h0))") assertContains( lines, - "intrinsic(circt_chisel_ifelsefatal, clock, _T_5, _T_6, io.in, io.out)" + """intrinsic(circt_chisel_ifelsefatal, clock, _T_5, _T_6, io.in, io.out)""" ) }