[core] Add knownLayers API to BlackBox, ExtModule #4983
Conversation
|
It looks like, when a layer is used as a knownlayer, it doesn't automatically get added to the list of layers on the builder, it would be nice if we could do that. object A extends layer.Layer(layer.LayerConfig.Extract()) {
}
class Qux extends BlackBox(knownLayers = Seq(A)) {
final val io = IO(new Bundle {})
} |
| @@ -296,6 +296,19 @@ abstract class RawModule extends BaseModule { | |||
| protected[chisel3] override def moduleBuilt(): Unit = _afterModuleBuilt.foreach(_()) | |||
|
|
|||
| private[chisel3] def initializeInParent(): Unit = {} | |||
|
|
|||
| final var _knownLayers: Option[Seq[layer.Layer]] = None | |||
| atModuleBodyEnd { | |||
There was a problem hiding this comment.
are there any sneaky APIs that add layers with atModuleBodyEnd? Can bored probes impact this? Should we at least add some sort of check that no one tries to add a new layer after this _knownLayers was evaluated?
There was a problem hiding this comment.
Boring could definitely cause this to be incorrect. I can probably show that this is a problem if it's not updated there.
The thing that's really annoying about this is that really only ExtModule/BlackBox should have known layers. However, because there are APIs that let you create an Instance from a Definition without examining the DesignAnnotation (which has layer information) then this forces everything up to BaseModule to need to record this.
It perhaps makes more sense this way, just this is surprisingly invasive just to get the separate elaboration examples working correctly.
@jackkoenig: Do you have any ideas on how to architect this more cleanly?
There was a problem hiding this comment.
However, because there are APIs that let you create an Instance from a Definition without examining the DesignAnnotation (which has layer information) then this forces everything up to BaseModule to need to record this.
I think this is a key observation that applies to a lot of things. We have Modules and then we have auxiliary information stored in the Builder that is largely lost once you exit the build phase for that particular circuit. Definitions act like small circuits and copy things to the Builder of the outer elaboration (if there is one).
I think what we probably need is to actually lean more in to Definitions as circuit. Instead of making knownLayers a public method on BaseModule, can it instead be on Definition? And then could Definitions capture more information from their DynamicContexts (in this case obviously the layers)?
There was a problem hiding this comment.
The problem with Definition is that it's currently too generic. I.e., you can have a Definition[A] for any A. Hence, it's weird to have this live on Definition[A], when it's really a property of A. (I generally agree with this, though, and we should move towards having a Definition be more tied to a module/extmodule and less generic.)
src/test/scala-2/chiselTests/experimental/hierarchy/SeparateElaborationSpec.scala
Outdated
Show resolved
Hide resolved
github-actions
bot
force-pushed
the
ci/ci-circt-nightly
branch
from
78c8ac1 to
121c08e
Compare
Add an API to add `knownlayers` specifications to FIRRTL `extmodule`s. This is a mechanism to declare that an external module was compiled with support for specific layers. This then allows a FIRRTL comopiler compiler to know that it should automatically enable these layers if needed. In order to make this work, a number of other changes had to be made. First, move `atModuleBodyEnd` from `RawModule` to `BaseModule`. This allows capturing of the `Builder` state when the module closes. Here, this is used to capture the layers that are known to the `Builder` if a module is later converted to a `Definition`. Second, the default layers are now added to the `Builder` when it starts as opposed to when it ends. This allows for the default layers to be captured by module or external modules and then emitted correctly if needed (in the `Definition` or as FIRRTL `knownlayers`). Third, when an `Instance` is created, any layers that it has need to be added to the `Builder`. This can arise if a `Definition` is imported via an annotation and used to create a blackbox which will define additional `knownlayers`. If these layers are not added to the FIRRTL circuit, then this is illegal FIRRTL. Signed-off-by: Schuyler Eldridge <[email protected]>
seldridge
force-pushed
the
dev/seldridge/blackbox-knownlayers
branch
from
1227b95 to
200bef0
Compare
|
The main change here since the last review is that now external modules which add layer-colored probe ports or use the |
|
☝️ I talked to @jackkoenig about this offline and he's good with this approach. |
Add an API to add `knownlayers` specifications to FIRRTL `extmodule`s. This is a mechanism to declare that an external module was compiled with support for specific layers. This then allows a FIRRTL comopiler compiler to know that it should automatically enable these layers if needed. In order to make this work, a number of other changes had to be made. First, move `atModuleBodyEnd` from `RawModule` to `BaseModule`. This allows capturing of the `Builder` state when the module closes. Here, this is used to capture the layers that are known to the `Builder` if a module is later converted to a `Definition`. Second, the default layers are now added to the `Builder` when it starts as opposed to when it ends. This allows for the default layers to be captured by module or external modules and then emitted correctly if needed (in the `Definition` or as FIRRTL `knownlayers`). Third, when an `Instance` is created, any layers that it has need to be added to the `Builder`. This can arise if a `Definition` is imported via an annotation and used to create a blackbox which will define additional `knownlayers`. If these layers are not added to the FIRRTL circuit, then this is illegal FIRRTL. Signed-off-by: Schuyler Eldridge <[email protected]>
Add an API to add
knownlayerspecifications to FIRRTLextmodules.This is a mechanism to declare that an external module was compiled with
support for specific layers. This then allows a FIRRTL comopiler compiler
to know that it should automatically enable these layers if needed.
Release Notes
Add
knownLayersAPI toBlackBoxandExternalModule. This allows for external modules to declare that they will contain layers, e.g., that they contain debug collateral. This allows a FIRRTL compiler using them to know that they should enable these layers in concert with the parent circuit.