-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Batch: disable split strategy for FPGA to reduce gates #562
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor style improvements.
Maybe we should let @xiaokamikami known more on the functionalities and help review them in the future. It seems difficult for me to know all details because I don't participate in daily discussions.
src/main/scala/Batch.scala
Outdated
assert(remain_stats.info_size + 1.U <= param.MaxInfoSize.U) | ||
val data_exceed = Wire(Bool()) | ||
val info_exceed = Wire(Bool()) | ||
val step_all_exceed = Option.when(config.batchSplit)(Wire(Bool())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For so many Option.when
, it is better to use
if (config.batchSplit) {
}
else {
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we seperate logic of data split and state update, some signals can only be declared as Option. If we wrap them in if/else, it seems hard to cover some shared logic in same if/else.
src/main/scala/Batch.scala
Outdated
val append_finish_map = Seq.tabulate(param.StepGroupSize) { g => | ||
(g.U, (BatchFinish.asUInt << (g * param.infoWidth)).asUInt) | ||
|
||
val append_data = if (config.batchSplit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the same. Readability can be improved
I have no objections to this approach. Regarding the FPGA, reducing resource utilization is beneficial, and the data configuration can be fine-tuned based on the available PCIe bandwidth. |
4afa091
to
bbe919b
Compare
Previous we enable splitting Batch data to handle peak data valid in same cycle (we call these data collected in same cycle Stepdata). The step data may be splitting according to remain space of Batch state, and part of them will be appended to Batch output to make full use of Batch DPIC. However, this split strategy need some gates to handle splitting and appending behaviour. Now for FPGA, we need to reduce gates of Batch, so we disable split strategy. According to gateCount of Palladium, disable split strategy will reduce gates of Batch from 12M to 8M (in full Difftest).
This change wrap all middle variable inside if/else to avoid too many Options. |
* commit: b537f528bbb9e400b9d0da8756219a5f6d107be9 Including: * fix(xdma): remove replicate data parsing when USE_THREAD_MEMPOOL (OpenXiangShan/difftest#555) * Preprocess: move from Gateway to seperate file (OpenXiangShan/difftest#559) * Batch: collect from different groups in one cycle to reduce gates (OpenXiangShan/difftest#558) * feat(query): support query for difftest-dpic data (OpenXiangShan/difftest#557) * PerfCnt: count and sum for each DiffState when Batch (OpenXiangShan/difftest#560) * Preprocess: skip loadEvent for single-core (OpenXiangShan/difftest#561) * vcs: Refact DifftestEndpoint by split large always-block into piece (OpenXiangShan/difftest#565) * vcs: Ensure correct execution order of difftest DPI calls (OpenXiangShan/difftest#563) * Batch: disable split strategy for FPGA to reduce gates (OpenXiangShan/difftest#562) * fix(elfloader): Sort phdr entries by paddr before return to readFromElf() (OpenXiangShan/difftest#566) * Batch: rename BatchInterval to BatchStep, move to tail of StepInfo (OpenXiangShan/difftest#564)
Previous we enable splitting Batch data to handle peak data valid in same cycle (we call these data collected in same cycle Stepdata). The step data may be splitting according to remain space of Batch state, and part of them will be appended to Batch output to make full use of Batch DPIC.
However, this split strategy need some gates to handle splitting and appending behaviour. Now for FPGA, we need to reduce gates of Batch, so we disable split strategy.
According to gateCount of Palladium, disable split strategy will reduce gates of Batch from 12M to 8M (in full Difftest). And in FPGA, LUT of XS and Difftest(Basic-diff) will reduce from 81% to 77%, with XS accounts for 70% itself.