Bump chipyard to integrate mempress #1253
Conversation
| +/* lazy val sha3 = (project in file("generators/sha3")) */ | ||
| +/* .dependsOn(rocketchip, midasTargetUtils) */ | ||
| +/* .settings(libraryDependencies ++= rocketLibDeps.value) */ | ||
| +/* .settings(chiselTestSettings) */ | ||
| +/* .settings(commonSettings) */ |
There was a problem hiding this comment.
Can you keep the same type of comments // instead of /**/? This matters for tutorial users.
| new chipyard.config.WithExtMemIdBits(7) ++ | ||
| new freechips.rocketchip.subsystem.WithNMemoryChannels(4) ++ | ||
| new chipyard.config.WithSystemBusWidth(128) ++ | ||
| new RocketConfig) |
There was a problem hiding this comment.
Please also have the full package path for this as well.
| @@ -21,6 +21,14 @@ class TinyRocketConfig extends Config( | |||
| new freechips.rocketchip.subsystem.With1TinyCore ++ // single tiny rocket-core | |||
| new chipyard.config.AbstractConfig) | |||
|
|
|||
| class MempressConfig extends Config( | |||
| new mempress.WithMemPress ++ | |||
There was a problem hiding this comment.
Add a small descriptive comment on what this is doing.
There was a problem hiding this comment.
Generally this looks fine (I think the comments could be cleaned up a bit), but what do you want the mempress config to look like? Want it to be beefy (match the mem. heir. to something more realistic) or do you want to just show how to integrate the mempress accel. If the former is the case, we should change the cache to be better (which I think you reverted to the normal size).
|
@abejgonzalez
I want it to match the memory hierarchy, but if I do something like WithInclusiveCache(nBanks=8, nWays=16, capacityKB=2048), CI gives me an error. I'm not sure if I can see the CI errors again, but it was something along the lines of "nWays is in parameter position 1".
…On Oct 12, 2022 11:11 PM -0700, Abraham Gonzalez ***@***.***>, wrote:
@abejgonzalez commented on this pull request.
Generally this looks fine (I think the comments could be cleaned up a bit), but what do you want the mempress config to look like? Want it to be beefy (match the mem. heir. to something more realistic) or do you want to just show how to integrate the mempress accel. If the former is the case, we should change the cache to be better (which I think you reverted to the normal size).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
@joey0320 Most likely the API for WithInclusveCache has changed. Now you need to use |
|
I think there should be a single test with the beefy-mem config, as well as some documentation on how to use this, and its capabilities. |
Related PRs / Issues:
Type of change:
Impact:
Contributor Checklist:
mainas the base branch?changelog:<topic>label?changelog:label?.conda-lock.ymlfile if you updated the conda requirements file?Please Backport?