Skip to content
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

Preprocess: skip loadEvent for single-core #561

Merged
merged 1 commit into from
Feb 5, 2025
Merged

Preprocess: skip loadEvent for single-core #561

merged 1 commit into from
Feb 5, 2025

Conversation

klin02
Copy link
Member

@klin02 klin02 commented Feb 2, 2025

After optimized by Squash, DPIC bytes of LoadEvent accounts for nearly 30% of total. However in do_load_check(difftest.cpp), LoadEvent will only be used when numCores greater than 1, so we skip LoadEvent for singleCore.

After optimized by Squash, DPIC bytes of LoadEvent accounts for
nearly 30% of total. However in do_load_check(difftest.cpp),
LoadEvent will only be used when numCores greater than 1, so we skip
LoadEvent for singleCore.
@klin02
Copy link
Member Author

klin02 commented Feb 2, 2025

BTW, according to DPIC data recorded by Query, Increment transmitting works for other bundle taking up major part of DPIC, including RegState/CSR/RefillEvent, but not LoadEvent.
LoadEvent accounts for a large part of DPIC, but will not be checked for single-core. We skip LoadEvent to highlight the benefits of Incremental.

@klin02 klin02 requested a review from poemonsense February 5, 2025 10:59
Copy link
Member

@poemonsense poemonsense left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may also disable DPI-C calls of LoadEvent in single-core configs (possibly by adding ifdef NUM_CORES in DPI-C functions?)

@klin02
Copy link
Member Author

klin02 commented Feb 5, 2025

We may also disable DPI-C calls of LoadEvent in single-core configs (possibly by adding ifdef NUM_CORES in DPI-C functions?)

Maybe just leave DiffLoadEvent ungenerated for single-core?
Now with Preprocess we will skip LoadEvent if we use any difftest-config(then we will use GatewayEndpoint), shall we also skip it by default (now used in common verilator flow)?

@poemonsense
Copy link
Member

We may also disable DPI-C calls of LoadEvent in single-core configs (possibly by adding ifdef NUM_CORES in DPI-C functions?)

Maybe just leave DiffLoadEvent ungenerated for single-core? Now with Preprocess we will skip LoadEvent if we use any difftest-config(then we will use GatewayEndpoint), shall we also skip it by default (now used in common verilator flow)?

Someone may need to generated these C++ interfaces in other use cases. So if we generate them but with a macro #if NUM_CORES > 1 then they also work. This can be done in the future. I think in verilator flow the loadEvent should not bring significant overhead.

@klin02 klin02 merged commit 153ee37 into master Feb 5, 2025
5 checks passed
@klin02 klin02 deleted the skip-load branch February 5, 2025 14:56
klin02 added a commit to OpenXiangShan/XiangShan that referenced this pull request Feb 10, 2025
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants