Skip to content

Commit

Permalink
Introduce Blocks, track, use for improved scope checking and boring (#…
Browse files Browse the repository at this point in the history
…4443)

* Blocks, binding tracks insertion block, visibility and boring fixes.
* MonoConnect: Check when visibility after other errors.
    Report error involving general access or module-level access
    before more narrowly diagnosing problem with block scope.
* MonoConnect: If no dynamic context, assume visible.
* Add hasElse to When.
* MonoConnect: require hasDynamicContext.
* track if block is "closed", sizeHint
* Blocks now have "secret" commands that can be added to after inspection.
    Needed to support Select (getCommands()) over Blocks, while still allowing
    the addition of "secret" commands via BoringUtils.
  • Loading branch information
dtzSiFive authored Oct 8, 2024
1 parent d0f8401 commit 75a095d
Show file tree
Hide file tree
Showing 23 changed files with 258 additions and 169 deletions.
10 changes: 5 additions & 5 deletions core/src/main/scala/chisel3/DataImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ private[chisel3] trait DataImpl extends HasId with NamedComponent { self: Data =
)
}

private[chisel3] def isVisible: Boolean = isVisibleFromModule && visibleFromWhen.isEmpty
private[chisel3] def isVisible: Boolean = isVisibleFromModule && visibleFromBlock.isEmpty
private[chisel3] def isVisibleFromModule: Boolean = {
val topBindingOpt = this.topBindingOpt // Only call the function once
val mod = topBindingOpt.flatMap(_.location)
Expand All @@ -683,15 +683,15 @@ private[chisel3] trait DataImpl extends HasId with NamedComponent { self: Data =
case _ => false
}
}
private[chisel3] def visibleFromWhen: Option[SourceInfo] = MonoConnect.checkWhenVisibility(this)
private[chisel3] def visibleFromBlock: Option[SourceInfo] = MonoConnect.checkBlockVisibility(this)
private[chisel3] def requireVisible(): Unit = {
if (!isVisibleFromModule) {
throwException(s"operand '$this' is not visible from the current module ${Builder.currentModule.get.name}")
}
visibleFromWhen match {
visibleFromBlock match {
case Some(sourceInfo) =>
throwException(
s"operand '$this' has escaped the scope of the when (${sourceInfo.makeMessage()}) in which it was constructed."
s"operand '$this' has escaped the scope of the block (${sourceInfo.makeMessage()}) in which it was constructed."
)
case None => ()
}
Expand Down Expand Up @@ -1074,7 +1074,7 @@ trait WireFactory {
val x = if (!t.mustClone(prevId)) t else t.cloneTypeFull

// Bind each element of x to being a Wire
x.bind(WireBinding(Builder.forcedUserModule, Builder.currentWhen))
x.bind(WireBinding(Builder.forcedUserModule, Builder.currentBlock))

pushCommand(DefWire(sourceInfo, x))

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/chisel3/Intrinsic.scala
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ object IntrinsicExpr {
requireIsChiselType(t, "intrinsic type")
val int = if (!t.mustClone(prevId)) t else t.cloneTypeFull

int.bind(OpBinding(Builder.forcedUserModule, Builder.currentWhen))
int.bind(OpBinding(Builder.forcedUserModule, Builder.currentBlock))
require(params.map(_._1).distinct.size == params.size, "parameter names must be unique")
pushCommand(DefIntrinsicExpr(sourceInfo, intrinsic, int, data.map(_.ref), params))
int
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/chisel3/MemImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ private[chisel3] trait MemBaseImpl[T <: Data] extends HasId with NamedComponent
DefMemPort(sourceInfo, t.cloneTypeFull, Node(this), dir, i.ref, clock.ref)
).id
// Bind each element of port to being a MemoryPort
port.bind(MemoryPortBinding(Builder.forcedUserModule, Builder.currentWhen))
port.bind(MemoryPortBinding(Builder.forcedUserModule, Builder.currentBlock))
port
}
}
Expand Down
28 changes: 27 additions & 1 deletion core/src/main/scala/chisel3/ModuleImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

package chisel3

import scala.collection.immutable.ListMap
import scala.collection.immutable.{ListMap, VectorBuilder}
import scala.collection.mutable.{ArrayBuffer, HashMap, LinkedHashSet}

import chisel3.internal._
Expand Down Expand Up @@ -60,6 +60,7 @@ private[chisel3] trait ObjectModuleImpl {

val parent = Builder.currentModule
val parentWhenStack = Builder.whenStack
val parentBlockStack = Builder.blockStack
val parentLayerStack = Builder.layerStack

// Save then clear clock and reset to prevent leaking scope, must be set again in the Module
Expand All @@ -79,10 +80,12 @@ private[chisel3] trait ObjectModuleImpl {
// - set currentModule
// - unset readyForModuleConstr
// - reset whenStack to be empty
// - reset blockStack to body or empty if none
// - reset layerStack to be root :: nil
// - set currentClockAndReset
val module: T = bc // bc is actually evaluated here

require(Builder.blockDepth == module.getBody.size, "block leftover")
if (Builder.whenDepth != 0) {
throwException("Internal Error! when() scope depth is != 0, this should have been caught!")
}
Expand All @@ -104,6 +107,7 @@ private[chisel3] trait ObjectModuleImpl {
// scope of the current Module.
Builder.currentModule = parent // Back to parent!
Builder.whenStack = parentWhenStack
Builder.blockStack = parentBlockStack
Builder.layerStack = parentLayerStack
Builder.currentClock = saveClock // Back to clock and reset scope
Builder.currentReset = saveReset
Expand Down Expand Up @@ -168,10 +172,12 @@ private[chisel3] trait ObjectModuleImpl {
val parent = Builder.currentModule
val whenStackOpt = Option.when(Builder.hasDynamicContext)(Builder.whenStack)
val layerStackOpt = Option.when(Builder.hasDynamicContext)(Builder.layerStack)
val blockStackOpt = Option.when(Builder.hasDynamicContext)(Builder.blockStack)
val module: T = bc // bc is actually evaluated here
if (!parent.isEmpty) { Builder.currentModule = parent }
whenStackOpt.foreach(Builder.whenStack = _)
layerStackOpt.foreach(Builder.layerStack = _)
blockStackOpt.foreach(Builder.blockStack = _)

module
}
Expand Down Expand Up @@ -452,6 +458,24 @@ package experimental {

/** Represents an eagerly-determined unique and descriptive identifier for this module */
final val definitionIdentifier = _definitionIdentifier

// Modules that contain bodies should override this.
protected def hasBody: Boolean = false
protected val _body: Block = if (hasBody) new Block(_sourceInfo, None /* command not available */ ) else null
private[chisel3] def getBody: Option[Block] = Some(_body)

// Current block at point of creation.
private var _blockVar: Block = Builder.currentBlock.getOrElse(null)
private[chisel3] def _block: Option[Block] = {
Option(_blockVar)
}
private[chisel3] def _block_=(target: Option[Block]): Unit = {
_blockVar = target.getOrElse(null)
}

// Return Block containing the instance of this module.
protected[chisel3] def getInstantiatingBlock: Option[Block] = _block

//
// Builder Internals - this tracks which Module RTL construction belongs to.
//
Expand All @@ -467,6 +491,8 @@ package experimental {

Builder.currentModule = Some(this)
Builder.whenStack = Nil
Builder.blockStack = Nil
getBody.foreach(Builder.pushBlock(_))
Builder.layerStack = layer.Layer.root :: Nil
}

Expand Down
42 changes: 10 additions & 32 deletions core/src/main/scala/chisel3/RawModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -84,38 +84,24 @@ abstract class RawModule extends BaseModule {
//
// RTL construction internals
//
// Perhaps this should be an ArrayBuffer (or ArrayBuilder), but DefModule is public and has Seq[Command]
// so our best option is to share a single Seq datastructure with that
private val _commands = new VectorBuilder[Command]()
protected override def hasBody: Boolean = true

/** The current region to which commands will be added. */
private var _currentRegion = _commands

private[chisel3] def changeRegion(newRegion: VectorBuilder[Command]): Unit = {
_currentRegion = newRegion
}

private[chisel3] def withRegion[A](newRegion: VectorBuilder[Command])(thunk: => A): A = {
_currentRegion ++= stagedSecretCommands
stagedSecretCommands.clear()
var oldRegion = _currentRegion
changeRegion(newRegion)
private[chisel3] def withRegion[A](newRegion: Block)(thunk: => A): A = {
Builder.pushBlock(newRegion)
val result = thunk
_currentRegion ++= stagedSecretCommands
stagedSecretCommands.clear()
changeRegion(oldRegion)
require(Builder.currentBlock == Some(newRegion), "didn't return to same region")
Builder.popBlock()
result
}

private[chisel3] def addCommand(c: Command): Unit = {
require(!_closed, "Can't write to module after module close")
_currentRegion += c
require(Builder.currentBlock.isDefined, "must have block set")
Builder.currentBlock.get.addCommand(c)
}
protected def getCommands: Seq[Command] = {
require(_closed, "Can't get commands before module close")
// Unsafe cast but we know that any RawModule uses a DefModule
// _component is defined as a var on BaseModule and we cannot override mutable vars
_component.get.asInstanceOf[DefModule].commands
_body.getCommands()
}

//
Expand Down Expand Up @@ -219,15 +205,12 @@ abstract class RawModule extends BaseModule {
// Generate IO invalidation commands to initialize outputs as unused,
// unless the client wants explicit control over their generation.
val component =
DefModule(this, name, _isPublic, Builder.enabledLayers.toSeq, firrtlPorts, _commands.result())
DefModule(this, name, _isPublic, Builder.enabledLayers.toSeq, firrtlPorts, _body)

// Secret connections can be staged if user bored into children modules
component.secretCommands ++= stagedSecretCommands
stagedSecretCommands.clear()
_component = Some(component)
_component
}
private[chisel3] val stagedSecretCommands = collection.mutable.ArrayBuffer[Command]()

private[chisel3] def secretConnection(left: Data, _right: Data)(implicit si: SourceInfo): Unit = {
val (right: Data, _) = chisel3.experimental.dataview
Expand Down Expand Up @@ -263,12 +246,7 @@ abstract class RawModule extends BaseModule {
}

val rhs = computeConnection(left, right)
val secretCommands = if (_closed) {
_component.get.asInstanceOf[DefModule].secretCommands
} else {
stagedSecretCommands
}
secretCommands += rhs
Builder.currentBlock.get.addSecretCommand(rhs)
}

private[chisel3] def initializeInParent(): Unit = {}
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/scala/chisel3/Reg.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ object Reg {
val reg = if (!t.mustClone(prevId)) t else t.cloneTypeFull
val clock = Builder.forcedClock.ref

reg.bind(RegBinding(Builder.forcedUserModule, Builder.currentWhen))
reg.bind(RegBinding(Builder.forcedUserModule, Builder.currentBlock))
pushCommand(DefReg(sourceInfo, reg, clock))
reg
}
Expand Down Expand Up @@ -174,7 +174,7 @@ object RegInit {
val clock = Builder.forcedClock
val reset = Builder.forcedReset

reg.bind(RegBinding(Builder.forcedUserModule, Builder.currentWhen))
reg.bind(RegBinding(Builder.forcedUserModule, Builder.currentBlock))
requireIsHardware(init, "reg initializer")
requireNoProbeTypeModifier(t, "Cannot make a register of a Chisel type with a probe modifier.")
pushCommand(DefRegInit(sourceInfo, reg, clock.ref, reset.ref, init.ref))
Expand Down
5 changes: 2 additions & 3 deletions core/src/main/scala/chisel3/When.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ object when {
)(
implicit sourceInfo: SourceInfo
): WhenContext = {
new WhenContext(sourceInfo, () => cond, block, 0, Nil)
new WhenContext(sourceInfo, () => cond, block, Nil)
}

/** Returns the current `when` condition
Expand Down Expand Up @@ -87,7 +87,6 @@ final class WhenContext private[chisel3] (
_sourceInfo: SourceInfo,
cond: () => Bool,
block: => Any,
firrtlDepth: Int,
// For capturing conditions from prior whens or elsewhens
altConds: List[() => Bool]) {

Expand Down Expand Up @@ -124,7 +123,7 @@ final class WhenContext private[chisel3] (
implicit sourceInfo: SourceInfo
): WhenContext = {
Builder.forcedUserModule.withRegion(whenCommand.elseRegion) {
new WhenContext(sourceInfo, () => elseCond, block, firrtlDepth + 1, cond :: altConds)
new WhenContext(sourceInfo, () => elseCond, block, cond :: altConds)
}
}

Expand Down
40 changes: 18 additions & 22 deletions core/src/main/scala/chisel3/internal/Binding.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package chisel3.internal

import chisel3._
import chisel3.experimental.{BaseModule, SourceInfo}
import chisel3.internal.firrtl.ir.{LitArg, PropertyLit}
import chisel3.internal.firrtl.ir.{Block, LitArg, PropertyLit}
import chisel3.properties.Class

import scala.collection.immutable.VectorMap
Expand Down Expand Up @@ -64,9 +64,9 @@ private[chisel3] object binding {
// A binding representing a data that cannot be (re)assigned to.
sealed trait ReadOnlyBinding extends TopBinding

// A component that can potentially be declared inside a 'when'
sealed trait ConditionalDeclarable extends TopBinding {
def visibility: Option[WhenContext]
// A component placed within a Block.
sealed trait BlockBinding extends TopBinding {
def parentBlock: Option[Block]
}

// TODO(twigg): Ops between unenclosed nodes can also be unenclosed
Expand All @@ -76,22 +76,18 @@ private[chisel3] object binding {
// Added to handle BoringUtils in Chisel
case class SecretPortBinding(enclosure: BaseModule) extends ConstrainedBinding

case class OpBinding(enclosure: RawModule, visibility: Option[WhenContext])
case class OpBinding(enclosure: RawModule, parentBlock: Option[Block])
extends ConstrainedBinding
with ReadOnlyBinding
with ConditionalDeclarable
case class MemoryPortBinding(enclosure: RawModule, visibility: Option[WhenContext])
with BlockBinding
case class MemoryPortBinding(enclosure: RawModule, parentBlock: Option[Block])
extends ConstrainedBinding
with ConditionalDeclarable
case class SramPortBinding(enclosure: RawModule, visibility: Option[WhenContext])
with BlockBinding
case class SramPortBinding(enclosure: RawModule, parentBlock: Option[Block])
extends ConstrainedBinding
with ConditionalDeclarable
case class RegBinding(enclosure: RawModule, visibility: Option[WhenContext])
extends ConstrainedBinding
with ConditionalDeclarable
case class WireBinding(enclosure: RawModule, visibility: Option[WhenContext])
extends ConstrainedBinding
with ConditionalDeclarable
with BlockBinding
case class RegBinding(enclosure: RawModule, parentBlock: Option[Block]) extends ConstrainedBinding with BlockBinding
case class WireBinding(enclosure: RawModule, parentBlock: Option[Block]) extends ConstrainedBinding with BlockBinding

case class ClassBinding(enclosure: Class) extends ConstrainedBinding with ReadOnlyBinding

Expand Down Expand Up @@ -168,10 +164,10 @@ private[chisel3] object binding {
}

// Views currently only support 1:1 Element-level mappings
case class ViewBinding(target: Element, writability: ViewWriteability) extends Binding with ConditionalDeclarable {
case class ViewBinding(target: Element, writability: ViewWriteability) extends Binding with BlockBinding {
def location: Option[BaseModule] = target.binding.flatMap(_.location)
def visibility: Option[WhenContext] = target.binding.flatMap {
case c: ConditionalDeclarable => c.visibility
def parentBlock: Option[Block] = target.binding.flatMap {
case b: BlockBinding => b.parentBlock
case _ => None
}
}
Expand All @@ -188,7 +184,7 @@ private[chisel3] object binding {
childMap: Map[Data, Data],
writabilityMap: Option[Map[Data, ViewWriteability]])
extends Binding
with ConditionalDeclarable {
with BlockBinding {
// Helper lookup function since types of Elements always match
def lookup(key: Element): Option[Element] = childMap.get(key).map(_.asInstanceOf[Element])

Expand Down Expand Up @@ -218,9 +214,9 @@ private[chisel3] object binding {
if (locations.size == 1) Some(locations.head)
else None
}
lazy val visibility: Option[WhenContext] = {
lazy val parentBlock: Option[Block] = {
val contexts = childMap.values.view
.flatMap(_.binding.toSeq.collect { case c: ConditionalDeclarable => c.visibility }.flatten)
.flatMap(_.binding.toSeq.collect { case b: BlockBinding => b.parentBlock }.flatten)
.toVector
.distinct
if (contexts.size == 1) Some(contexts.head)
Expand Down
Loading

0 comments on commit 75a095d

Please sign in to comment.