chipsalliance / chipsalliance/rocket-chip
BaseTile Auxiliary Constructors Bug
- Dominant language
- Scala
- Stars
- 3.9k
- Forks
- 1.3k
- Avg merge
- 5d 13m
- Merged PRs (30d)
- 1
Description
Inside the `BaseTile` there is `val visibilityNode = p(TileVisibilityNodeKey)` _(it represents the view that the intra-tile masters have of the rest of the system)_. So, conceptually this diolomatic node should be in the scope of the Tile(Subclassing from the BaseTile---> RocketTile). However, the resulting location of this node is actually in the parent scope of the Tile, which is the `tile_reset_domain`(I change the Type from `TLEphemeralNode` to `TLIdentityNode` so that it appears on the graph):

The reason why this happens has to do with the BaseTile auxiliary constructor:
```
abstract class BaseTile private (val crossing: ClockCrossingType, q: Parameters)
extends LazyModule()(q)
with CrossesToOnlyOneClockDomain
with HasNonDiplomaticTileParameters
with HasLogicalTreeNode
{
// Public constructor alters Parameters to supply some legacy compatibility keys
def this(tileParams: TileParams, crossing: ClockCrossingType, lookup: LookupByHartIdImpl, p: Parameters) = {
this(crossing, p.alterMap(Map(
TileKey -> tileParams,
TileVisibilityNodeKey -> { println("LazyScope shoot1 " + LazyModule.scope ); TLEphemeralNode()(ValName("tile_master"))},
LookupByHartId -> lookup
)))
println("LazyScope shoot2 " + LazyModule.scope)
}
```
Before executing the constructor of the parent class` LazyModule()(q)` (inside which the tile itself will be rightly pushed onto the `LazyModule.scope` stack ), the parameter `q` should be evaluated first, the` q` actually refers to :
```
p.alterMap(Map(
TileKey -> tileParams,
TileVisibilityNodeKey -> TLEphemeralNode()(ValName("tile_master")),
LookupByHartId -> lookup
))
```
During the instantiation of `TLEphemeralNode()(ValName("tile_master")` scope of the node will be set (`val scope: Option[LazyModule] = LazyModule.scope`), however, the LazyModule.scope now is actually `tile_reset_domain`; Concequently the socpe of the pre instantiated TLEphemeralNode is the LazyModule.scope before the tile is pushed onto the stack, which is the resetDomain. This is just **not** right.
I am actually not certain this is an actual bug or an intended behavior. Can you guys check this ?
@hcook @sequencer
Contributor guide
Research direction
Start with the BaseTile auxiliary constructor and trace evaluation of its altered Parameters, then inspect LazyModule.scope and the LazyModule parent-constructor path. Compare the resulting TileVisibilityNode scope with the expected Tile scope shown in the issue's graph; done means the behavior is confirmed as intended or the scope issue is corrected and verified.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- scala
- Domain
- embedded-iot, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100