Conversation
重命名配置文件： - idealkmhv3.py -> kmhv3_18.py - kmhv3.py -> kmhv3_align.py 重命名脚本文件： - kmh_v3_btb_nosc.sh -> kmh_v3_btb_18_nosc.sh - kmh_v3_btb.sh -> kmh_v3_btb_align.sh - kmh_v3_ideal.sh -> kmh_v3_btb_22.sh 更新脚本文件中的配置文件路径引用： - kmh_v3_btb_align.sh: 更新为使用 kmhv3_align.py - kmh_v3_btb_18_nosc.sh: 更新为使用 kmhv3_18.py - kmh_v3_btb_22.sh: 使用 kmhv3_22.py 更新 GitHub Actions workflow： - gem5-ideal-btb-perf-weekly.yml: 更新所有脚本路径引用以匹配新文件名 Change-Id: Ied453dba5571f51ff2e53cef13789228dc380d00
重命名配置文件： - idealkmhv3.py -> kmhv3_18.py - kmhv3.py -> kmhv3_align.py 重命名脚本文件： - kmh_v3_btb_nosc.sh -> kmh_v3_btb_18_nosc.sh - kmh_v3_btb.sh -> kmh_v3_btb_align.sh - kmh_v3_ideal.sh -> kmh_v3_btb_22.sh 更新脚本文件中的配置文件路径引用： - kmh_v3_btb_align.sh: 更新为使用 kmhv3_align.py - kmh_v3_btb_18_nosc.sh: 更新为使用 kmhv3_18.py - kmh_v3_btb_22.sh: 使用 kmhv3_22.py 更新 GitHub Actions workflow： - gem5-ideal-btb-perf-weekly.yml: 更新所有脚本路径引用以匹配新文件名 - manual-perf.yml: 更新配置选项和脚本路径映射，支持新的配置文件命名 * 移除旧选项 idealkmhv3.py 和 kmhv3.py * 新增选项 kmhv3_align.py、kmhv3_18.py、kmhv3_22.py * 更新默认配置为 kmhv3_18.py - pr-quick-check.yml: 更新配置文件路径引用 (kmhv3.py -> kmhv3_align.py) Change-Id: Ied453dba5571f51ff2e53cef13789228dc380d00
📝 WalkthroughWalkthroughThese changes introduce and integrate new gem5 configuration variants (kmhv3_18, kmhv3_22, kmhv3_align) by updating GitHub workflow scripts and adding a new Python configuration file that replaces existing configurations across performance testing and PR validation pipelines. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/gem5-ideal-btb-perf-weekly.yml:
- Around line 35-45: The workflow contains duplicate jobs: perf_test_22_spec06
and perf_test_22_spec17 are identical to existing perf_test_spec06 and
perf_test_spec17 (same script_path and benchmark_type), causing redundant CI
runs; either remove the redundant perf_test_22_* jobs or modify them to be
intentionally distinct (e.g., change script_path, add a distinguishing input
like variant: "v22", set a descriptive name/comment, or add env/input
parameters) so their purpose is clear—update the job blocks for
perf_test_22_spec06 and perf_test_22_spec17 accordingly or delete them to
eliminate duplicate execution.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/gem5-ideal-btb-perf-nosc.yml.github/workflows/gem5-ideal-btb-perf-weekly.yml.github/workflows/manual-perf.yml.github/workflows/pr-quick-check.ymlconfigs/example/kmhv3_18.pyconfigs/example/kmhv3_22.pyconfigs/example/kmhv3_align.pyutil/xs_scripts/kmh_v3_btb_18.shutil/xs_scripts/kmh_v3_btb_18_nosc.shutil/xs_scripts/kmh_v3_btb_22.shutil/xs_scripts/kmh_v3_btb_align.sh
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-16T08:28:53.334Z
Learnt from: Lingrui98
Repo: OpenXiangShan/GEM5 PR: 649
File: configs/common/xiangshan.py:0-0
Timestamp: 2025-12-16T08:28:53.334Z
Learning: In XiangShan trace mode (configs/common/xiangshan.py), bootloaders are not needed. The correct approach is to set test_sys.workload.bootloader = '' (empty string) to indicate no bootloader, rather than providing a bootloader path.
Applied to files:
configs/example/kmhv3_22.py
🪛 Ruff (0.14.11)
configs/example/kmhv3_22.py
7-7: from m5.objects import * used; unable to detect undefined names
(F403)
9-9: from m5.util.fdthelper import * used; unable to detect undefined names
(F403)
15-15: from common.FSConfig import * used; unable to detect undefined names
(F403)
16-16: from common.SysPaths import * used; unable to detect undefined names
(F403)
17-17: from common.Benchmarks import * used; unable to detect undefined names
(F403)
19-19: from common.Caches import * used; unable to detect undefined names
(F403)
20-20: from common.xiangshan import * used; unable to detect undefined names
(F403)
49-49: KMHV3Scheduler may be undefined, or defined from star imports
(F405)
106-106: XSDRRIPRP may be undefined, or defined from star imports
(F405)
115-115: LayerBandwidthConfig may be undefined, or defined from star imports
(F405)
116-116: LayerBandwidthConfig may be undefined, or defined from star imports
(F405)
126-126: xiangshan_system_init may be undefined, or defined from star imports
(F405)
138-138: build_xiangshan_system may be undefined, or defined from star imports
(F405)
142-142: Root may be undefined, or defined from star imports
(F405)
🪛 Shellcheck (0.11.0)
util/xs_scripts/kmh_v3_btb_22.sh
[warning] 10-10: gem5 is referenced but not assigned.
(SC2154)
[warning] 10-10: gem5_home is referenced but not assigned.
(SC2154)
util/xs_scripts/kmh_v3_btb_18.sh
[warning] 10-10: gem5 is referenced but not assigned.
(SC2154)
[warning] 10-10: gem5_home is referenced but not assigned.
(SC2154)
util/xs_scripts/kmh_v3_btb_align.sh
[warning] 10-10: gem5 is referenced but not assigned.
(SC2154)
[warning] 10-10: gem5_home is referenced but not assigned.
(SC2154)
util/xs_scripts/kmh_v3_btb_18_nosc.sh
[warning] 10-10: gem5 is referenced but not assigned.
(SC2154)
[warning] 10-10: gem5_home is referenced but not assigned.
(SC2154)
🔇 Additional comments (12)
util/xs_scripts/kmh_v3_btb_18_nosc.sh (1)
10-10: LGTM!The update to
kmhv3_18.pyis consistent withkmh_v3_btb_18.sh, and the--disable-mgscflag is correctly preserved. Shellcheck warnings are false positives since variables come fromcommon.sh..github/workflows/gem5-ideal-btb-perf-nosc.yml (1)
16-16: LGTM!The script path update to
kmh_v3_btb_18_nosc.shaligns with the corresponding shell script changes in this PR..github/workflows/pr-quick-check.yml (1)
38-38: Configuration path updates are correct.Both the PR branch test (line 38) and base branch test (line 70) now use
kmhv3_align.pyconsistently, which ensures accurate IPC comparison. The configuration fileconfigs/example/kmhv3_align.pyexists in the repository.Also applies to: 70-70
util/xs_scripts/kmh_v3_btb_align.sh (1)
1-10: New script follows established patterns and all referenced resources exist.The script structure is consistent with other
kmh_v3_btb_*.shscripts. Theconfigs/example/kmhv3_align.pyconfiguration file exists, and thegem5variable is properly defined incommon.sh.util/xs_scripts/kmh_v3_btb_18.sh (1)
10-10: Verify thatkmhv3_18.pyexists.The script path update is correct—
configs/example/kmhv3_18.pyexists in the repository.The Shellcheck warnings about
gem5andgem5_homeare false positives. Both variables are defined and exported incommon.sh(sourced on line 4):gem5_homeis set to the repository root directory, andgem5is set to the GEM5 executable path. Although the validation loop only checksgem5_home, both variables are available when the script executes.configs/example/kmhv3_22.py (3)
1-21: Star imports are acceptable for gem5 config files.The star imports flagged by Ruff (F403/F405) are a standard pattern in gem5 configuration scripts. Symbols like
KMHV3Scheduler,XSDRRIPRP,LayerBandwidthConfig,Root,xiangshan_system_init, andbuild_xiangshan_systemare provided by these imports. This is consistent with gem5's configuration conventions.
23-121: Configuration function looks well-structured.The
setKmhV3IdealParamsfunction properly configures CPU and cache parameters with clear sectioning. The DRRIP replacement policy calculation (512KB slice with 8-way, 64B line → 1024 sets) at line 106 is consistent with the comment.
123-144: Verify the ordering of argument modifications.The hardcoded values for
args.bp_typeandargs.l2_sizeare set afterxiangshan_system_init()but beforebuild_xiangshan_system(). Confirm this ordering is correct by checking: (1) whetherxiangshan_system_init()also sets these values (safe to override), and (2) whetherbuild_xiangshan_system()depends on these being set during init rather than before the build call.util/xs_scripts/kmh_v3_btb_22.sh (1)
10-10: LGTM!The script correctly updates the Python target from
idealkmhv3.pytokmhv3_22.py. The Shellcheck warnings aboutgem5andgem5_homeare false positives—these variables are defined incommon.shwhich is sourced at line 4..github/workflows/gem5-ideal-btb-perf-weekly.yml (1)
14-21: Script path updates look correct.The align tests now correctly reference
kmh_v3_btb_align.sh..github/workflows/manual-perf.yml (2)
10-15: Configuration options updated correctly.The new configuration choices (
kmhv3_align.py,kmhv3_18.py,kmhv3_22.py) withkmhv3_18.pyas the default align with the PR's renaming objective.
55-67: Case mapping and error handling look good.The configuration-to-script mapping is consistent:
kmhv3_align.py→kmh_v3_btb_align.shkmhv3_18.py→kmh_v3_btb_18.shkmhv3_22.py→kmh_v3_btb_22.shThe default error case with
exit 1ensures unsupported configurations fail fast.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| perf_test_22_spec06: | ||
| uses: ./.github/workflows/gem5-perf-template.yml | ||
| with: | ||
| script_path: ../kmh_v3_btb_22.sh | ||
| benchmark_type: "spec06-1.0c" | ||
|
|
||
| perf_test_22_spec17: | ||
| uses: ./.github/workflows/gem5-perf-template.yml | ||
| with: | ||
| script_path: ../kmh_v3_btb_22.sh | ||
| benchmark_type: "spec17-1.0c" |
There was a problem hiding this comment.
Duplicate jobs detected: perf_test_22_spec06 and perf_test_22_spec17 are identical to perf_test_spec06 and perf_test_spec17.
Both perf_test_spec06 (lines 23-27) and perf_test_22_spec06 (lines 35-39) use:
script_path: ../kmh_v3_btb_22.shbenchmark_type: "spec06-1.0c"
Similarly, perf_test_spec17 and perf_test_22_spec17 are identical. This will run the same tests twice per workflow execution, wasting CI resources.
If these duplicates are intentional (e.g., for comparing results), please add distinguishing parameters or comments explaining the purpose. Otherwise, consider removing the duplicate jobs.
Proposed fix: Remove duplicate jobs
perf_test_spec17:
uses: ./.github/workflows/gem5-perf-template.yml
with:
script_path: ../kmh_v3_btb_22.sh
benchmark_type: "spec17-1.0c"
- perf_test_22_spec06:
- uses: ./.github/workflows/gem5-perf-template.yml
- with:
- script_path: ../kmh_v3_btb_22.sh
- benchmark_type: "spec06-1.0c"
-
- perf_test_22_spec17:
- uses: ./.github/workflows/gem5-perf-template.yml
- with:
- script_path: ../kmh_v3_btb_22.sh
- benchmark_type: "spec17-1.0c"
-
# perf_test_spec06_vector:🤖 Prompt for AI Agents
In @.github/workflows/gem5-ideal-btb-perf-weekly.yml around lines 35 - 45, The
workflow contains duplicate jobs: perf_test_22_spec06 and perf_test_22_spec17
are identical to existing perf_test_spec06 and perf_test_spec17 (same
script_path and benchmark_type), causing redundant CI runs; either remove the
redundant perf_test_22_* jobs or modify them to be intentionally distinct (e.g.,
change script_path, add a distinguishing input like variant: "v22", set a
descriptive name/comment, or add env/input parameters) so their purpose is
clear—update the job blocks for perf_test_22_spec06 and perf_test_22_spec17
accordingly or delete them to eliminate duplicate execution.
1 participant
重命名配置文件：
重命名脚本文件：
更新脚本文件中的配置文件路径引用：
更新 GitHub Actions workflow：
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.