XiangShan
XiangShan copied to clipboard
Config: set minimal hartid width to 6
This can help users who only build one core but then manually instantiate more than two cores in the SoC.
Thanks for doing this! Looks good to me.
We may also need a config to split XSTop and XSCore to change something outside of XSCore faster. Just leave it as future work.
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
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.
Indeed.
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.
Another way is to use LazyModule cloning, but we need to fix ChiselDB and ConstantIn first.
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.
We don't know how to fix the issue better. cc @sequencer
Please use D/I... I'm prototyping the new diplomacy framework, but that needs a large refactor...
This can help users who only build one core but then manually instantiate more than two cores in the SoC.
Please wait, this line in cpl2 should also be fixed https://github.com/OpenXiangShan/CoupledL2/pull/102/files#diff-566b098f719cfe2cdea01bb41cd67c977a43516affa844e0512e321dd742170bR83
From my mind:
- All
LazyModule
should only be aSerializableModule
that only depends on aSerializableModuleParameter
. In this case, allSerializableModule
can be safely instantiated via D/I. and is cacheable in Chisel. - All
LazyModule
should be aFixedIOModule
that can instantiate its Interface(IO) before the Module elaboration.
The current implementation in diplomacy has these design warts, which won't be supported:
- CDE.
- implicit parameters.
- In-memory linking from
Edge
toBundle
.
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:
src/main/scala/xiangshan/L2Top.scala:91
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:
coupledL2/src/main/scala/coupledL2/CoupledL2.scala:84
lazy val msgSizeBits = edgeIn.bundle.sizeBits
lazy val sourceIdAll = 1 << sourceIdBits
lazy val hartIdLen: Int = log2Up(cacheParams.hartIds.length)
val mshrsAll = cacheParams.mshrs
So, 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
I found many things that should be fixed first, especially for ConstantIn and ChiselDB support for hartid from io.
FYI, https://github.com/OpenXiangShan/XiangShan/pull/2672
Also, by default, both ChiselDB and constantin are disabled. We can skip them first and fix them in the next step.
Also, by default, both ChiselDB and constantin are disabled. We can skip them first and fix them in the next step.
There is an AlwaysBasicDB
which mainly used by tllog. It may make dedup fail.
[Generated by IPC robot] commit: f4b08c240986b6eb3a1c9e9bc06da4dd8cd75e3d
commit | astar | copy_and_run | coremark | gcc | gromacs | lbm | linux | mcf | microbench | milc | namd | povray | wrf | xalancbmk |
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
f4b08c2 | 1.854 | 0.449 | 2.103 | 1.182 | 2.170 | 2.181 | 2.329 | 0.973 | 1.386 | 1.294 | 2.742 | 2.605 | 2.283 | 2.951 |
master branch:
commit | astar | copy_and_run | coremark | gcc | gromacs | lbm | linux | mcf | microbench | milc | namd | povray | wrf | xalancbmk |
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
1e018fb | 1.854 | 0.450 | 2.103 | 1.190 | 2.468 | 2.593 | 2.330 | 0.962 | 1.377 | 1.427 | 3.123 | 2.639 | 2.451 | 2.960 |
3c71816 | 1.854 | 0.450 | 2.103 | 1.190 | 2.468 | 2.593 | 2.330 | 0.962 | 1.377 | 1.427 | 3.123 | 2.639 | 2.451 | 2.960 |
dfe034b | 1.854 | 0.449 | 2.103 | 1.182 | 2.170 | 2.181 | 2.328 | 0.972 | 1.386 | 1.294 | 2.742 | 2.592 | 2.283 | 2.951 |
bdc1606 | 1.854 | 0.449 | 2.101 | 1.182 | 2.170 | 2.181 | 2.329 | 0.973 | 1.386 | 1.294 | 2.742 | 2.589 | 2.283 | 2.951 |
dc5a918 | 1.854 | 0.449 | 2.103 | 1.181 | 2.170 | 2.181 | 2.329 | 0.973 | 1.386 | 1.294 | 2.742 | 2.589 | 2.283 | 2.951 |
bad6084 | 1.854 | 0.449 | 2.103 | 1.182 | 2.170 | 2.181 | 2.329 | 0.972 | 1.386 | 1.294 | 2.742 | 2.589 | 2.283 | 2.951 |
c686adc | 1.854 | 0.449 | 2.103 | 1.182 | 2.170 | 2.181 | 2.335 | 0.972 | 1.386 | 1.294 | 2.742 | 2.589 | 2.283 | 2.951 |
bc3d558 | 1.854 | 0.449 | 2.100 | 1.182 | 2.170 | 2.181 | 2.329 | 0.972 | 1.380 | 1.294 | 2.742 | 2.589 | 2.283 | 2.951 |
a58f171 | 1.854 | 0.449 | 2.104 | 1.182 | 2.170 | 2.181 | 2.329 | 0.972 | 1.386 | 1.294 | 2.742 | 2.586 | 2.283 | 2.951 |
ff74867 | 1.898 | 0.448 | 2.105 | 1.186 | 2.173 | 2.175 | 2.335 | 0.960 | 1.372 | 1.288 | 2.745 | 2.583 | 2.285 | 2.958 |
[Generated by IPC robot] commit: 083769034d825678d17e79be1594602be0700ea8
commit | astar | copy_and_run | coremark | gcc | gromacs | lbm | linux | mcf | microbench | milc | namd | povray | wrf | xalancbmk |
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
0837690 | 1.854 | 0.450 | 2.103 | 1.190 | 2.468 | 2.593 | 2.330 | 0.962 | 1.377 | 1.427 | 3.123 | 2.639 | 2.451 | 2.960 |
master branch:
commit | astar | copy_and_run | coremark | gcc | gromacs | lbm | linux | mcf | microbench | milc | namd | povray | wrf | xalancbmk |
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
5e237ba | ||||||||||||||
363530d | ||||||||||||||
a72b131 | ||||||||||||||
05d833a | 1.854 | 0.450 | 2.103 | 1.190 | 2.468 | 2.593 | 2.330 | 0.961 | 1.377 | 1.427 | 3.123 | 2.639 | 2.451 | 2.960 |
9cb05b4 | 0.450 | 2.103 | 2.330 | 1.377 | 2.639 | |||||||||
4daa5bf | 0.450 | 2.103 | 1.190 | 2.468 | 2.593 | 2.329 | 0.962 | 1.377 | 1.427 | 3.123 | 2.639 | 2.960 | ||
1e018fb | 1.854 | 0.450 | 2.103 | 1.190 | 2.468 | 2.593 | 2.330 | 0.962 | 1.377 | 1.427 | 3.123 | 2.639 | 2.451 | 2.960 |
3c71816 | 1.854 | 0.450 | 2.103 | 1.190 | 2.468 | 2.593 | 2.330 | 0.962 | 1.377 | 1.427 | 3.123 | 2.639 | 2.451 | 2.960 |
dfe034b | 1.854 | 0.449 | 2.103 | 1.182 | 2.170 | 2.181 | 2.328 | 0.972 | 1.386 | 1.294 | 2.742 | 2.592 | 2.283 | 2.951 |
bdc1606 | 1.854 | 0.449 | 2.101 | 1.182 | 2.170 | 2.181 | 2.329 | 0.973 | 1.386 | 1.294 | 2.742 | 2.589 | 2.283 | 2.951 |