Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RAS: handle a case where a jalr instruction requires a pop followed by a push #3277

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

RAS: handle a case where a jalr instruction requires a pop followed by a push #3277

wants to merge 3 commits into from

Conversation

my-mayfly
Copy link
Collaborator

When two different link registers (x1 and x5) are given as rs1 and rd, then the RAS is both popped and pushed to support coroutines.

@XiangShanRobot
Copy link

[Generated by IPC robot]
commit: 3f3ecf8

commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
3f3ecf8 1.760 0.458 1.991 1.154 2.930 2.497 2.292 0.928 1.377 1.367 3.167 2.507 2.389 2.847

master branch:

commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
af9e7cd 0.458 1.991 2.292 1.377 2.507
6ebd27e 1.760 0.458 1.991 1.154 2.930 2.497 2.292 0.928 1.377 1.367 3.167 2.507 2.389 2.847
b03c55a 1.768 0.458 1.991 1.154 2.930 2.497 2.292 0.928 1.377 1.367 3.167 2.507 2.389 2.847
5003e6f 0.458 1.991 1.154 2.292 0.928 1.377 1.367 3.167 2.507 2.847
96d2b58 0.458 1.991 1.154 2.292 0.928 1.377 1.367 3.167 2.507 2.852
5f70522 1.760 0.458 1.991 1.154 2.930 2.497 2.292 0.928 1.377 1.367 3.167 2.507 2.389 2.852
5cac1ae 1.768 0.458 1.991 1.154 2.930 2.497 2.293 0.928 1.377 1.367 3.167 2.507 2.389 2.852
a9b27cd 1.768 0.458 1.991 1.154 2.930 2.497 2.293 0.928 1.377 1.367 3.167 2.507 2.389 2.852
d73f370 1.768 0.458 1.991 1.154 2.930 2.497 2.293 0.928 1.377 1.367 3.167 2.507 2.389 2.852
e3da8ba 1.760 0.458 1.991 1.154 2.930 2.497 2.293 0.928 1.377 1.367 3.167 2.507 2.389 2.852

@eastonman
Copy link
Member

Mostly LGTM.
However, this seems to increase the width of the FTB SRAM to something like 81, which is odd. Should we encode call/ret info from one-hot to something storage efficient?

@my-mayfly
Copy link
Collaborator Author

You are right, increasing the width of sram is not a good way. I will try to adjust the code according to your suggestion.

@XiangShanRobot
Copy link

[Generated by IPC robot]
commit: a1599fa

commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
a1599fa 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.371 1.367 3.167 2.421 2.387 2.846

master branch:

commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
8855a44 0.458 1.985 1.161 2.296 0.928 1.380 3.167 2.421 2.846
1f90301 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.370 1.367 3.167 2.421 2.387 2.846
56db494 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.375 1.367 3.167 2.421 2.387 2.846
54c6d89 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.381 1.367 3.167 2.421 2.387 2.846
af9e7cd 1.768 0.458 1.991 1.154 2.930 2.497 2.292 0.928 1.377 1.367 3.167 2.507 2.389 2.847
6ebd27e 1.760 0.458 1.991 1.154 2.930 2.497 2.292 0.928 1.377 1.367 3.167 2.507 2.389 2.847
b03c55a 1.768 0.458 1.991 1.154 2.930 2.497 2.292 0.928 1.377 1.367 3.167 2.507 2.389 2.847
5003e6f 1.760 0.458 1.991 1.154 2.930 2.497 2.292 0.928 1.377 1.367 3.167 2.507 2.389 2.847
96d2b58 1.760 0.458 1.991 1.154 2.930 2.497 2.292 0.928 1.377 1.367 3.167 2.507 2.389 2.852
5f70522 1.760 0.458 1.991 1.154 2.930 2.497 2.292 0.928 1.377 1.367 3.167 2.507 2.389 2.852

ngc7331
ngc7331 reviewed Jul 25, 2024
View reviewed changes
Copy link
Member

@ngc7331 ngc7331 left a comment

Choose a reason for hiding this comment

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

I'm not familiar to the RAS part, overall it LGTM, just a few questions.

src/main/scala/xiangshan/frontend/NewFtq.scala Outdated Show resolved Hide resolved
src/main/scala/xiangshan/frontend/PreDecode.scala Outdated Show resolved Hide resolved
@XiangShanRobot
Copy link

[Generated by IPC robot]
commit: b7adbee

commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
b7adbee 1.732 0.458 1.979 1.158 2.930 2.497 2.286 0.928 1.371 1.367 3.167 2.265 2.350 2.856

master branch:

commit astar copy_and_run coremark gcc gromacs lbm linux mcf microbench milc namd povray wrf xalancbmk
75487d0 1.758 0.458 1.981 1.161 2.930 2.497 2.296 0.928 1.360 1.367 3.167 2.426 2.387 2.846
dcadd37 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.374 1.367 3.167 2.421 2.387 2.846
06f0a37 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.380 1.367 3.167 2.421 2.387 2.846
a7eed6c 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.381 1.367 3.167 2.421 2.387 2.846
7263b59 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.374 1.367 3.167 2.421 2.387 2.846
3e8a017 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.374 1.367 3.167 2.421 2.387 2.846
a941bfc 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.374 1.367 3.167 2.421 2.387 2.846
8855a44 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.380 1.367 3.167 2.421 2.387 2.846
1f90301 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.370 1.367 3.167 2.421 2.387 2.846
56db494 1.752 0.458 1.985 1.161 2.930 2.497 2.296 0.928 1.375 1.367 3.167 2.421 2.387 2.846

eastonman
eastonman approved these changes Aug 17, 2024
View reviewed changes
Copy link
Member

@eastonman eastonman left a comment

Choose a reason for hiding this comment

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

LGTM, RAS code is on your own though.

@eastonman eastonman added the do not merge Do not merge this pull request label Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ngc7331 ngc7331 ngc7331 left review comments

@eastonman eastonman eastonman approved these changes

@Gao-Zeyu Gao-Zeyu Awaiting requested review from Gao-Zeyu Gao-Zeyu is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
do not merge Do not merge this pull request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@my-mayfly @XiangShanRobot @eastonman @ngc7331