diff options
4 files changed, 77 insertions, 34 deletions
diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 82a1708e..236f528e 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -251,7 +251,9 @@ package experimental { /** Legalized name of this module. */ final lazy val name = try { - Builder.globalNamespace.name(desiredName) + // If this is a module aspect, it should share the same name as the original module + // Thus, the desired name should be returned without uniquification + if(this.isInstanceOf[ModuleAspect]) desiredName else Builder.globalNamespace.name(desiredName) } catch { case e: NullPointerException => throwException( s"Error: desiredName of ${this.getClass.getName} is null. Did you evaluate 'name' before all values needed by desiredName were available?", e) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 3988ac68..30fa2db2 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -627,7 +627,11 @@ private[chisel3] object Builder { def build[T <: RawModule](f: => T): (Circuit, T) = { - dynamicContextVar.withValue(Some(new DynamicContext())) { + build(f, new DynamicContext()) + } + + private [chisel3] def build[T <: RawModule](f: => T, dynamicContext: DynamicContext): (Circuit, T) = { + dynamicContextVar.withValue(Some(dynamicContext)) { checkScalaVersion() errors.info("Elaborating design...") val mod = f diff --git a/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala b/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala index 747ba18d..8754f4e7 100644 --- a/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala +++ b/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala @@ -2,9 +2,9 @@ package chisel3.aop.injecting -import chisel3.{Module, ModuleAspect, experimental, withClockAndReset, RawModule, MultiIOModule} +import chisel3.{Module, ModuleAspect, MultiIOModule, RawModule, experimental, withClockAndReset} import chisel3.aop._ -import chisel3.internal.Builder +import chisel3.internal.{Builder, DynamicContext} import chisel3.internal.firrtl.DefModule import chisel3.stage.DesignAnnotation import firrtl.annotations.ModuleTarget @@ -42,17 +42,32 @@ abstract class InjectorAspect[T <: RawModule, M <: RawModule]( injection: M => Unit ) extends Aspect[T] { final def toAnnotation(top: T): AnnotationSeq = { - toAnnotation(selectRoots(top), top.name) + val moduleNames = Select.collectDeep(top) { case i => i.name }.toSeq + toAnnotation(selectRoots(top), top.name, moduleNames) } - final def toAnnotation(modules: Iterable[M], circuit: String): AnnotationSeq = { + /** Returns annotations which contain all injection logic + * + * @param modules The modules to inject into + * @param circuit Top level circuit + * @param moduleNames The names of all existing modules in the original circuit, to avoid name collisions + * @return + */ + final def toAnnotation(modules: Iterable[M], circuit: String, moduleNames: Seq[String]): AnnotationSeq = { + val dynamicContext = new DynamicContext() + // Add existing module names into the namespace. If injection logic instantiates new modules + // which would share the same name, they will get uniquified accordingly + moduleNames.foreach { n => + dynamicContext.globalNamespace.name(n) + } RunFirrtlTransformAnnotation(new InjectingTransform) +: modules.map { module => val (chiselIR, _) = Builder.build(Module(new ModuleAspect(module) { module match { case x: MultiIOModule => withClockAndReset(x.clock, x.reset) { injection(module) } case x: RawModule => injection(module) } - })) + }), dynamicContext) + val comps = chiselIR.components.map { case x: DefModule if x.name == module.name => x.copy(id = module) case other => other @@ -65,7 +80,7 @@ abstract class InjectorAspect[T <: RawModule, M <: RawModule]( case m: firrtl.ir.Module if m.name == module.name => stmts += m.body Nil - case other => + case other: firrtl.ir.Module => Seq(other) } diff --git a/src/test/scala/chiselTests/aop/InjectionSpec.scala b/src/test/scala/chiselTests/aop/InjectionSpec.scala index 3cb0d48b..60233f48 100644 --- a/src/test/scala/chiselTests/aop/InjectionSpec.scala +++ b/src/test/scala/chiselTests/aop/InjectionSpec.scala @@ -3,42 +3,46 @@ package chiselTests.aop import chisel3.testers.{BasicTester, TesterDriver} -import chiselTests.ChiselFlatSpec +import chiselTests.{ChiselFlatSpec, Utils} import chisel3._ import chisel3.aop.injecting.InjectingAspect +import logger.{LogLevel, LogLevelAnnotation} -class SubmoduleManipulationTester extends BasicTester { - val moduleSubmoduleA = Module(new SubmoduleA) -} +object InjectionHierarchy { -class SubmoduleA extends Module { - val io = IO(new Bundle { - val out = Output(Bool()) - }) - io.out := false.B -} + class SubmoduleManipulationTester extends BasicTester { + val moduleSubmoduleA = Module(new SubmoduleA) + } -class SubmoduleB extends Module { - val io = IO(new Bundle { - val in = Input(Bool()) - }) -} + class SubmoduleA extends Module { + val io = IO(new Bundle { + val out = Output(Bool()) + }) + io.out := false.B + } -class AspectTester(results: Seq[Int]) extends BasicTester { - val values = VecInit(results.map(_.U)) - val counter = RegInit(0.U(results.length.W)) - counter := counter + 1.U - when(counter >= values.length.U) { - stop() - }.otherwise { - when(reset.asBool() === false.B) { - printf("values(%d) = %d\n", counter, values(counter)) - assert(counter === values(counter)) + class SubmoduleB extends Module { + val io = IO(new Bundle { + val in = Input(Bool()) + }) + } + + class AspectTester(results: Seq[Int]) extends BasicTester { + val values = VecInit(results.map(_.U)) + val counter = RegInit(0.U(results.length.W)) + counter := counter + 1.U + when(counter >= values.length.U) { + stop() + }.otherwise { + when(reset.asBool() === false.B) { + assert(counter === values(counter)) + } } } } -class InjectionSpec extends ChiselFlatSpec { +class InjectionSpec extends ChiselFlatSpec with Utils { + import InjectionHierarchy._ val correctValueAspect = InjectingAspect( {dut: AspectTester => Seq(dut)}, {dut: AspectTester => @@ -67,6 +71,16 @@ class InjectionSpec extends ChiselFlatSpec { } ) + val duplicateSubmoduleAspect = InjectingAspect( + {dut: SubmoduleManipulationTester => Seq(dut)}, + {_: SubmoduleManipulationTester => + // By creating a second SubmoduleA, the module names would conflict unless they were uniquified + val moduleSubmoduleA2 = Module(new SubmoduleA) + //if we're here then we've elaborated correctly + stop() + } + ) + "Test" should "pass if inserted the correct values" in { assertTesterPasses{ new AspectTester(Seq(0, 1, 2)) } } @@ -88,4 +102,12 @@ class InjectionSpec extends ChiselFlatSpec { "Test" should "pass if the submodules in SubmoduleManipulationTester can be manipulated by manipulateSubmoduleAspect" in { assertTesterPasses({ new SubmoduleManipulationTester} , Nil, Seq(manipulateSubmoduleAspect) ++ TesterDriver.verilatorOnly) } + + "Module name collisions when adding a new module" should "be resolved" in { + assertTesterPasses( + { new SubmoduleManipulationTester}, + Nil, + Seq(duplicateSubmoduleAspect) ++ TesterDriver.verilatorOnly + ) + } } |