From 1ee6f657fbb7030ab7edb58f724e23f2a5a1a46d Mon Sep 17 00:00:00 2001 From: Adam Izraelevitz Date: Fri, 18 Oct 2024 10:37:06 -0700 Subject: [PATCH] Add Probes to .toString Data methods (#4478) * Add probe string to .toString Data methods * Update chisel tests with new probe toString (cherry picked from commit 8c718b2b634a7b1aba3a4df54549a280042ba80a) # Conflicts: # core/src/main/scala/chisel3/Data.scala # src/test/scala/chisel3/TypeEquivalenceSpec.scala # src/test/scala/chiselTests/LayerSpec.scala # src/test/scala/chiselTests/ProbeSpec.scala --- core/src/main/scala/chisel3/Data.scala | 17 ++- .../scala/chisel3/TypeEquivalenceSpec.scala | 64 ++++++++++- src/test/scala/chiselTests/DataPrint.scala | 11 ++ src/test/scala/chiselTests/LayerSpec.scala | 102 ++++++++++++++++++ src/test/scala/chiselTests/ProbeSpec.scala | 12 ++- 5 files changed, 201 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index a22822a78f1..c172c400f2b 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -408,6 +408,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { } } +<<<<<<< HEAD:core/src/main/scala/chisel3/Data.scala // Both _direction and _resolvedUserDirection are saved versions of computed variables (for // efficiency, avoid expensive recomputation of frequent operations). // Both are only valid after binding is set. @@ -424,13 +425,25 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { _directionVar = actualDirection } +======= + // Specializes the .toString method of a [[Data]] for conditions such as + // DataView, Probe modifiers, a DontCare, and whether it is bound or a pure chisel type +>>>>>>> 8c718b2b6 (Add Probes to .toString Data methods (#4478)):core/src/main/scala/chisel3/DataImpl.scala private[chisel3] def stringAccessor(chiselType: String): String = { + // Add probe and layer color (if they exist) to the returned String + val chiselTypeWithModifier = + probeInfo match { + case None => chiselType + case Some(ProbeInfo(writeable, layer)) => + val layerString = layer.map(x => s"[${x.fullName}]").getOrElse("") + (if (writeable) "RWProbe" else "Probe") + s"$layerString<$chiselType>" + } // Trace views to give better error messages // Reifying involves checking against ViewParent which requires being in a Builder context // Since we're just printing a String, suppress such errors and use this object val thiz = Try(reifySingleTarget(this)).toOption.flatten.getOrElse(this) thiz.topBindingOpt match { - case None => chiselType + case None => chiselTypeWithModifier // Handle DontCares specially as they are "literal-like" but not actually literals case Some(DontCareBinding()) => s"$chiselType(DontCare)" case Some(topBinding) => @@ -438,7 +451,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc { val name = thiz.earlyName val mod = thiz.parentNameOpt.map(_ + ".").getOrElse("") - s"$mod$name: $binding[$chiselType]" + s"$mod$name: $binding[$chiselTypeWithModifier]" } } diff --git a/src/test/scala/chisel3/TypeEquivalenceSpec.scala b/src/test/scala/chisel3/TypeEquivalenceSpec.scala index 4d22e2ca1ee..ba570097a2e 100644 --- a/src/test/scala/chisel3/TypeEquivalenceSpec.scala +++ b/src/test/scala/chisel3/TypeEquivalenceSpec.scala @@ -309,7 +309,11 @@ class TypeEquivalenceSpec extends AnyFlatSpec { it should "detect differences between Probe and Not-Probe" in { Probe(Bool()).findFirstTypeMismatch(Bool(), true, true, true) should be( Some( +<<<<<<< HEAD ": Left (Bool with probeInfo: Some(writeable=false)) and Right (Bool with probeInfo: None) have different probeInfo." +======= + ": Left (Probe with probeInfo: Some(writeable=false, color=None)) and Right (Bool with probeInfo: None) have different probeInfo." +>>>>>>> 8c718b2b6 (Add Probes to .toString Data methods (#4478)) ) ) } @@ -321,7 +325,11 @@ class TypeEquivalenceSpec extends AnyFlatSpec { it should "detect differences between Probe and Not-Probe within a Bundle" in { new BundleWithProbe(true).findFirstTypeMismatch(new BundleWithProbe(false), true, true, true) should be( Some( +<<<<<<< HEAD ".maybeProbe: Left (Bool with probeInfo: Some(writeable=false)) and Right (Bool with probeInfo: None) have different probeInfo." +======= + ".maybeProbe: Left (Probe with probeInfo: Some(writeable=false, color=None)) and Right (Bool with probeInfo: None) have different probeInfo." +>>>>>>> 8c718b2b6 (Add Probes to .toString Data methods (#4478)) ) ) } @@ -329,21 +337,75 @@ class TypeEquivalenceSpec extends AnyFlatSpec { it should "detect differences between probe types" in { RWProbe(Bool()).findFirstTypeMismatch(Probe(Bool()), true, true, true) should be( Some( +<<<<<<< HEAD ": Left (Bool with probeInfo: Some(writeable=true)) and Right (Bool with probeInfo: Some(writeable=false)) have different probeInfo." +======= + ": Left (RWProbe with probeInfo: Some(writeable=true, color=None)) and Right (Probe with probeInfo: Some(writeable=false, color=None)) have different probeInfo." +>>>>>>> 8c718b2b6 (Add Probes to .toString Data methods (#4478)) ) ) } it should "detect differences through probes" in { Probe(Bool()).findFirstTypeMismatch(Probe(Clock()), true, true, true) should be( - Some(": Left (Bool) and Right (Clock) have different types.") + Some(": Left (Probe) and Right (Probe) have different types.") ) } +<<<<<<< HEAD it should "detect differences in probe within a Vector" in { Vec(3, Probe(Bool())).findFirstTypeMismatch(Vec(3, Bool()), true, true, true) should be( Some( "[_]: Left (Bool with probeInfo: Some(writeable=false)) and Right (Bool with probeInfo: None) have different probeInfo." +======= + it should "detect differences in presence of probe colors" in { + Probe(Bool()).findFirstTypeMismatch(Probe(Bool(), Green), true, true, true) should be( + Some( + ": Left (Probe with probeInfo: Some(writeable=false, color=None)) and Right (Probe[Green] with probeInfo: Some(writeable=false, color=Some(Green))) have different probeInfo." + ) + ) + } + + it should "detect differences in probe colors" in { + Probe(Bool(), Red).findFirstTypeMismatch(Probe(Bool(), Green), true, true, true) should be( + Some( + ": Left (Probe[Red] with probeInfo: Some(writeable=false, color=Some(Red))) and Right (Probe[Green] with probeInfo: Some(writeable=false, color=Some(Green))) have different probeInfo." + ) + ) + } + + it should "work with probes within a Bundle" in { + new BundleWithAColor(Some(Red)).findFirstTypeMismatch(new BundleWithAColor(Some(Red)), true, true, true) should be( + None + ) + } + + it should "detect differences in probe colors within a Bundle" in { + new BundleWithAColor(Some(Red)).findFirstTypeMismatch( + new BundleWithAColor(Some(Green)), + true, + true, + true + ) should be( + Some( + ".probe: Left (Probe[Red] with probeInfo: Some(writeable=false, color=Some(Red))) and Right (Probe[Green] with probeInfo: Some(writeable=false, color=Some(Green))) have different probeInfo." + ) + ) + } + + it should "detect differences in probe color presence within a Bundle" in { + new BundleWithAColor(Some(Red)).findFirstTypeMismatch(new BundleWithAColor(None), true, true, true) should be( + Some( + ".probe: Left (Probe[Red] with probeInfo: Some(writeable=false, color=Some(Red))) and Right (Probe with probeInfo: Some(writeable=false, color=None)) have different probeInfo." + ) + ) + } + + it should "detect differences in probe within a Vector" in { + Vec(3, Probe(Bool())).findFirstTypeMismatch(Vec(3, Bool()), true, true, true) should be( + Some( + "[_]: Left (Probe with probeInfo: Some(writeable=false, color=None)) and Right (Bool with probeInfo: None) have different probeInfo." +>>>>>>> 8c718b2b6 (Add Probes to .toString Data methods (#4478)) ) ) } diff --git a/src/test/scala/chiselTests/DataPrint.scala b/src/test/scala/chiselTests/DataPrint.scala index c61b45867c3..a0497381963 100644 --- a/src/test/scala/chiselTests/DataPrint.scala +++ b/src/test/scala/chiselTests/DataPrint.scala @@ -26,17 +26,26 @@ class DataPrintSpec extends ChiselFlatSpec with Matchers { val f = EnumTest.Type() } + object A extends layer.Layer(layer.LayerConfig.Extract()) { + object B extends layer.Layer(layer.LayerConfig.Extract()) + } + "Data types" should "have a meaningful string representation" in { ChiselStage.emitCHIRRTL { new RawModule { UInt().toString should be("UInt") UInt(8.W).toString should be("UInt<8>") + probe.Probe(UInt(8.W)).toString should be("Probe>") + probe.RWProbe(UInt(8.W)).toString should be("RWProbe>") + probe.Probe(UInt(8.W), A).toString should be("Probe[A]>") + probe.Probe(UInt(8.W), A.B).toString should be("Probe[A.B]>") SInt(15.W).toString should be("SInt<15>") Bool().toString should be("Bool") Clock().toString should be("Clock") Vec(3, UInt(2.W)).toString should be("UInt<2>[3]") EnumTest.Type().toString should be("EnumTest") (new BundleTest).toString should be("BundleTest") + (probe.Probe(new BundleTest)).toString should be("Probe") new Bundle { val a = UInt(8.W) }.toString should be("AnonymousBundle") new Bundle { val a = UInt(8.W) }.a.toString should be("UInt<8>") } @@ -45,6 +54,8 @@ class DataPrintSpec extends ChiselFlatSpec with Matchers { class BoundDataModule extends Module { // not in the test to avoid anon naming suffixes Wire(UInt()).toString should be("BoundDataModule.?: Wire[UInt]") + Wire(probe.Probe(UInt(1.W))).toString should be("BoundDataModule.?: Wire[Probe>]") + Wire(probe.Probe(UInt(1.W), A)).toString should be("BoundDataModule.?: Wire[Probe[A]>]") Reg(SInt()).toString should be("BoundDataModule.?: Reg[SInt]") val io = IO(Output(Bool())) // needs a name so elaboration doesn't fail io.toString should be("BoundDataModule.io: IO[Bool]") diff --git a/src/test/scala/chiselTests/LayerSpec.scala b/src/test/scala/chiselTests/LayerSpec.scala index 4042806fe2f..ebc82dd3899 100644 --- a/src/test/scala/chiselTests/LayerSpec.scala +++ b/src/test/scala/chiselTests/LayerSpec.scala @@ -63,4 +63,106 @@ class LayerSpec extends ChiselFlatSpec with Utils with MatchesAndOmits { } +<<<<<<< HEAD +======= + it should "check that a define from inside a layerblock is to a legal layer-colored probe" in { + class Foo extends RawModule { + val a = IO(Output(Probe(Bool(), A))) + layer.block(C) { + val b = Wire(Bool()) + define(a, ProbeValue(b)) + } + } + intercept[ChiselException] { ChiselStage.emitCHIRRTL(new Foo, Array("--throw-on-first-error")) } + .getMessage() should include( + "Cannot define 'Foo.a: IO[Probe[A]]' from colors {'C'} since at least one of these is NOT enabled when 'Foo.a: IO[Probe[A]]' is enabled" + ) + } + + it should "not consider enabled layers in error" in { + class Foo extends RawModule { + layer.enable(A) + val a = IO(Output(Probe(Bool(), A))) + layer.block(C) { + val b = Wire(Bool()) + define(a, ProbeValue(b)) + } + } + + intercept[ChiselException] { ChiselStage.convert(new Foo, Array("--throw-on-first-error")) } + .getMessage() should include( + "Cannot define 'Foo.a: IO[Probe[A]]' from colors {'C'} since at least one of these is NOT enabled when 'Foo.a: IO[Probe[A]]' is enabled" + ) + } + + "Inline layers" should "generated expected FIRRTL" in { + object A extends layer.Layer(layer.LayerConfig.Inline) { + object B extends layer.Layer(layer.LayerConfig.Inline) + } + + class Foo extends RawModule { + layer.block(A) { + layer.block(A.B) {} + } + } + + matchesAndOmits(ChiselStage.emitCHIRRTL(new Foo))( + "layer A, inline :", + "layer B, inline :" + )() + } + + "Inline layers" should "be ignored when choosing default output directories" in { + object LayerWithDefaultOutputDir extends layer.Layer(layer.LayerConfig.Extract()) { + object InlineSublayer extends layer.Layer(layer.LayerConfig.Inline) { + object SublayerWithDefaultOutputDir extends layer.Layer(layer.LayerConfig.Extract()) {} + } + } + + object LayerWithCustomOutputDir + extends layer.Layer(layer.LayerConfig.Extract(layer.CustomOutputDir(Paths.get("myOutputDir")))) { + object InlineSublayer extends layer.Layer(layer.LayerConfig.Inline) { + object SublayerWithDefaultOutputDir extends layer.Layer(layer.LayerConfig.Extract()) {} + } + } + + object LayerWithNoOutputDir extends layer.Layer(layer.LayerConfig.Extract(layer.NoOutputDir)) { + object InlineSublayer extends layer.Layer(layer.LayerConfig.Inline) { + object SublayerWithDefaultOutputDir extends layer.Layer(layer.LayerConfig.Extract()) {} + } + } + + class Foo extends RawModule { + layer.block(LayerWithDefaultOutputDir) { + layer.block(LayerWithDefaultOutputDir.InlineSublayer) { + layer.block(LayerWithDefaultOutputDir.InlineSublayer.SublayerWithDefaultOutputDir) {} + } + } + + layer.block(LayerWithCustomOutputDir) { + layer.block(LayerWithCustomOutputDir.InlineSublayer) { + layer.block(LayerWithCustomOutputDir.InlineSublayer.SublayerWithDefaultOutputDir) {} + } + } + + layer.block(LayerWithNoOutputDir) { + layer.block(LayerWithNoOutputDir.InlineSublayer) { + layer.block(LayerWithNoOutputDir.InlineSublayer.SublayerWithDefaultOutputDir) {} + } + } + } + + matchesAndOmits(ChiselStage.emitCHIRRTL(new Foo))( + bindLayer("LayerWithDefaultOutputDir", List("LayerWithDefaultOutputDir")), + inlineLayer("InlineSublayer"), + bindLayer("SublayerWithDefaultOutputDir", List("LayerWithDefaultOutputDir", "SublayerWithDefaultOutputDir")), + bindLayer("LayerWithCustomOutputDir", List("myOutputDir")), + inlineLayer("InlineSublayer"), + bindLayer("SublayerWithDefaultOutputDir", List("myOutputDir", "SublayerWithDefaultOutputDir")), + bindLayer("LayerWithNoOutputDir", List()), + inlineLayer("InlineSublayer"), + bindLayer("SublayerWithDefaultOutputDir", List("SublayerWithDefaultOutputDir")) + )() + } +>>>>>>> 8c718b2b6 (Add Probes to .toString Data methods (#4478)) } diff --git a/src/test/scala/chiselTests/ProbeSpec.scala b/src/test/scala/chiselTests/ProbeSpec.scala index 9b6b69573bf..e6f7fa4277d 100644 --- a/src/test/scala/chiselTests/ProbeSpec.scala +++ b/src/test/scala/chiselTests/ProbeSpec.scala @@ -235,7 +235,7 @@ class ProbeSpec extends ChiselFlatSpec with Utils { ) } exc.getMessage should include( - "mismatched probe/non-probe types in ProbeSpec_Anon.io.out[0]: IO[Bool] and ProbeSpec_Anon.io.in[0]: IO[Bool]." + "mismatched probe/non-probe types in ProbeSpec_Anon.io.out[0]: IO[Probe] and ProbeSpec_Anon.io.in[0]: IO[Bool]." ) } @@ -253,7 +253,7 @@ class ProbeSpec extends ChiselFlatSpec with Utils { ) } exc.getMessage should include( - "Connection between sink (ProbeSpec_Anon.io.out: IO[Bool]) and source (ProbeSpec_Anon.io.in: IO[Bool]) failed @: Sink io.out in ProbeSpec_Anon of Probed type cannot participate in a mono connection (:=)" + "Connection between sink (ProbeSpec_Anon.io.out: IO[Probe]) and source (ProbeSpec_Anon.io.in: IO[Bool]) failed @: Sink io.out in ProbeSpec_Anon of Probed type cannot participate in a mono connection (:=)" ) } @@ -284,7 +284,11 @@ class ProbeSpec extends ChiselFlatSpec with Utils { ) } exc.getMessage should include( +<<<<<<< HEAD "Connection between sink (ProbeSpec_Anon.io.in: IO[Bool]) and source (ProbeSpec_Anon.io.out: IO[Bool]) failed @: io.in in ProbeSpec_Anon cannot be written from module ProbeSpec_Anon" +======= + "Connection between sink (OutProbe.p: IO[Probe]) and source (ProbeSpec_Anon.out: IO[Probe]) failed @: p in OutProbe cannot be written from module ProbeSpec_Anon." +>>>>>>> 8c718b2b6 (Add Probes to .toString Data methods (#4478)) ) } @@ -354,7 +358,11 @@ class ProbeSpec extends ChiselFlatSpec with Utils { } exc.getMessage should include("Cannot define a probe on a non-equivalent type.") exc.getMessage should include( +<<<<<<< HEAD "Left (ProbeSpec_Anon.p: IO[UInt<4>]) and Right (ProbeSpec_Anon.Node(ProbeSpec_Anon.w: Wire[Bool]): OpResult[Bool]) have different types" +======= + "Left (ProbeSpec_Anon.p: IO[Probe>]) and Right (ProbeSpec_Anon.w: OpResult[Probe]) have different types" +>>>>>>> 8c718b2b6 (Add Probes to .toString Data methods (#4478)) ) }