Support Printable SimLog filenames (#4892)

All runtime-varying filenames via substitutions.

Author: jackkoenig

core/src/main/scala/chisel3/Data.scala

@@ -694,12 +694,14 @@ abstract class Data extends HasId with NamedComponent with DataIntf {
   }
   private[chisel3] def visibleFromBlock: Option[SourceInfo] = MonoConnect.checkBlockVisibility(this)
   private[chisel3] def requireVisible()(implicit info: SourceInfo): Unit = {
+    this.checkVisible.foreach(err => Builder.error(err))
+  }
+  // Some is an error message, None means no error
+  private[chisel3] def checkVisible(implicit info: SourceInfo): Option[String] = {
     if (!isVisibleFromModule) {
-      throwException(s"operand '$this' is not visible from the current module ${Builder.currentModule.get.name}")
-    }
-    visibleFromBlock match {
-      case Some(blockInfo) => MonoConnect.escapedScopeError(this, blockInfo)
-      case None            => ()
+      Some(s"operand '$this' is not visible from the current module ${Builder.currentModule.get.name}")
+    } else {
+      visibleFromBlock.map(MonoConnect.escapedScopeErrorMsg(this, _))
     }
   }
 

core/src/main/scala/chisel3/Printable.scala

@@ -3,6 +3,7 @@
 package chisel3
 
 import chisel3.experimental.SourceInfo
+import chisel3.internal.Builder
 import chisel3.internal.firrtl.ir.Component
 import chisel3.FirrtlFormat.FormatWidth
 
@@ -167,7 +168,7 @@ object Printable {
     }
   }
 
-  private[chisel3] def checkScope(message: Printable)(implicit info: SourceInfo): Unit = {
+  private[chisel3] def checkScope(message: Printable, msg: String = "")(implicit info: SourceInfo): Unit = {
     def getData(x: Printable): Seq[Data] = {
       x match {
         case y: FirrtlFormat => Seq(y.bits)
@@ -177,7 +178,11 @@ object Printable {
         case _             => Seq() // Handles subtypes PString and Percent
       }
     }
-    getData(message).foreach(_.requireVisible())
+    for (data <- getData(message)) {
+      for (err <- data.checkVisible) {
+        Builder.error(msg + err)
+      }
+    }
   }
 
   /** Extract the Data that will be arguments to Firrtl

core/src/main/scala/chisel3/SimLog.scala

@@ -8,10 +8,19 @@ import chisel3.internal.Builder
 import chisel3.experimental.SourceInfo
 
 /** A file or I/O device to print to in simulation
-  * 
+  *
   * {{{
-  * val fd = SimLog.file("logfile.log")
-  * fd.printf(cf"in = $in%0d\n")
+  * val log = SimLog.file("logfile.log")
+  * log.printf(cf"in = $in%0d\n")
+  * 
+  * val stderr = SimLog.StdErr
+  * stderr.printf(cf"in = $in%0d\n")
+  * 
+  * // SimLog filenames themselves can be Printable.
+  * // Be careful to avoid uninitialized registers.
+  * val idx = Wire(UInt(8.W))
+  * val log2 = SimLog.file(cf"logfile_$idx%0d.log")
+  * log2.printf(cf"in = $in%0d\n")
   * }}}
   */
 sealed trait SimLog extends SimLogIntf {
@@ -34,8 +43,8 @@ sealed trait SimLog extends SimLogIntf {
     this.printfWithReset(pable)(sourceInfo)
   }
 
-  // Eventually this might be a richer type but for now Some[String] is filename, None is Stderr
-  protected def _filename: Option[String]
+  // Eventually this might be a richer type but for now Some[Printable] is filename, None is Stderr
+  protected def _filename: Option[Printable]
 
   private[chisel3] def printfWithReset(
     pable: Printable
@@ -57,7 +66,8 @@ sealed trait SimLog extends SimLogIntf {
     val clock = Builder.forcedClock
     val printfId = new chisel3.printf.Printf(pable)
 
-    Printable.checkScope(pable)
+    Printable.checkScope(pable, "printf ")
+    _filename.foreach(Printable.checkScope(_, "SimLog filename "))
 
     layer.block(layers.Verification, skipIfAlreadyInBlock = true, skipIfLayersEnabled = true) {
       Builder.pushCommand(Printf(printfId, sourceInfo, _filename, clock.ref, pable))
@@ -74,21 +84,25 @@ sealed trait SimLog extends SimLogIntf {
     printfWithoutReset(Printable.pack(fmt, data: _*))
 }
 
-// Uses firrtl fprintf with a String filename
-private case class SimLogFile(filename: String) extends SimLog {
-  override protected def _filename: Option[String] = Some(filename)
+// Uses firrtl fprintf with a Printable filename
+private case class SimLogFile(filename: Printable) extends SimLog {
+  override protected def _filename: Option[Printable] = Some(filename)
 }
 
 // Defaults to firrtl printf
 private object StdErrSimLog extends SimLog {
-  override protected def _filename: Option[String] = None
+  override protected def _filename: Option[Printable] = None
 }
 
 object SimLog {
 
   /** Print to a file given by `filename`
     */
   def file(filename: String)(implicit sourceInfo: SourceInfo): SimLog = {
+    new SimLogFile(PString(filename))
+  }
+
+  def file(filename: Printable)(implicit sourceInfo: SourceInfo): SimLog = {
     new SimLogFile(filename)
   }
 

core/src/main/scala/chisel3/internal/MonoConnect.scala

@@ -52,9 +52,8 @@ private[chisel3] object MonoConnect {
     MonoConnectException(
       s"""${formatName(sink)} cannot be written from module ${source.parentNameOpt.getOrElse("(unknown)")}."""
     )
-  def escapedScopeError(data: Data, blockInfo: SourceInfo)(implicit lineInfo: SourceInfo): Unit = {
-    val msg = s"'$data' has escaped the scope of the block (${blockInfo.makeMessage()}) in which it was constructed."
-    Builder.error(msg)
+  def escapedScopeErrorMsg(data: Data, blockInfo: SourceInfo) = {
+    s"'$data' has escaped the scope of the block (${blockInfo.makeMessage()}) in which it was constructed."
   }
   def UnknownRelationException =
     MonoConnectException("Sink or source unavailable to current module.")
@@ -284,12 +283,12 @@ private[chisel3] object MonoConnect {
     }
 
     checkBlockVisibility(sink) match {
-      case Some(blockInfo) => escapedScopeError(sink, blockInfo)
+      case Some(blockInfo) => Builder.error(escapedScopeErrorMsg(sink, blockInfo))
       case None            => ()
     }
 
     checkBlockVisibility(source) match {
-      case Some(blockInfo) => escapedScopeError(source, blockInfo)
+      case Some(blockInfo) => Builder.error(escapedScopeErrorMsg(source, blockInfo))
       case None            => ()
     }
 
@@ -516,7 +515,7 @@ private[chisel3] object checkConnect {
     // Import helpers and exception types.
     import MonoConnect.{
       checkBlockVisibility,
-      escapedScopeError,
+      escapedScopeErrorMsg,
       UnknownRelationException,
       UnreadableSourceException,
       UnwritableSinkException
@@ -594,12 +593,12 @@ private[chisel3] object checkConnect {
     else throw UnknownRelationException
 
     checkBlockVisibility(sink) match {
-      case Some(blockInfo) => escapedScopeError(sink, blockInfo)(sourceInfo)
+      case Some(blockInfo) => Builder.error(escapedScopeErrorMsg(sink, blockInfo))(sourceInfo)
       case None            => ()
     }
 
     checkBlockVisibility(source) match {
-      case Some(blockInfo) => escapedScopeError(source, blockInfo)(sourceInfo)
+      case Some(blockInfo) => Builder.error(escapedScopeErrorMsg(source, blockInfo))(sourceInfo)
       case None            => ()
     }
   }

core/src/main/scala/chisel3/internal/firrtl/Converter.scala

@@ -187,8 +187,10 @@ private[chisel3] object Converter {
       fir.Stop(convert(info), ret, convert(clock, ctx, info), firrtl.Utils.one, e.name)
     case e @ Printf(_, info, filename, clock, pable) =>
       val mkPrintf = filename match {
-        case None    => fir.Print.apply _
-        case Some(f) => fir.Fprint.apply(_, f, _, _, _, _, _)
+        case None => fir.Print.apply _
+        case Some(f) =>
+          val (ffmt, fargs) = unpack(f, ctx, info)
+          fir.Fprint.apply(_, fir.StringLit(ffmt), fargs.map(a => convert(a, ctx, info)), _, _, _, _, _)
       }
       val (fmt, args) = unpack(pable, ctx, info)
       mkPrintf(

core/src/main/scala/chisel3/internal/firrtl/IR.scala

@@ -492,8 +492,13 @@ private[chisel3] object ir {
 
   case class Port(id: Data, dir: SpecifiedDirection, sourceInfo: SourceInfo)
 
-  case class Printf(id: printf.Printf, sourceInfo: SourceInfo, filename: Option[String], clock: Arg, pable: Printable)
-      extends Definition
+  case class Printf(
+    id:         printf.Printf,
+    sourceInfo: SourceInfo,
+    filename:   Option[Printable],
+    clock:      Arg,
+    pable:      Printable
+  ) extends Definition
 
   case class ProbeDefine(sourceInfo: SourceInfo, sink: Arg, probe: Arg) extends Command
   case class ProbeForceInitial(sourceInfo: SourceInfo, probe: Arg, value: Arg) extends Command

core/src/main/scala/chisel3/internal/firrtl/Serializer.scala

@@ -255,7 +255,11 @@ private[chisel3] object Serializer {
       val (fmt, args) = unpack(pable, ctx, info)
       if (filename.isEmpty) b ++= "printf("; else b ++= "fprintf(";
       serialize(clock, ctx, info); b ++= ", UInt<1>(0h1), ";
-      filename.foreach { f => b ++= "\""; b ++= f; b ++= "\", " }
+      filename.foreach { fable =>
+        val (ffmt, fargs) = unpack(fable, ctx, info)
+        b ++= fir.StringLit(ffmt).escape; b ++= ", "
+        fargs.foreach { a => serialize(a, ctx, info); b ++= ", " }
+      }
       b ++= fir.StringLit(fmt).escape;
       args.foreach { a => b ++= ", "; serialize(a, ctx, info) }; b += ')'
       val lbl = e.name

docs/src/explanations/printing.md

@@ -232,6 +232,23 @@ class MyModule extends Module {
 This is the same as standard `printf`.
 :::
 
+SimLog filenames can themselves be `Printable` values:
+
+```scala mdoc:compile-only
+class MyModule extends Module {
+  val idx = IO(Input(UInt(8.W)))
+  val log = SimLog.file(cf"logfile_$idx%0d.log")
+  val in = IO(Input(UInt(8.W)))
+  log.printf(cf"in = $in%d\n")
+}
+```
+
+It is strongly recommended to use `%0d` with UInts in filenames to avoid spaces in the filename.
+
+:::warning
+Be careful to avoid uninitialized registers in the filename.
+:::
+
 ### Writing Generic Code
 
 `SimLog` allows you to write code that can work with any log destination. This is useful when creating reusable components:

firrtl/src/main/scala/firrtl/ir/IR.scala

@@ -437,13 +437,14 @@ case class Print(
 
 @deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")
 case class Fprint(
-  val info:     Info,
-  val filename: String,
-  val string:   StringLit,
-  val args:     Seq[Expression],
-  val clk:      Expression,
-  val en:       Expression,
-  val name:     String = ""
+  val info:         Info,
+  val filename:     StringLit,
+  val filenameArgs: Seq[Expression],
+  val string:       StringLit,
+  val args:         Seq[Expression],
+  val clk:          Expression,
+  val en:           Expression,
+  val name:         String = ""
 ) extends Statement
     with HasInfo
     with IsDeclaration

firrtl/src/main/scala/firrtl/ir/Serializer.scala

@@ -284,8 +284,10 @@ object Serializer {
       b ++= "printf("; s(clk); b ++= ", "; s(en); b ++= ", "; b ++= string.escape
       if (args.nonEmpty) b ++= ", "; s(args, ", "); b += ')'
       sStmtName(print.name); s(info)
-    case print @ Fprint(info, filename, string, args, clk, en, _) =>
-      b ++= "fprintf("; s(clk); b ++= ", "; s(en); b ++= ", \""; b ++= filename; b ++= "\", "; b ++= string.escape
+    case print @ Fprint(info, filename, fargs, string, args, clk, en, _) =>
+      b ++= "fprintf("; s(clk); b ++= ", "; s(en); b ++= ", "; b ++= filename.escape; b ++= ", "
+      s(fargs, ", "); if (fargs.nonEmpty) b ++= ", ";
+      b ++= string.escape
       if (args.nonEmpty) b ++= ", "; s(args, ", "); b += ')'
       sStmtName(print.name); s(info)
     case IsInvalid(info, expr)    => b ++= "invalidate "; s(expr); s(info)

firrtl/src/test/scala/firrtlTests/SerializerSpec.scala

@@ -412,13 +412,14 @@ class SerializerSpec extends AnyFlatSpec with Matchers {
     Serializer.serialize(
       Fprint(
         NoInfo,
-        "filename",
+        StringLit("filename_%0d"),
+        Seq(Reference("x")),
         StringLit("hello %x"),
         Seq(Reference("arg")),
         Reference("clock"),
         Reference("enable"),
         "label"
       )
-    ) should include("""fprintf(clock, enable, "filename", "hello %x", arg) : label""")
+    ) should include("""fprintf(clock, enable, "filename_%0d", x, "hello %x", arg) : label""")
   }
 }

src/test/scala-2/chiselTests/SimLogSpec.scala

@@ -57,6 +57,34 @@ class SimLogSpec extends AnyFlatSpec with Matchers with FileCheck with ChiselSim
       )
   }
 
+  it should "support Printable filenames" in {
+    class MyModule extends Module {
+      val idx = IO(Input(UInt(8.W)))
+      val fd = SimLog.file(cf"logfile_$idx%0d.log")
+      fd.printf(cf"An exact string")
+    }
+    ChiselStage
+      .emitCHIRRTL(new MyModule)
+      .fileCheck()(
+        """CHECK: fprintf(clock, UInt<1>(0h1), "logfile_%0d.log", idx, "An exact string")"""
+      )
+  }
+
+  it should "check scope for Printable filenames" in {
+    class Child(log: SimLog) extends Module {
+      val bar = IO(Input(UInt(8.W)))
+      log.printf(cf"bar = $bar%0d\n")
+    }
+    class MyModule extends Module {
+      val foo = Wire(UInt(8.W))
+      val log = SimLog.file(cf"logfile_$foo%0d.log")
+      val child = Module(new Child(log))
+      child.bar := foo
+    }
+    val e = the[ChiselException] thrownBy ChiselStage.emitCHIRRTL(new MyModule, Array("--throw-on-first-error"))
+    (e.getMessage should include).regex("SimLog filename operand '.*' is not visible from the current module Child")
+  }
+
   it should "support writing to a file in simulation" in {
     val testdir = implicitly[HasTestingDirectory].getDirectory
     val logfile = testdir.resolve("workdir-verilator").resolve("logfile.log").toFile