Skip to content

configs: Initialize classic L2 DRRIP slice params#818

Merged
jensen-yan merged 1 commit intoxs-devfrom
fix-classic-l2-drrip-config
Apr 9, 2026
Merged

configs: Initialize classic L2 DRRIP slice params#818
jensen-yan merged 1 commit intoxs-devfrom
fix-classic-l2-drrip-config

Conversation

@jensen-yan
Copy link
Copy Markdown
Collaborator

@jensen-yan jensen-yan commented Apr 9, 2026

Summary

Initialize DRRIP slice-related parameters for the classic-L2 configuration.

Problem

The default L2 cache uses DRRIPRP, which inherits from DuelingRP. In the classic-L2 configuration, the replacement policy did not initialize num_slices and num_sets_per_slice.

As a result, the simulator could hit floorLog2(0) during initialization and abort at tick 0 when running with --classic-l2.

Fix

When classic L2 is configured, initialize the DRRIP replacement policy with:

  • num_slices = 1
  • num_sets_per_slice = size / (64 * assoc)

This keeps the classic-L2 configuration consistent with the assumptions made by the replacement-policy implementation.

Validation

Validated with SE matrix tests using --classic-l2:

  • precomp_rand_repro: passes
  • gemm_precomp: all 8 precomputed cases pass

Change-Id: I4973ed1585461ac134fc26ca254bf08ea636405f
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 9, 2026

📝 Walkthrough

Walkthrough

The L2 cache configuration now conditionally customizes replacement policy parameters when using DRRIPRP instances, setting num_slices to 1 and deriving num_sets_per_slice from the cache's size and associativity using a fixed divisor.

Changes

Cohort / File(s) Summary
Cache Configuration Customization
configs/common/CacheConfig.py
Adds conditional logic to customize DRRIPRP replacement policy parameters in L2 cache config, setting num_slices to 1 and computing num_sets_per_slice from cache size and associativity.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A whisker-twitch of cache delight,
With slices set just right, so tight,
DRRIPRP now knows its place,
In configs where the L2 race,
Hops faster through the memory space!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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
Title check ✅ Passed The title accurately summarizes the main change: it specifies that DRRIP slice parameters are being initialized in the classic L2 cache configuration.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-classic-l2-drrip-config

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.

@jensen-yan jensen-yan requested a review from happy-lx April 9, 2026 03:53
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 64ea8a148c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@configs/common/CacheConfig.py`:
- Around line 95-98: DRRIP set geometry currently hardcodes a 64B line size;
update the computation in the DRRIPRP block
(system.l2_caches[i].replacement_policy) so num_sets_per_slice =
system.l2_caches[i].size // (system.cache_line_size * system.l2_caches[i].assoc)
(use system.cache_line_size rather than 64), and defensively fall back to 64 if
system.cache_line_size is missing/None; keep other assignments to num_slices and
num_sets_per_slice unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a35af615-1370-45c4-a3c1-81c96d55e949

📥 Commits

Reviewing files that changed from the base of the PR and between 86825b3 and 64ea8a1.

📒 Files selected for processing (1)
  • configs/common/CacheConfig.py

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

🚀 Coremark Smoke Test Results

Branch IPC Change
Base (xs-dev) 2.2665 -
This PR 2.2665 ➡️ 0.0000 (0.00%)

✅ Difftest smoke test passed!

@jensen-yan jensen-yan merged commit 7a9783c into xs-dev Apr 9, 2026
2 checks passed
@jensen-yan jensen-yan deleted the fix-classic-l2-drrip-config branch April 9, 2026 04:56
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