search

chipsalliance / chisel

Chisel: A Modern Hardware Design Language
https://www.chisel-lang.org/
Apache License 2.0
4.01k stars 603 forks source link

Bundle literal construction outside test() is not allowed in Chisel 5.0.0 (works in 3.5.4) #3573

Open KasperHesse opened 1 year ago

KasperHesse commented 1 year ago

Type of issue: Bug Report

Please provide the steps to reproduce the problem: Upgrading my project from Chisel 3.5.4 and Chiseltest 0.5.4 to 5.0.0 in both cases, I ran across the following "issue". Consider the following:

import chisel3._
import chisel3.experimental.BundleLiterals._
import chiseltest._
import org.scalatest.flatspec.AnyFlatSpec

class MyBundle extends Bundle {
  val a = UInt(8.W)
  val b = UInt(8.W)
}

class DUT extends Module {
  val io = IO(new Bundle {
    val in = Input(new MyBundle)
    val out = Output(UInt(8.W))
  })
  io.out := io.in.a + io.in.b
}

class MyTests extends AnyFlatSpec with ChiselScalatestTester {
  "Bundle literals outside test" should "pass" in {
    val in = (new MyBundle).Lit(_.a -> 5.U, _.b -> 4.U)
    test(new DUT) { dut =>
      dut.io.in.poke(in)
      dut.io.out.expect(9.U)
    }
  }

  "Bundle literals inside test" should "pass" in {
    test(new DUT) { dut =>
      val in = (new MyBundle).Lit(_.a -> 5.U, _.b -> 4.U)
      dut.io.in.poke(in)
      dut.io.out.expect(9.U)
    }
  }
}

What is the current behavior? On Chisel 5.0.0, the test where the bundle literal is constructed inside the test passes, but the test where it is constructed before test fails with a "must be inside Builder context", which in turn raises a java.lang.ExceptionInInitializerError. Stacktrace attached below.

I don't know if this is a bug per-se, as it does make some sense to disallow bundle literal construction outside tests. But then again, as they are not really "hardware" but used as a container, I would prefer to have the behaviour of Chisel 3.5.4 (such that I can generate and validate my input vectors before elaborating the design)

What is the expected behavior? On Chisel 3.5.4, both tests pass

Please tell us about your environment:

Other Information

Stacktrace ``` An exception or error caused a run to abort. java.lang.ExceptionInInitializerError at chisel3.internal.Namespace.name(Builder.scala:85) at chisel3.Record.$anonfun$setElementRefs$3(Aggregate.scala:967) at chisel3.Record.$anonfun$setElementRefs$3$adapted(Aggregate.scala:966) at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:563) at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:561) at scala.collection.AbstractIterable.foreach(Iterable.scala:926) at scala.collection.IterableOps$WithFilter.foreach(Iterable.scala:896) at chisel3.Record.setElementRefs(Aggregate.scala:966) at chisel3.Record.bind(Aggregate.scala:1037) at chisel3.Record._makeLit(Aggregate.scala:1154) at chisel3.experimental.package$BundleLiterals$AddBundleLiteralConstructor.Lit(package.scala:149) at MyTests.$anonfun$new$4(MyTest.scala:23) at chisel3.experimental.prefix$.apply(prefix.scala:50) at MyTests.$anonfun$new$3(MyTest.scala:23) at chisel3.internal.plugin.package$.autoNameRecursively(package.scala:33) at MyTests.$anonfun$new$2(MyTest.scala:23) at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85) at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83) at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104) at org.scalatest.Transformer.apply(Transformer.scala:22) at org.scalatest.Transformer.apply(Transformer.scala:20) at org.scalatest.flatspec.AnyFlatSpecLike$$anon$5.apply(AnyFlatSpecLike.scala:1832) at org.scalatest.TestSuite.withFixture(TestSuite.scala:196) at org.scalatest.TestSuite.withFixture$(TestSuite.scala:195) at MyTests.chiseltest$ChiselScalatestTester$$super$withFixture(MyTest.scala:21) at chiseltest.ChiselScalatestTester.$anonfun$withFixture$1(ChiselScalatestTester.scala:87) at scala.util.DynamicVariable.withValue(DynamicVariable.scala:59) at chiseltest.ChiselScalatestTester.withFixture(ChiselScalatestTester.scala:87) at chiseltest.ChiselScalatestTester.withFixture$(ChiselScalatestTester.scala:84) at MyTests.withFixture(MyTest.scala:21) at org.scalatest.flatspec.AnyFlatSpecLike.invokeWithFixture$1(AnyFlatSpecLike.scala:1830) at org.scalatest.flatspec.AnyFlatSpecLike.$anonfun$runTest$1(AnyFlatSpecLike.scala:1842) at org.scalatest.SuperEngine.runTestImpl(Engine.scala:306) at org.scalatest.flatspec.AnyFlatSpecLike.runTest(AnyFlatSpecLike.scala:1842) at org.scalatest.flatspec.AnyFlatSpecLike.runTest$(AnyFlatSpecLike.scala:1824) at org.scalatest.flatspec.AnyFlatSpec.runTest(AnyFlatSpec.scala:1686) at org.scalatest.flatspec.AnyFlatSpecLike.$anonfun$runTests$1(AnyFlatSpecLike.scala:1900) at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:413) at scala.collection.immutable.List.foreach(List.scala:333) at org.scalatest.SuperEngine.traverseSubNodes$1(Engine.scala:401) at org.scalatest.SuperEngine.runTestsInBranch(Engine.scala:390) at org.scalatest.SuperEngine.$anonfun$runTestsInBranch$1(Engine.scala:427) at scala.collection.immutable.List.foreach(List.scala:333) at org.scalatest.SuperEngine.traverseSubNodes$1(Engine.scala:401) at org.scalatest.SuperEngine.runTestsInBranch(Engine.scala:396) at org.scalatest.SuperEngine.runTestsImpl(Engine.scala:475) at org.scalatest.flatspec.AnyFlatSpecLike.runTests(AnyFlatSpecLike.scala:1900) at org.scalatest.flatspec.AnyFlatSpecLike.runTests$(AnyFlatSpecLike.scala:1899) at org.scalatest.flatspec.AnyFlatSpec.runTests(AnyFlatSpec.scala:1686) at org.scalatest.Suite.run(Suite.scala:1114) at org.scalatest.Suite.run$(Suite.scala:1096) at org.scalatest.flatspec.AnyFlatSpec.org$scalatest$flatspec$AnyFlatSpecLike$$super$run(AnyFlatSpec.scala:1686) at org.scalatest.flatspec.AnyFlatSpecLike.$anonfun$run$1(AnyFlatSpecLike.scala:1945) at org.scalatest.SuperEngine.runImpl(Engine.scala:535) at org.scalatest.flatspec.AnyFlatSpecLike.run(AnyFlatSpecLike.scala:1945) at org.scalatest.flatspec.AnyFlatSpecLike.run$(AnyFlatSpecLike.scala:1943) at org.scalatest.flatspec.AnyFlatSpec.run(AnyFlatSpec.scala:1686) at org.scalatest.tools.SuiteRunner.run(SuiteRunner.scala:47) at org.scalatest.tools.Runner$.$anonfun$doRunRunRunDaDoRunRun$13(Runner.scala:1321) at org.scalatest.tools.Runner$.$anonfun$doRunRunRunDaDoRunRun$13$adapted(Runner.scala:1315) at scala.collection.immutable.List.foreach(List.scala:333) at org.scalatest.tools.Runner$.doRunRunRunDaDoRunRun(Runner.scala:1315) at org.scalatest.tools.Runner$.$anonfun$runOptionallyWithPassFailReporter$24(Runner.scala:992) at org.scalatest.tools.Runner$.$anonfun$runOptionallyWithPassFailReporter$24$adapted(Runner.scala:970) at org.scalatest.tools.Runner$.withClassLoaderAndDispatchReporter(Runner.scala:1481) at org.scalatest.tools.Runner$.runOptionallyWithPassFailReporter(Runner.scala:970) at org.scalatest.tools.Runner$.run(Runner.scala:798) at org.scalatest.tools.Runner.run(Runner.scala) at org.jetbrains.plugins.scala.testingSupport.scalaTest.ScalaTestRunner.runScalaTest2or3(ScalaTestRunner.java:38) at org.jetbrains.plugins.scala.testingSupport.scalaTest.ScalaTestRunner.main(ScalaTestRunner.java:25) Caused by: java.lang.IllegalArgumentException: requirement failed: must be inside Builder context at scala.Predef$.require(Predef.scala:337) at chisel3.internal.Builder$.dynamicContext(Builder.scala:499) at chisel3.internal.Builder$.readyForModuleConstr(Builder.scala:694) at chisel3.Module$.do_apply(Module.scala:30) at chisel3.internal.package$.(package.scala:103) ... 70 more ```

What is the use case for changing the behavior? Consistency, as I haven't seen any release mentioning that this behaviour was changed. If one such exists, I'm sorry for the confusion, I just haven't noticed it.

sequencer commented 1 year ago

Please submit this bug report to chiseltest.

KasperHesse commented 1 year ago

Ok, sorry about the confusion. Closing here.

jackkoenig commented 1 year ago

Since the error is must be inside Builder context, I think this is actually a Chisel issue and this should be supported.

KasperHesse commented 1 year ago

I also opened an issue on Chiseltest: https://github.com/ucb-bar/chiseltest/issues/690 and got the following reply from Kevin.

It looks like this is a Chisel problem and also it is probably fixed in Chisel 6. At least your tests all pass with the current main development version of chiseltest which pulls in Chisel 6. The fix in Chisel 6 was probably in this PR: https://github.com/chipsalliance/chisel/pull/3414 It changed the ViewParent to be a lazy val instead of val which would probably make your test work: https://github.com/chipsalliance/chisel/blob/668d638aff8e7503b15d83430d38ea0a8c9bfc58/core/src/main/scala/chisel3/internal/package.scala#L104

jackkoenig commented 1 year ago

Weird accidental bugfix I guess. That PR has an open backport to 5.x: https://github.com/chipsalliance/chisel/pull/3431 and needs to be merged to release 5.1.0 (which is badly needed).