Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error on S-interpolator usage for assert, assume and printf #2751

Merged
merged 11 commits into from
Sep 30, 2022
37 changes: 34 additions & 3 deletions core/src/main/scala/chisel3/Printf.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

package chisel3

import scala.language.experimental.macros
import chisel3.internal._
import chisel3.internal.Builder.pushCommand
import chisel3.internal.sourceinfo.SourceInfo
import scala.language.experimental.macros
import scala.reflect.macros.blackbox

/** Prints a message in simulation
*
Expand Down Expand Up @@ -76,7 +77,29 @@ object printf {
* @param data format string varargs containing data to print
*/
def apply(fmt: String, data: Bits*)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf =
apply(Printable.pack(fmt, data: _*))
macro _applyMacroWithInterpolatorCheck

def _applyMacroWithInterpolatorCheck(
c: blackbox.Context
)(fmt: c.Tree,
data: c.Tree*
)(sourceInfo: c.Tree,
compileOptions: c.Tree
): c.Tree = {
import c.universe._
fmt match {
case q"scala.StringContext.apply(..$_).s(..$_)" =>
c.error(
c.enclosingPosition,
"The s-interpolator prints the Scala .toString of Data objects rather than the value " +
"of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
"an elaboration time print, use println."
)
case _ =>
}
val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("printfWithReset"))
q"$apply_impl_do(_root_.chisel3.Printable.pack($fmt, ..$data))($sourceInfo, $compileOptions)"
}

/** Prints a message in simulation
*
Expand All @@ -92,7 +115,15 @@ object printf {
* @see [[Printable]] documentation
* @param pable [[Printable]] to print
*/
def apply(pable: Printable)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf = {
def apply(pable: Printable)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Printf =
printfWithReset(pable)(sourceInfo, compileOptions)

private[chisel3] def printfWithReset(
pable: Printable
)(
implicit sourceInfo: SourceInfo,
compileOptions: CompileOptions
): Printf = {
var printfId: Printf = null
when(!Module.reset.asBool) {
printfId = printfWithoutReset(pable)
Expand Down
56 changes: 52 additions & 4 deletions core/src/main/scala/chisel3/VerificationStatement.scala
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ object assert extends VerifPrintMacrosDoc {
)(
implicit sourceInfo: SourceInfo,
compileOptions: CompileOptions
): Assert = macro _applyMacroWithStringMessage
): Assert = macro _applyMacroWithInterpolatorCheck

/** Checks for a condition to be valid in the circuit at all times. If the
* condition evaluates to false, the circuit simulation stops with an error.
Expand Down Expand Up @@ -79,6 +79,32 @@ object assert extends VerifPrintMacrosDoc {
def apply(cond: Bool)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Assert =
macro _applyMacroWithNoMessage

import VerificationStatement._

/** @group VerifPrintMacros */
def _applyMacroWithInterpolatorCheck(
c: blackbox.Context
)(cond: c.Tree,
message: c.Tree,
data: c.Tree*
)(sourceInfo: c.Tree,
compileOptions: c.Tree
): c.Tree = {
import c.universe._
message match {
case q"scala.StringContext.apply(..$_).s(..$_)" =>
c.error(
c.enclosingPosition,
"The s-interpolator prints the Scala .toString of Data objects rather than the value " +
"of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
"an elaboration time check, call assert with a Boolean condition."
)
case _ =>
}
val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable"))
q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some(_root_.chisel3.Printable.pack($message, ..$data)))($sourceInfo, $compileOptions)"
}

/** An elaboration-time assertion. Calls the built-in Scala assert function. */
def apply(cond: Boolean, message: => String): Unit = Predef.assert(cond, message)

Expand All @@ -88,8 +114,6 @@ object assert extends VerifPrintMacrosDoc {
/** Named class for assertions. */
final class Assert private[chisel3] () extends VerificationStatement

import VerificationStatement._

/** @group VerifPrintMacros */
@deprecated(
"This method has been deprecated in favor of _applyMacroWithStringMessage. Please use the same.",
Expand Down Expand Up @@ -215,7 +239,7 @@ object assume extends VerifPrintMacrosDoc {
)(
implicit sourceInfo: SourceInfo,
compileOptions: CompileOptions
): Assume = macro _applyMacroWithStringMessage
): Assume = macro _applyMacroWithInterpolatorCheck

/** Assumes a condition to be valid in the circuit at all times.
* Acts like an assertion in simulation and imposes a declarative
Expand Down Expand Up @@ -256,6 +280,30 @@ object assume extends VerifPrintMacrosDoc {

import VerificationStatement._

/** @group VerifPrintMacros */
def _applyMacroWithInterpolatorCheck(
c: blackbox.Context
)(cond: c.Tree,
message: c.Tree,
data: c.Tree*
)(sourceInfo: c.Tree,
compileOptions: c.Tree
): c.Tree = {
import c.universe._
message match {
case q"scala.StringContext.apply(..$_).s(..$_)" =>
c.error(
c.enclosingPosition,
"The s-interpolator prints the Scala .toString of Data objects rather than the value " +
"of the hardware wire during simulation. Use the cf-interpolator instead. If you want " +
"an elaboration time check, call assert with a Boolean condition."
)
case _ =>
}
val apply_impl_do = symbolOf[this.type].asClass.module.info.member(TermName("_applyWithSourceLinePrintable"))
q"$apply_impl_do($cond, ${getLine(c)}, _root_.scala.Some(_root_.chisel3.Printable.pack($message, ..$data)))($sourceInfo, $compileOptions)"
}

/** @group VerifPrintMacros */
@deprecated(
"This method has been deprecated in favor of _applyMacroWithStringMessage. Please use the same.",
Expand Down
14 changes: 13 additions & 1 deletion docs/src/explanations/printing.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,23 @@ Chisel provides the `printf` function for debugging purposes. It comes in two fl

### Scala-style

Chisel also supports printf in a style similar to [Scala's String Interpolation](http://docs.scala-lang.org/overviews/core/string-interpolation.html). Chisel provides a custom string interpolator `cf` which follows C-style format specifiers (see section [C-style](#c-style) below). Here's a few examples of using the `cf` interpolator:
Chisel also supports printf in a style similar to [Scala's String Interpolation](http://docs.scala-lang.org/overviews/core/string-interpolation.html). Chisel provides a custom string interpolator `cf` which follows C-style format specifiers (see section [C-style](#c-style) below).

Note that the Scala s-interpolator is not supported in Chisel constructs and will throw an error:

```scala mdoc:invisible
import chisel3._
```

```scala mdoc:fail
class MyModule extends Module {
val in = IO(Input(UInt(8.W)))
printf(s"in = $in\n")
}
```

Instead, use Chisel's `cf` interpolator as in the following examples:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice :)


```scala mdoc:compile-only
val myUInt = 33.U
printf(cf"myUInt = $myUInt") // myUInt = 33
Expand Down
7 changes: 2 additions & 5 deletions src/test/scala/chiselTests/IntervalSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class ClipSqueezeWrapDemo(
val wrapped = counter.wrap(0.U.asInterval(targetRange))

when(counter === startValue) {
printf(s"Target range is $range\n")
printf(cf"Target range is $range\n")
printf("value clip squeeze wrap\n")
}

Expand Down Expand Up @@ -245,10 +245,7 @@ class SqueezeFunctionalityTester(range: IntervalRange, startNum: BigDecimal, end
squeezeTemplate := toSqueeze.squeeze(squeezeInterval)

printf(
s"SqueezeTest %d %d.squeeze($range) => %d\n",
counter,
toSqueeze.asSInt(),
squeezeTemplate.asSInt()
cf"SqueezeTest $counter%d ${toSqueeze.asSInt()}%d.squeeze($range) => ${squeezeTemplate.asSInt()}%d\n"
)
}

Expand Down
4 changes: 2 additions & 2 deletions src/test/scala/chiselTests/Vec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class FillTester(n: Int, value: Int) extends BasicTester {
val x = VecInit(Array.fill(n)(value.U))
val u = VecInit.fill(n)(value.U)

assert(x.asUInt() === u.asUInt(), s"Expected Vec to be filled like $x, instead VecInit.fill created $u")
assert(x.asUInt() === u.asUInt(), cf"Expected Vec to be filled like $x, instead VecInit.fill created $u")
stop()
}

Expand Down Expand Up @@ -235,7 +235,7 @@ class IterateTester(start: Int, len: Int)(f: UInt => UInt) extends BasicTester {
val testVec = VecInit.iterate(start.U, len)(f)
assert(
controlVec.asUInt() === testVec.asUInt(),
s"Expected Vec to be filled like $controlVec, instead creaeted $testVec\n"
cf"Expected Vec to be filled like $controlVec, instead created $testVec\n"
)
stop()
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/scala/chiselTests/VecLiteralSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ class VecLiteralSpec extends ChiselFreeSpec with Utils {

assertTesterPasses {
new BasicTester {
chisel3.assert(outsideVecLit(0) === 0xdd.U, s"v(0)")
chisel3.assert(outsideVecLit(0) === 0xdd.U, "v(0)")
stop()
}
}
Expand All @@ -216,7 +216,7 @@ class VecLiteralSpec extends ChiselFreeSpec with Utils {

assertTesterPasses {
new BasicTester {
chisel3.assert(outsideVecLit(0) === 0xdd.U, s"v(0)")
chisel3.assert(outsideVecLit(0) === 0xdd.U, "v(0)")
chisel3.assert(outsideVecLit(1) === 0xcc.U)
chisel3.assert(outsideVecLit(2) === 0xbb.U)
chisel3.assert(outsideVecLit(3) === 0xaa.U)
Expand Down