Skip to content

Revert "mem: Add 8 channel ddr profiling (#703)"#716

Merged
Ergou-ren merged 1 commit intoxs-devfrom
revert_ddr
Jan 15, 2026
Merged

Revert "mem: Add 8 channel ddr profiling (#703)"#716
Ergou-ren merged 1 commit intoxs-devfrom
revert_ddr

Conversation

@Ergou-ren
Copy link
Copy Markdown
Collaborator

@Ergou-ren Ergou-ren commented Jan 15, 2026

This reverts commit 081db5f.

Summary by CodeRabbit

  • Chores
    • Updated default DRAM memory configuration from 8-channel to 2-channel DDR4 across all simulation workflows and tools.
    • Removed legacy 8-channel configuration file.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

This PR updates the DRAMSim3 configuration across the codebase from an 8-channel DDR4 setup to a 2-channel DDR4 setup. The 8-channel ini file is deleted, and all references in configuration files and scripts are updated to point to the 2-channel ini instead.

Changes

Cohort / File(s) Summary
Workflow Configuration
.github/workflows/autotest/gem5-vec.cfg, .github/workflows/autotest/gem5.cfg
Updated dramsim3-ini path from 8-channel to 2-channel configuration
Python Configuration
configs/common/xiangshan.py, configs/example/se.py
Changed default DRAMSim3 ini file reference from 8-channel to 2-channel DDR4 configuration
Shell Scripts
util/memory_check/run-xs-with-valgrind.sh, util/pgo/basic_pgo.sh, util/runse.sh
Updated --dramsim3-ini parameter in gem5 invocations to reference 2-channel configuration
Configuration File Removal
ext/dramsim3/xiangshan_configs/xiangshan_DDR4_8Gb_x8_3200_8ch.ini
Deleted 8-channel DRAMSim3 configuration file (67 lines)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • mem: Add 8 channel ddr profiling #703: Previously switched defaults to the 8-channel xiangshan_DDR4_8Gb_x8_3200_8ch.ini configuration; this PR reverts to the 2-channel variant across the same config files.

Suggested reviewers

  • jensen-yan
  • tastynoob

Poem

🐰 From eight channels down to two so neat,
Our memory sim takes lighter feet,
Old configs fade, the new ones sing,
A simpler DRAM, the change we bring! 🎵

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: reverting a previous commit that added 8-channel DDR profiling, as evidenced by the systematic replacement of 8ch.ini with 2ch.ini files across all configuration files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 081db5f and a102bfa.

📒 Files selected for processing (8)
  • .github/workflows/autotest/gem5-vec.cfg
  • .github/workflows/autotest/gem5.cfg
  • configs/common/xiangshan.py
  • configs/example/se.py
  • ext/dramsim3/xiangshan_configs/xiangshan_DDR4_8Gb_x8_3200_8ch.ini
  • util/memory_check/run-xs-with-valgrind.sh
  • util/pgo/basic_pgo.sh
  • util/runse.sh
💤 Files with no reviewable changes (1)
  • ext/dramsim3/xiangshan_configs/xiangshan_DDR4_8Gb_x8_3200_8ch.ini
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-16T08:28:46.184Z
Learnt from: Lingrui98
Repo: OpenXiangShan/GEM5 PR: 649
File: configs/common/xiangshan.py:0-0
Timestamp: 2025-12-16T08:28:46.184Z
Learning: In XiangShan trace mode (configs/common/xiangshan.py), there is no bootloader. Update the config by setting test_sys.workload.bootloader = '' (empty string) to indicate no bootloader, instead of providing a bootloader path. This ensures the test setup correctly reflects the absence of a bootloader for XiangShan trace mode.

Applied to files:

  • configs/common/xiangshan.py
⏰ 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 (7)
configs/common/xiangshan.py (1)

171-174: LGTM - Consistent revert to 2-channel DDR configuration.

The default DRAMSim3 ini path is correctly reverted to the 2-channel configuration when no explicit ini file is provided.

.github/workflows/autotest/gem5.cfg (1)

188-189: LGTM - CI configuration updated for 2-channel DDR.

The DRAMSim3 ini path in the autotest workflow is correctly reverted to the 2-channel configuration.

configs/example/se.py (1)

162-165: LGTM - Default SE mode configuration reverted to 2-channel DDR.

The default DRAMSim3 ini path in the SE (Syscall Emulation) mode configuration is correctly reverted.

util/pgo/basic_pgo.sh (1)

65-66: LGTM - PGO script updated for 2-channel DDR.

The DRAMSim3 ini path in the PGO profiling script is correctly reverted to the 2-channel configuration.

.github/workflows/autotest/gem5-vec.cfg (1)

45-47: LGTM - Vector test CI configuration updated for 2-channel DDR.

The DRAMSim3 ini path in the vector test autotest workflow is correctly reverted to the 2-channel configuration.

util/memory_check/run-xs-with-valgrind.sh (1)

25-25: LGTM - Consistent with the revert across the codebase.

The DRAMSim3 ini path change is consistent with the other script updates in this PR, correctly reverting to the 2-channel configuration.

util/runse.sh (1)

25-25: LGTM - DRAMSim3 configuration reverted to 2-channel.

The change correctly switches the DDR configuration from 8-channel back to 2-channel, and the target ini file exists at the referenced path.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

🚀 Coremark Smoke Test Results

Branch IPC Change
Base (xs-dev) 2.1664 -
This PR 2.1691 📈 +0.0027 (+0.13%)

✅ Difftest smoke test passed!

@Ergou-ren Ergou-ren merged commit e02272c into xs-dev Jan 15, 2026
2 checks passed
@Ergou-ren Ergou-ren deleted the revert_ddr branch January 15, 2026 10:01
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