Skip to content

Implement module prefix API #2155

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.

Sign up for GitHub

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

Draft
jared-barocsi wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Implement module prefix API #2155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
700f18c
Implement module prefixing API
jardhu Oct 7, 2021
b5180f0
Scala doc for withModulePrefix object
jardhu Oct 11, 2021
374aafb
Remove ChiselStages.construct
jardhu Oct 11, 2021
bad9f00
Rename 'currentModulePrefix' to 'Module.currentModulePrefix'
jardhu Oct 11, 2021
6a22f97
Add empty prefix test
jardhu Oct 11, 2021
86eb496
Fix compilation errors
jardhu Oct 11, 2021
4a0fb28
Add negative blackbox prefixing tests
jardhu Oct 14, 2021
f8f335f
Fix blackbox name resolution in namespace
jardhu Oct 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 51 additions & 2 deletions core/src/main/scala/chisel3/Module.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ object Module extends SourceInfoDoc {
def reset: Reset = Builder.forcedReset
/** Returns the current Module */
def currentModule: Option[BaseModule] = Builder.currentModule
/** Returns the current nested module prefix */
def currentModulePrefix: String = Builder.getModulePrefix
}

/** Abstract base class for Modules, which behave much like Verilog modules.
Expand Down Expand Up @@ -140,6 +142,52 @@ abstract class Module(implicit moduleCompileOptions: CompileOptions) extends Raw
}
}

/** Creates a new module prefix scope that prefixes all instantiated modules
*
* @example {{{
* val m = Module(new Module {
* val child = withModulePrefix("Foo") {
* Module(new Module {
* override val desiredName = "Module"
* })
* }
* })
*
* // m.child.name will be equal to "FooModule"
* }}}
*
* Module prefixes can be nested within each other, like so:
* @example {{{
* val m = ChiselStage.construct(new Module {
* val child = withModulePrefix("Foo") {
* Module(new chisel3.Module {
* val nestedChild = withModulePrefix("Bar") {
* Module(new chisel3.Module {
* override val desiredName = "Module"
* })
* }
* })
* }
* })
*
* // m.child.nestedChild.name will be equal to "FooBarModule"
* }}}
*/
object withModulePrefix {
/** Creates a new Module prefix scope
*
* @param prefix the prefix to add to all modules in the scope
* @param block the block of code to run with the new prefix
* @return the result of the block
*/
def apply[T](prefix: String)(block: => T): T = {
Builder.pushModulePrefix(prefix)
val res = block // execute block
Builder.popModulePrefix()
res
}

}

package experimental {

Expand Down Expand Up @@ -450,8 +498,9 @@ package experimental {
// PseudoModules are not "true modules" and thus should share
// their original modules names without uniquification
this match {
case _: PseudoModule => desiredName
case _ => Builder.globalNamespace.name(desiredName)
case _: PseudoModule => Module.currentModulePrefix + desiredName
case _: BlackBox => Builder.globalNamespace.name(desiredName)
case _ => Module.currentModulePrefix + Builder.globalNamespace.name(desiredName)
}
} catch {
case e: NullPointerException => throwException(
Expand Down
20 changes: 20 additions & 0 deletions core/src/main/scala/chisel3/internal/Builder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,9 @@ private[chisel3] class ChiselContext() {
// Records the different prefixes which have been scoped at this point in time
var prefixStack: Prefix = Nil

// Records the current nested module prefix
var modulePrefixStack: Prefix = Nil

// Views belong to a separate namespace (for renaming)
// The namespace outside of Builder context is useless, but it ensures that views can still be created
// and the resulting .toTarget is very clearly useless (_$$View$$_...)
Expand Down Expand Up @@ -498,6 +501,23 @@ private[chisel3] object Builder extends LazyLogging {
// Returns the prefix stack at this moment
def getPrefix: Prefix = chiselContext.get().prefixStack

// Puts a module prefix string onto the module prefix stack
def pushModulePrefix(prefix: String): Unit = {
val context = chiselContext.get()
context.modulePrefixStack = prefix :: context.modulePrefixStack
}

// Remove the module prefix on top of the stack
def popModulePrefix(): List[String] = {
val context = chiselContext.get()
val tail = context.modulePrefixStack.tail
context.modulePrefixStack = tail
tail
}

// Returns the nested module prefix at this moment
def getModulePrefix: String = chiselContext.get().modulePrefixStack.foldLeft("")((a, b) => b + a)

def currentModule: Option[BaseModule] = dynamicContextVar.value match {
case Some(dyanmicContext) => dynamicContext.currentModule
case _ => None
Expand Down
81 changes: 81 additions & 0 deletions src/test/scala/chiselTests/Module.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package chiselTests
import chisel3._
import chisel3.experimental.DataMirror
import chisel3.stage.{ChiselGeneratorAnnotation, ChiselStage, NoRunFirrtlCompilerAnnotation}
import chisel3.util.HasBlackBoxInline
import firrtl.annotations.NoTargetAnnotation
import firrtl.options.Unserializable

Expand Down Expand Up @@ -200,4 +201,84 @@ class ModuleSpec extends ChiselPropSpec with Utils {
val s = Source.fromFile("generated/PlusOne.v").mkString("")
assert(s.contains("assign io_out = io_in + 32'h1"))
}

property("withModulePrefix should prefix modules within it") {
val m = elaborateAndGetModule(new Module {
val child = withModulePrefix("Foo") {
Module(new chisel3.Module {
Module.currentModulePrefix should be ("Foo")

override val desiredName = "Module"
})
}
})

m.child.name should be ("FooModule")
}

property("withModulePrefix should support nested module prefixing") {
val m = elaborateAndGetModule(new Module {
val child = withModulePrefix("Foo") {
Module(new chisel3.Module {
Module.currentModulePrefix should be ("Foo")

val nestedChild = withModulePrefix("Bar") {
Module(new chisel3.Module {
Module.currentModulePrefix should be ("FooBar")

override val desiredName = "Module"
})
}
})
}
})

m.child.nestedChild.name should be ("FooBarModule")
Copy link
Contributor

@mwachs5 mwachs5 Oct 8, 2021

Choose a reason for hiding this comment

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

can you please add a test which includes a SeqMem and run the ReplSeqMem? What is the prefixing behavior for the seq mems?

And also please add a test for blackbox/inline black box, which should NOT get prefixes applied

Copy link
Contributor

@jackkoenig jackkoenig Oct 12, 2021

Choose a reason for hiding this comment

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

Agreed with all of the tests. I'll note that prefixing of SyncReadMems is going to be tricky. I think maybe we should add a test for it but have it as ignore right now because I'm not yet sure how we should handle it.

}
Copy link
Contributor

@jackkoenig jackkoenig Oct 8, 2021

Choose a reason for hiding this comment

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

Because this is a generator, people may want to have generated prefixes, including empty ones. I believe this works but can you add a test showing that an empty prefix (withModulePrefix("")) does not change the resulting modules names?


property("withModulePrefix should not prefix if given an empty argument") {
val m = elaborateAndGetModule(new Module {
val child = withModulePrefix("") {
Module(new chisel3.Module {
Module.currentModulePrefix should be ("")
override val desiredName = "NoPrefixModule"
})
}
})

m.child.name should be ("NoPrefixModule")
}

property("withModulePrefix should not prefix blackboxes") {
val m = elaborateAndGetModule(new Module {
val bb = withModulePrefix("Foo") {
Module(new BlackBox {
val io = IO(new Bundle { })
override val desiredName = "BlackBox"
})
}
})

m.bb.name should be ("BlackBox")
}

property("withModulePrefix should not prefix inlined blackboxes") {
val m = elaborateAndGetModule(new Module {
val bb = withModulePrefix("Foo") {
Module(new BlackBox with HasBlackBoxInline {
val io = IO(new Bundle { })
override val desiredName = "InlineBlackBox"

setInline("InlineBlackBox.v",
s"""
|module InlineBlackBox ();
|
|endmodule
""".stripMargin)
})
}
})

m.bb.name should be ("InlineBlackBox")
}
}