arch-riscv: configurable vector splitting unit#949
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughRefactors O3 vector memory split handling to use per-unit split state and scheduling, adds matching RVV documentation and configuration, updates several predictor files to use lambda-based deferred cleanup, and conditionally skips Python program-name setup on Python 3.11+. ChangesVector Split Unit Refactor
Deferred Cleanup Lambda Updates
Python 3.11 Program Name Guard
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the O3 backend’s vector-memory “split” delay modeling in IssueQue to support multiple independent split units, aiming to avoid global blocking when one split stream is occupied by a blocking vector mem op.
Changes:
- Replace the single global vector split queue/blocker with per-unit split state (
vectorSplitStates) and round-robin unit selection. - Update vector-ready event scheduling logic to account for multiple split units and their next release times.
- Add documentation describing the multi-unit split model and intended configuration surface.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/cpu/o3/issue_queue.hh | Introduces per-split-unit state and new helper APIs for unit selection/scheduling. |
| src/cpu/o3/issue_queue.cc | Implements multi-unit split behavior, per-unit blocking semantics, and updated event scheduling. |
| docs/Gem5_Docs/RVV/vector.md | Documents the new split-unit model and intended configurability via vectorSplitUnits. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jueshiwenli
force-pushed
the
calibrateVStride-split-0708
branch
from
3b3bb6c to
1d89eb8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cpu/o3/issue_queue.hh (1)
221-224: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse lower_snake_case for the new helper APIs.
Please rename the new methods and their definitions/call sites, e.g.
has_available_vector_split_unit,select_vector_split_unit,next_vector_split_release_tick, anderase_vector_split_blocker. As per coding guidelines, "Functions and methods should use lower_snake_case naming convention".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cpu/o3/issue_queue.hh` around lines 221 - 224, The new helper APIs in IssueQueue use non-conforming camelCase names, so rename them to lower_snake_case and update every definition and call site accordingly. Specifically, change the method declarations and usages of hasAvailableVectorSplitUnit, selectVectorSplitUnit, nextVectorSplitReleaseTick, and eraseVectorSplitBlocker to their lower_snake_case equivalents in IssueQueue and any related implementation code.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/cpu/o3/issue_queue.hh`:
- Around line 221-224: The new helper APIs in IssueQueue use non-conforming
camelCase names, so rename them to lower_snake_case and update every definition
and call site accordingly. Specifically, change the method declarations and
usages of hasAvailableVectorSplitUnit, selectVectorSplitUnit,
nextVectorSplitReleaseTick, and eraseVectorSplitBlocker to their
lower_snake_case equivalents in IssueQueue and any related implementation code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8750eb92-2d36-4bbc-94a8-25f1a01e2845
📒 Files selected for processing (3)
docs/Gem5_Docs/RVV/vector.mdsrc/cpu/o3/issue_queue.ccsrc/cpu/o3/issue_queue.hh
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
|
[Generated by GEM5 Performance Robot] Standard PerformanceOverall Score
|
Change-Id: I5ba2c40f363863dcce0a29bb4911a02f372d3bb9
Implement multiple configurable vector split units, with no blocking between different split units. Change-Id: Id1f7918060a321dff33cac3c02f2daef9c75cb23
jueshiwenli
force-pushed
the
calibrateVStride-split-0708
branch
from
1b8e3ec to
f09e596
Compare
🚀 Coremark Smoke Test Results
✅ Difftest smoke test passed! |
jueshiwenli
requested review from
Anzooooo,
BirdsTweetWindWhisper,
Lemover,
XDu5,
huxuan0307 and
weidingliu
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement multiple configurable vector split units, with no blocking between different split units.
Change-Id: I7d2487187f3d45f76ebc7eec44c845c8aa87d666
Summary by CodeRabbit