Config: set minimal hartid width to 6 #2966
Conversation
|
Thanks for doing this! We may also need a config to split XSTop and XSCore to change something outside of XSCore faster. Just leave it as future work. |
The major issue here is the diplomacy. Getting the correct parameters for outside XSCore requires all LazyModule nodes. We have to use fake nodes (and make sure they are the same as XSCore) if we want to separate XSCore and uncore. Actually we have a transform to copy Core 0 to other cores. This reduces the FIRRTL -> verilog time but does not help the Chisel elaboration stage. We don't know how to fix the issue better. cc @sequencer |
Indeed.
Another way is to use LazyModule cloning, but we need to fix ChiselDB and ConstantIn first. |
Yes. No matter how we address the multicore issue, we need to ensure they are the same. ChiselDB and ConstantIn are destroying the dedup between multiple cores. |
Please use D/I... I'm prototyping the new diplomacy framework, but that needs a large refactor... |
Please wait, this line in cpl2 should also be fixed https://github.com/OpenXiangShan/CoupledL2/pull/102/files#diff-566b098f719cfe2cdea01bb41cd67c977a43516affa844e0512e321dd742170bR83 |
|
From my mind:
The current implementation in diplomacy has these design warts, which won't be supported:
I'm drafting a NoC-based SoC framework w/ this methodology, eta 6m to get it ready. |
|
I found a bug since the last time I modified CoupledL2:
val l2cache = coreParams.L2CacheParamsOpt.map(l2param =>
LazyModule(new CoupledL2()(new Config((_, _, _) => {
case L2ParamKey => l2param.copy(
hartIds = Seq(p(XSCoreParamsKey).HartId),Then, l2cache will only get a hartId sized 1. But we use the size of this List to construct hartId in CoupledL2:
lazy val msgSizeBits = edgeIn.bundle.sizeBits
lazy val sourceIdAll = 1 << sourceIdBits
lazy val hartIdLen: Int = log2Up(cacheParams.hartIds.length)
val mshrsAll = cacheParams.mshrsSo, currently, the hartIdLen in coupledL2 is wrong. I wanted to fix it today, but then I found many things that should be fixed first, especially for ConstantIn and ChiselDB support for hartid from io. If you want to use it temporarily, you can just apply the following patches with your branch: Xiangshan: diff --git a/src/main/scala/top/Top.scala b/src/main/scala/top/Top.scala
index 11b5b8806..cab8ad853 100644
--- a/src/main/scala/top/Top.scala
+++ b/src/main/scala/top/Top.scala
@@ -27,6 +27,7 @@ import device._
import chisel3.stage.ChiselGeneratorAnnotation
import org.chipsalliance.cde.config._
import freechips.rocketchip.diplomacy._
+import freechips.rocketchip.tile._
import freechips.rocketchip.tilelink._
import freechips.rocketchip.jtag.JTAGIO
import chisel3.experimental.{annotate, ChiselAnnotation}
@@ -74,6 +75,7 @@ class XSTop()(implicit p: Parameters) extends BaseXSSoc() with HasSoCParameter
hartIds = tiles.map(_.HartId),
FPGAPlatform = debugOpts.FPGAPlatform
)
+ case MaxHartIdBits => p(MaxHartIdBits)
})))
)
diff --git a/src/main/scala/xiangshan/L2Top.scala b/src/main/scala/xiangshan/L2Top.scala
index 7102237d3..182b87c8f 100644
--- a/src/main/scala/xiangshan/L2Top.scala
+++ b/src/main/scala/xiangshan/L2Top.scala
@@ -21,7 +21,7 @@ import org.chipsalliance.cde.config._
import chisel3.util.{Valid, ValidIO}
import freechips.rocketchip.diplomacy._
import freechips.rocketchip.interrupts._
-import freechips.rocketchip.tile.{BusErrorUnit, BusErrorUnitParams, BusErrors}
+import freechips.rocketchip.tile.{BusErrorUnit, BusErrorUnitParams, BusErrors, MaxHartIdBits}
import freechips.rocketchip.tilelink._
import coupledL2.{CoupledL2, L2ParamKey}
import system.HasSoCParameter
@@ -94,6 +94,7 @@ class L2Top()(implicit p: Parameters) extends LazyModule
hartIds = Seq(p(XSCoreParamsKey).HartId),
FPGAPlatform = debugOpts.FPGAPlatform
)
+ case MaxHartIdBits => p(MaxHartIdBits)
})))
)
val l2_binder = coreParams.L2CacheParamsOpt.map(_ => BankBinder(coreParams.L2NBanks, 64))submodule CoupledL2 diff --git a/src/main/scala/coupledL2/CoupledL2.scala b/src/main/scala/coupledL2/CoupledL2.scala
index e6cf2ae..4e743f9 100644
--- a/src/main/scala/coupledL2/CoupledL2.scala
+++ b/src/main/scala/coupledL2/CoupledL2.scala
@@ -23,6 +23,7 @@ import chisel3._
import chisel3.util._
import utility.{FastArbiter, ParallelMax, ParallelPriorityMux, Pipeline, RegNextN}
import freechips.rocketchip.diplomacy._
+import freechips.rocketchip.tile.MaxHartIdBits
import freechips.rocketchip.tilelink._
import freechips.rocketchip.tilelink.TLMessages._
import freechips.rocketchip.util._
@@ -83,7 +84,7 @@ trait HasCoupledL2Parameters {
lazy val msgSizeBits = edgeIn.bundle.sizeBits
lazy val sourceIdAll = 1 << sourceIdBits
- lazy val hartIdLen: Int = log2Up(cacheParams.hartIds.length)
+ lazy val hartIdLen: Int = p(MaxHartIdBits)
val mshrsAll = cacheParams.mshrs
val idsAll = 256// ids of L2 //TODO: Paramterize like this: max(mshrsAll * 2, sourceIdAll * 2)submodule HuanCun diff --git a/src/main/scala/huancun/HuanCun.scala b/src/main/scala/huancun/HuanCun.scala
index 5722a71..52f9eb9 100644
--- a/src/main/scala/huancun/HuanCun.scala
+++ b/src/main/scala/huancun/HuanCun.scala
@@ -23,6 +23,7 @@ import org.chipsalliance.cde.config.Parameters
import chisel3._
import chisel3.util._
import freechips.rocketchip.diplomacy._
+import freechips.rocketchip.tile.MaxHartIdBits
import freechips.rocketchip.tilelink._
import freechips.rocketchip.tilelink.TLMessages._
import freechips.rocketchip.util.{BundleField, BundleFieldBase, UIntToOH1}
@@ -103,7 +104,7 @@ trait HasHuanCunParameters {
lazy val outerSinkBits = edgeOut.bundle.sinkBits
- lazy val hartIdLen: Int = log2Up(cacheParams.hartIds.length)
+ lazy val hartIdLen: Int = p(MaxHartIdBits)
val block_granularity = if (!cacheParams.inclusive && cacheParams.clientCaches.nonEmpty) {
cacheParams.clientCaches.head.blockGranularity |
FYI, #2672 |
|
Also, by default, both ChiselDB and constantin are disabled. We can skip them first and fix them in the next step. |
There is an |
Before this commit, we use HartIds: Seq[Int] in L2Param. However, it will only contains one element since the code are copied from L3, and L2 is private to a core now, which confuse some develops and caused a bug [1]. To prevent it being happend again, we should use a single HartId here and refactor many its consumes to use single HartId. [1] OpenXiangShan/XiangShan#2966 (comment) Signed-off-by: Yangyu Chen <[email protected]>
Before this commit, we use HartIds: Seq[Int] in L2Param. However, it only contain one element since the code is copied from L3, and L2 is now private to a core, which confuses some developers and causes a bug [1]. To prevent it from happening again, we should use a single HartId here and refactor many of its consumers to use a single HartId. [1] OpenXiangShan/XiangShan#2966 (comment) Signed-off-by: Yangyu Chen <[email protected]>
* configs: use single hartid Before this commit, we use HartIds: Seq[Int] in L2Param. However, it only contain one element since the code is copied from L3, and L2 is now private to a core, which confuses some developers and causes a bug [1]. To prevent it from happening again, we should use a single HartId here and refactor many of its consumers to use a single HartId. [1] OpenXiangShan/XiangShan#2966 (comment) Signed-off-by: Yangyu Chen <[email protected]> * Bump HuanCun Signed-off-by: Yangyu Chen <[email protected]> --------- Signed-off-by: Yangyu Chen <[email protected]>
|
[Generated by IPC robot]
master branch:
|
This can help users who only build one core but then manually instantiate more than two cores in the SoC.
Since we have set the default HartIdBits to 6, we should also allow it to be overridden by parameters. Signed-off-by: Yangyu Chen <[email protected]>
|
[Generated by IPC robot]
master branch:
|
This can help users who only build one core but then manually instantiate more than two cores in the SoC. --------- Signed-off-by: Yangyu Chen <[email protected]> Co-authored-by: Yangyu Chen <[email protected]>
This can help users who only build one core but then manually instantiate more than two cores in the SoC.