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

Specify can_branch and decode the sret instruction #443

Merged
merged 1 commit into from
May 27, 2024

Conversation

ChinYikMing
Copy link
Collaborator

sret instruction is used for returning from a trap when trap occurs in S-mode level. Thus, the execution flow will not be sequential. During basic block translation, the sret instruction should be considered as can_branch instruction.

@ChinYikMing ChinYikMing requested a review from qwe661234 May 21, 2024 13:27
@jserv
Copy link
Contributor

jserv commented May 21, 2024

@henrybear327, The item CI / docker-hub-build-and-publish (pull_request) has been In progress for more than 1 hour (details). Can you check?

@henrybear327
Copy link
Collaborator

henrybear327 commented May 21, 2024

@henrybear327, The item CI / docker-hub-build-and-publish (pull_request) has been In progress for more than 1 hour (details). Can you check?

@jserv according to Github Status, there is an issue with We are investigating delayed updates to Actions job statuses., as shown below (screenshot is used in case the update of the status page changed the text we are currently discussing)

Screenshot 2024-05-21 at 4 43 51 PM

So I would advise that we just wait for the incident to be resolved.

@henrybear327
Copy link
Collaborator

henrybear327 commented May 21, 2024

@henrybear327, The item CI / docker-hub-build-and-publish (pull_request) has been In progress for more than 1 hour (details). Can you check?

Incident is ongoing still, but the job has been completed!

@jserv jserv changed the title Tweak can_branch of sret instruction Specify can_branch for sret instruction May 22, 2024
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Benchmarks

Benchmark suite Current: 6f8864b Previous: 037f5ad Ratio
Dhrystone 4.44 Average DMIPS over 10 runs 4.57 Average DMIPS over 10 runs 1.03
Coremark 0.004 Average iterations/sec over 10 runs 0.004 Average iterations/sec over 10 runs 1

This comment was automatically generated by workflow using github-action-benchmark.

@ChinYikMing
Copy link
Collaborator Author

The sret instruction involves privilege mode change, so the implementation of sret instruction will be tested along in #438 and skip for this PR.

@jserv
Copy link
Contributor

jserv commented May 26, 2024

The sret instruction involves privilege mode change, so the implementation of sret instruction will be tested along in #438 and skip for this PR.

Refine the subject of both pull request and git commit messages. You are indeed implementing sret instruction in a minimal way.

@ChinYikMing ChinYikMing changed the title Specify can_branch for sret instruction Specify can_branch and decode the sret instruction May 26, 2024
src/decode.c Show resolved Hide resolved
sret instruction is used for returning from a trap when trap occurs in
S-mode level. Thus, the execution flow will not be sequential. During
basic block translation, the sret instruction should be considered as
can_branch instruction.

Moreover, the existing system instruction decoder does not support
decoding the sret instruction. Thus, the ir->opcode should be set
correctly to support decoding the sret instruction. The implementation
of sret instruction is simply returning false for now, the improved
implementation will be completed and tested in sysprog21#438 since the sret
instruction involves privilege mode changing.
@jserv jserv merged commit a968310 into sysprog21:master May 27, 2024
8 checks passed
@jserv
Copy link
Contributor

jserv commented May 27, 2024

Thank @ChinYikMing for contributing!

@ChinYikMing ChinYikMing deleted the sret-can-branch branch May 27, 2024 04:44
ChinYikMing added a commit to ChinYikMing/rv32emu that referenced this pull request Jun 5, 2024
Since sret is made to branchable in sysprog21#443, the relevant checking function
which determine if the instruction is branchable also should be updated.
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