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
📝 WalkthroughWalkthroughThe PR updates gem5 CI workflows to redirect to new KMHv3 variant launcher scripts (btb_18, btb_22, btb_align), introduces a new kmhv3_22.py configuration script for multi-core Xiangshan system setups, and modifies launcher scripts to target updated Python configuration paths. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 23-45: The workflow defines duplicate CI jobs: perf_test_spec06
duplicates perf_test_22_spec06 and perf_test_spec17 duplicates
perf_test_22_spec17, both using script_path ../kmh_v3_btb_22.sh and the same
benchmark_type; update the duplicated jobs to use the intended renamed scripts
or remove the duplicates—locate the job blocks named perf_test_22_spec06 and
perf_test_22_spec17 and either change their script_path to the correct variant
(e.g., kmh_v3_btb_18.sh or the intended renamed script) and/or adjust
benchmark_type as required by the PR, or delete these redundant job entries so
each test runs only once.
🧹 Nitpick comments (1)
configs/example/kmhv3_22.py (1)
135-136: Minor: RedundantsetMemClasscall.
xiangshan_system_init()already callsSimulation.setMemClass(args)internally (seeconfigs/common/xiangshan.pylines 502-503). This second call on line 136 is harmless but redundant.♻️ Suggested cleanup
- # Match the memories with the CPUs, based on the options for the test system - TestMemClass = Simulation.setMemClass(args) - test_sys = build_xiangshan_system(args)Note:
TestMemClassis assigned but never used in this script.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/gem5-ideal-btb-perf-nosc.yml.github/workflows/gem5-ideal-btb-perf-weekly.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
🧬 Code graph analysis (1)
configs/example/kmhv3_22.py (5)
configs/example/kmhv3_18.py (1)
setKmhV3IdealParams(22-120)src/mem/cache/replacement_policies/ReplacementPolicies.py (1)
XSDRRIPRP(158-166)src/mem/ruby/network/garnet/Router.hh (1)
direction(123-123)configs/common/xiangshan.py (2)
xiangshan_system_init(493-510)build_xiangshan_system(198-490)configs/common/Simulation.py (2)
setMemClass(95-98)run_vanilla(774-856)
🪛 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_nosc.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)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Quick Build, Unit Tests & Smoke Test
🔇 Additional comments (10)
util/xs_scripts/kmh_v3_btb_18_nosc.sh (1)
10-10: LGTM!The script correctly references the renamed config file
kmhv3_18.pyand preserves the--disable-mgscflag for the "no SC" variant. Shellcheck warnings are false positives as variables are sourced fromcommon.sh.util/xs_scripts/kmh_v3_btb_18.sh (1)
10-10: LGTM!The script correctly references the renamed config file
kmhv3_18.py. Shellcheck warnings are false positives as the variables are sourced fromcommon.sh..github/workflows/gem5-ideal-btb-perf-nosc.yml (1)
16-16: LGTM!The workflow correctly references the renamed script
kmh_v3_btb_18_nosc.sh, aligning with the script rename inutil/xs_scripts/.util/xs_scripts/kmh_v3_btb_22.sh (1)
10-10: Approved. The script correctly referenceskmhv3_22.pyand properly sourcescommon.shto obtaingem5andgem5_homevariables. The config file exists atconfigs/example/kmhv3_22.py.util/xs_scripts/kmh_v3_btb_align.sh (1)
1-10: LGTM - New align script follows established patterns.The script is well-structured and consistent with the other
kmh_v3_btb_*.shscripts in the directory. References tokmhv3_align.pyare correct, and the sourcing ofcommon.shproperly provides thegem5andgem5_homevariables, making any shellcheck warnings about them false positives.configs/example/kmhv3_22.py (3)
1-21: Imports and path setup look standard for gem5 configurations.The wildcard imports flagged by static analysis (F403, F405) are typical in gem5 configuration scripts, where symbols like
KMHV3Scheduler,XSDRRIPRP,Root, etc. are intentionally imported from m5.objects and common.xiangshan modules. This pattern is consistent with other config files in the codebase (e.g.,kmhv3_18.py).
23-121: Configuration function is well-structured and consistent with existing patterns.The
setKmhV3IdealParamsfunction mirrors the implementation inkmhv3_18.py, covering CPU pipeline stages, cache hierarchies, and L2 DRRIP replacement policy configuration. The parameter values and structure are consistent with the project's KMHv3 configuration conventions.
138-144: Main execution flow looks correct.The sequence of building the system, applying ideal parameters with highest priority, creating the root, and running the vanilla simulation follows established gem5 configuration patterns.
.github/workflows/gem5-ideal-btb-perf-weekly.yml (2)
11-21: Align test jobs updated correctly.The script path updates to
../kmh_v3_btb_align.shfor both SPEC06 and SPEC17 align tests are consistent with the PR's renaming objectives.
47-53: Commented section updated for consistency.The commented-out
perf_test_spec06_vectorjob has been updated to reference the new script path, maintaining consistency if this job is re-enabled later.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| perf_test_spec06: | ||
| uses: ./.github/workflows/gem5-perf-template.yml | ||
| with: | ||
| script_path: ../kmh_v3_ideal.sh | ||
| script_path: ../kmh_v3_btb_22.sh | ||
| benchmark_type: "spec06-1.0c" | ||
|
|
||
| perf_test_spec17: | ||
| uses: ./.github/workflows/gem5-perf-template.yml | ||
| with: | ||
| script_path: ../kmh_v3_ideal.sh | ||
| 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" |
There was a problem hiding this comment.
Duplicate job configurations detected.
perf_test_spec06 (lines 23-27) and perf_test_22_spec06 (lines 35-39) have identical configurations:
- Both use
script_path: ../kmh_v3_btb_22.sh - Both use
benchmark_type: "spec06-1.0c"
Similarly, perf_test_spec17 (lines 29-33) and perf_test_22_spec17 (lines 41-45) are identical.
This appears to be unintentional duplication that will run the same tests twice, wasting CI resources. Based on the PR objectives mentioning renames, should one of these pairs use a different script (e.g., kmh_v3_btb_18.sh or another variant)?
🤖 Prompt for AI Agents
In @.github/workflows/gem5-ideal-btb-perf-weekly.yml around lines 23 - 45, The
workflow defines duplicate CI jobs: perf_test_spec06 duplicates
perf_test_22_spec06 and perf_test_spec17 duplicates perf_test_22_spec17, both
using script_path ../kmh_v3_btb_22.sh and the same benchmark_type; update the
duplicated jobs to use the intended renamed scripts or remove the
duplicates—locate the job blocks named perf_test_22_spec06 and
perf_test_22_spec17 and either change their script_path to the correct variant
(e.g., kmh_v3_btb_18.sh or the intended renamed script) and/or adjust
benchmark_type as required by the PR, or delete these redundant job entries so
each test runs only once.
1 participant
重命名配置文件：
重命名脚本文件：
更新脚本文件中的配置文件路径引用：
更新 GitHub Actions workflow：
Change-Id: Ied453dba5571f51ff2e53cef13789228dc380d00
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.