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.

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

m/sret optimization #1333

Merged
merged 1 commit into from
Aug 14, 2023
Merged

m/sret optimization #1333

merged 1 commit into from
Aug 14, 2023

Conversation

ezelioli
Copy link
Contributor

Problem:

With the current implementation, when an sret retires the pipeline stalls for several cycles (>30) due to i-cache misses that are never resolved (since outstanding cache misses are killed on a pipeline flush).

Proposed solution:

Treat m/sret operations as direct jumps to PC + 0 when computing npc_d. This avoids polluting the instruction cache with useless instructions, and generating spurious i-cache misses.
The only software workaround I found is to manually insert a j . instruction after each m/sret. However, this can simply be implemented as a hardware optimization to make ISR performance more reliable.
Assuming no other source of stalling (i.e. real i-cache miss, TLB miss), this brings the sret penalty the same as a pipeline flush (6 cycles).

@github-actions
Copy link
Contributor

❌ failed run, report available here.

@JeanRochCoulon
Copy link
Contributor

Hello @ezelioli Indeed this fixes a kind of performance bug. Thank for detecting it and fixing it !!
To merge could you sign the Eclipse Agreement ?

@ezelioli
Copy link
Contributor Author

Hello @JeanRochCoulon, sure I just signed it !

@JeanRochCoulon
Copy link
Contributor

Hello @ezelioli You have signed the agreement but this did not solved the github action job. I think the PR need to be resubmitted to take into account the modification. Anyway, I merge.

@JeanRochCoulon JeanRochCoulon merged commit 49d1262 into openhwgroup:master Aug 14, 2023
12 checks passed
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