Skip to content

issue_stage: add register renaming#2805

Closed
ricted98 wants to merge 7 commits intoopenhwgroup:masterfrom
ricted98:rt/rnm
Closed

issue_stage: add register renaming#2805
ricted98 wants to merge 7 commits intoopenhwgroup:masterfrom
ricted98:rt/rnm

Conversation

@ricted98
Copy link
Copy Markdown
Contributor

@ricted98 ricted98 commented Mar 3, 2025

This PR introduces a register renaming scheme to eliminate WAW hazards, as depicted below. Note that this change is not toggled by a parameter, rather it is a redesign of a portion of the issue logic.

immagine

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 3, 2025

❌ failed run, report available here.

@cathales
Copy link
Copy Markdown
Contributor

cathales commented Mar 5, 2025

Hello Riccardo,

Thank you for your pull request!

To be able to run the subsequent CI jobs, including synthesis and other benchmarks, the expected performance needs to be updated here in your PR:

"dhrystone_cv32a65x": 31976,
"dhrystone_cv32a60x": 39449,

Your new values are 37474 for cv32a60x and 30056 for cv32a60x (you can find them here: https://riscv-ci.pages.thales-invia.fr/dashboard/dashboard_cva6_2805.html)

@ricted98
Copy link
Copy Markdown
Contributor Author

ricted98 commented Mar 5, 2025

Thanks, done!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 5, 2025

❌ failed run, report available here.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 5, 2025

❌ failed run, report available here.

@ricted98
Copy link
Copy Markdown
Contributor Author

ricted98 commented Mar 5, 2025

@cathales is it possible to see the Spyglass detailed report to fix the CI failure?

@cathales
Copy link
Copy Markdown
Contributor

cathales commented Mar 5, 2025

There are 6 new warnings "[W362] Unequal length in arithmetic comparison operator" in issue_read_operands.sv, but the lines are not in the report.

@ricted98
Copy link
Copy Markdown
Contributor Author

ricted98 commented Mar 5, 2025

I see. So there is no way of knowing what triggers these warnings? Additionally, should I update the remaining performance reference numbers of the CI as well?

Comment thread core/issue_read_operands.sv Outdated
assign rs1_fwd_req[i][k] = (fwd_i.sbe[fwd_i.wb[k].trans_id].rd == issue_instr_i[i].rs1) & (fwd_i.still_issued[fwd_i.wb[k].trans_id]) & fwd_i.wb[k].valid & (~fwd_i.wb[k].ex_valid) & ((CVA6Cfg.FpPresent && ariane_pkg::is_rd_fpr(
fwd_i.sbe[fwd_i.wb[k].trans_id].op
)) == (CVA6Cfg.FpPresent && ariane_pkg::is_rs1_fpr(
assign rs1_fwd_req[i][k] = ((fwd_i.wb[k].trans_id == rd_clobber_gpr[issue_instr_i[i].rs1] && (!ariane_pkg::is_rs1_fpr(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Likely these comparisons? The other ones you changed/added are either with a genvar (unsized-ish) or a variant from the seemingly correct enumerated type.

And there are 6 of them.

@ricted98 ricted98 force-pushed the rt/rnm branch 2 times, most recently from 673e1bb to 804dcdb Compare March 6, 2025 09:41
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2025

❌ failed run, report available here.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2025

❌ failed run, report available here.

2 similar comments
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2025

❌ failed run, report available here.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2025

❌ failed run, report available here.

@ricted98
Copy link
Copy Markdown
Contributor Author

ricted98 commented Mar 6, 2025

@JeanRochCoulon would it be possible to obtain a detailed spyglass report to understand where the new SpyGlass warnings arise? Thanks!

@ricted98 ricted98 marked this pull request as ready for review March 6, 2025 18:10
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2025

❌ failed run, report available here.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2025

❌ failed run, report available here.

OttG and others added 2 commits March 7, 2025 10:45
Co-authored-by: Riccardo Tedeschi <riccardo.tedeschi6@unibo.it>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2025

❌ failed run, report available here.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2025

❌ failed run, report available here.

@ricted98
Copy link
Copy Markdown
Contributor Author

ricted98 commented Mar 7, 2025

The SpyGlass warnings are now fixed!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 8, 2025

❌ failed run, report available here.

@github-actions
Copy link
Copy Markdown
Contributor

❌ failed run, report available here.

@github-actions
Copy link
Copy Markdown
Contributor

❌ failed run, report available here.

@github-actions
Copy link
Copy Markdown
Contributor

❌ failed run, report available here.

@github-actions
Copy link
Copy Markdown
Contributor

👋 Hi there!

This pull request seems inactive. Need more help or have updates? Feel free to let us know. If there are no updates within the next few days, we'll go ahead and close this PR. 😊

@github-actions github-actions Bot added the Status:Stale Issue or PR is stale and hasn't received any updates. label Apr 20, 2025
@github-actions
Copy link
Copy Markdown
Contributor

❌ failed run, report available here.

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

❌ failed run, report available here.

@github-actions github-actions Bot removed the Status:Stale Issue or PR is stale and hasn't received any updates. label Apr 23, 2025
@ricted98 ricted98 closed this Apr 23, 2025
@ricted98 ricted98 deleted the rt/rnm branch June 19, 2025 10:36
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.

3 participants