Use BundleBridge to propagate tile input "constants" #2521

hcook · 2020-06-13T05:38:01Z

The "TileInputConstants" are increasingly less constant in certain configurations. Meanwhile, use of hartId: Int to generate circuits within the tile continues to occasionally foil deduplication across tiles. This PR attempts to address both issues by:

deprecating HasTileParmas.hartId and providing the more clearly named staticIdForMetadataUseOnly to replace it
using the new BundleBridgeNexus features in the subsystem to collect reset vectors from different sources (e.g. bootROMs), or per-tile subsystem inputs, and then broadcasting them to the tiles
using the new BundleBridgeNexus features in the subsystem to provide default hart id constant values into the tile, optionally merge them with prefixes driven by other components, or replace them with per-tile subsystem inputs, and then broadcasting them to the tiles
using BundleBroadcast within the tile to further propagate hart id and reset vector wires to tile submodules that have already been made diplomatic (i.e. everything other than the Rocket core pipeline itself).
misc cleanups of the parameterization of related tile width parameters
updating how ROMs are attached to the subsystem

Type of change: other enhancement

Impact: API modification

Development Phase: implementation

Release Notes

HasPeripheryBootROM and HasPeripheryBootROMModuleImp are removed and replaced by a call to BootROM.attach
BootROMParams Field is removed and replaced with BootROMLocated Field
MaskROMLocated Field is added
SubsystemExternalResetVectorKey, SubsystemExternalHartIdWidthKey and InsertTimingClosureRegistersOnHartIds Fields are added
Unused ResetVectorBits Field is removed
HasExternallyDrivenTileConstants bundle mixin is removed
HasResetVectorWire subsystem trait is removed
HasTileInputConstants and InstantiatesTiles subsystem traits are added
BaseTile exposes val hartIdNode: BundleBridgeNode[UInt] and resetVectorNode: BundleBridgeNode[UInt] and these are automatically connected to in HasTiles.
rocket.Frontend, rocket.ICache, rocket.DCache, rocket.NDCache now have BundleBridgeSink[UInt] for their reset vector or hartid wire inputs. If you instantiate them manually, i.e. not using the traits e.g. rocket.HasHellaCache, you will have to manually connect up those nodes to the aforementioned BaseTile nodes.

hcook · 2020-06-18T16:32:39Z

src/main/scala/system/ExampleRocketSystem.scala

+{
+  // optionally add ROM devices
+  val bootROM  = p(BootROMLocated(location)).map { BootROM.attach(_, this, CBUS) }
+  val maskROMs = p(MaskROMLocated(location)).map { MaskROM.attach(_, this, CBUS) }


@rmac-sifive These are candidate targets for a more generic HasDevices trait in rocket-chip, but note that the BootROM one needs more than just Attachable

src/main/scala/subsystem/HasTiles.scala

src/main/scala/diplomacy/BundleBridge.scala

src/main/scala/devices/tilelink/BootROM.scala

src/main/scala/rocket/Frontend.scala

src/main/scala/rocket/HellaCache.scala

mwachs5 · 2020-06-18T16:50:18Z

src/main/scala/rocket/ICache.scala

@@ -52,13 +52,14 @@ class ICacheErrors(implicit p: Parameters) extends CoreBundle()(p)
  val bus = Valid(UInt(width = paddrBits))
 }

-class ICache(val icacheParams: ICacheParams, val hartId: Int)(implicit p: Parameters) extends LazyModule {
+class ICache(val icacheParams: ICacheParams, val staticIdForMetadata: Int)(implicit p: Parameters) extends LazyModule {


maybe we should call this "staticIdForMetadataOnly"

yes Only seems like a useful addition in terms of my intended meaning

src/main/scala/subsystem/Configs.scala

src/main/scala/subsystem/HasTiles.scala

Co-authored-by: Megan Wachs <megan@sifive.com>

…ot HasTileParams

Co-authored-by: Megan Wachs <megan@sifive.com>

hcook · 2020-06-24T20:02:10Z

This PR's CI seems to be permanently stuck. Github action/cache always hits, but returns cache sizes of 0MB. Opening a new PR #2531 to replace this one seems to have magically fixed it.

chipsalliance/rocket-chip#2521 replaces case object BootROMParams extends Field[BootROMParams] with case class BootROMLocated(loc: HierarchicalLocation) extends Field[Option[BootROMParams]](None) This commit updates DromajoHelper to take a HierarchicalLocation and updates the lookup of DRAMOJO_RESET_VECTOR adn DRAMOJO_MMIO_START accordingly.

DecodeTheEncoded · 2021-02-26T07:21:49Z

src/main/scala/diplomacy/BundleBridge.scala

  def makeIO()(implicit valName: ValName): T = {
-    val io: T = IO(Flipped(chiselTypeClone(bundle)))
+    val io: T = IO(if (inferInput) Input(chiselTypeOf(bundle)) else Flipped(chiselTypeClone(bundle)))
    io.suggestName(valName.name)
    bundle <> io


I just felt confused here. By code reading, I felt like the direction of makeIO of BundleBridgeSourceNode is Input, While the direction of makeIO of BundleBridgeSinkNode is Output. I felt like this is strange, the source node only has output bundles and the sink only has input right? Why this is opposite here. Also, if inferInput is false in the makeIO of BundleBridgeSource, why we need to Flipped the orignal direction, while inside the BundleBridgeSink, there is no Flipped? it is wired. Seems like the io after calling makeIO in corresponding node is opposite with its original IO, why?
@hcook @mwachs5 @jackkoenig

I just felt confused here. By code reading, I felt like the direction of makeIO of BundleBridgeSourceNode is Input, While the direction of makeIO of BundleBridgeSinkNode is Output. I felt like this is strange, the source node only has output bundles and the sink only has input right? Why this is opposite here. Also, if inferInput is false in the makeIO of BundleBridgeSource, why we need to Flipped the orignal direction, while inside the BundleBridgeSink, there is no Flipped? it is wired. Seems like the io after calling makeIO in corresponding node is opposite with its original IO, why?
@hcook @mwachs5 @jackkoenig

I answer (more or less) this question in stackoverflow https://stackoverflow.com/questions/66963837/how-to-understand-the-flip-in-autobundle-and-in-makeios

hcook force-pushed the instantiate-all-tiles-before-connecting branch from b1d2bb1 to e5b737e Compare June 15, 2020 16:41

hcook force-pushed the bundle-bridge-tile-constants branch from bee7db8 to 230cf4f Compare June 15, 2020 23:55

hcook force-pushed the instantiate-all-tiles-before-connecting branch from e5b737e to 0286604 Compare June 16, 2020 20:53

hcook changed the base branch from instantiate-all-tiles-before-connecting to master June 18, 2020 16:09

hcook force-pushed the bundle-bridge-tile-constants branch from cc5a02e to f982cb0 Compare June 18, 2020 16:14

hcook changed the title ~~Use BundleBridge to propagate tile input constants~~ Use BundleBridge to propagate tile input "constants" Jun 18, 2020

hcook requested review from terpstra, aswaterman, jackkoenig, mwachs5 and rmac-sifive June 18, 2020 16:28

hcook commented Jun 18, 2020

View reviewed changes

src/main/scala/subsystem/HasTiles.scala Show resolved Hide resolved

hcook commented Jun 18, 2020

View reviewed changes

src/main/scala/diplomacy/BundleBridge.scala Show resolved Hide resolved