chipsalliance / chipsalliance/rocket-chip

BaseTile Auxiliary Constructors Bug

Open
#2,985 5 comments 0 reactions 0 assignees View on GitHub
bug? question
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):
![image](https://user-images.githubusercontent.com/1823246/168067138-33ebc4b6-e60f-4e37-96fe-9f631f55d2ae.png)
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.