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

Add unratified Smctr/Ssctr #471

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Add unratified Smctr/Ssctr #471

wants to merge 1 commit into from

Conversation

ved-rivos
Copy link
Contributor

Description

Add tests for Smctr/Ssctr extension.

Related Issues

NA

Ratified/Unratified Extensions

  • Ratified
  • Unratified

List Extensions

Smctr/Ssctr: : https://github.com/riscv/riscv-control-transfer-records

Reference Model Used

  • SAIL
  • Spike
  • Other - < SPECIFY HERE >

Mandatory Checklist:

  • All tests are compliant with the test-format spec present in this repo ?
  • Ran the new tests on RISCOF with SAIL/Spike as reference model successfully ?
  • Ran the new tests on RISCOF in coverage mode
  • Link to Google-Drive folder containing the new coverage reports (See this for more info): < SPECIFY HERE >
  • Link to PR in RISCV-ISAC from which the reports were generated : < SPECIFY HERE >
  • Changelog entry created with a minor patch

Optional Checklist:

  • RISCV-V CTG PR link if tests were generated using it : < SPECIFY HERE >
  • Were the tests hand-written/modified ?
  • Have you run these on any hard DUT model ? Please specify name and provide link if possible in the description
  • If you have modified arch_test.h Please provide a detailed description of the changes in the Description section above.

@ved-rivos ved-rivos mentioned this pull request Jun 10, 2024
Open
bcstrongx
bcstrongx approved these changes Jun 10, 2024
View reviewed changes
riscv-test-suite/rv64i_m/Smctr/src/smctr_m.S Outdated
addi x1, x1, 1
csrw CSR_SCTRDEPTH, x1
csrr x14, CSR_SCTRDEPTH
beq x1, x14, try_next_depth
Copy link

Choose a reason for hiding this comment

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

This will exit the loop for the first unsupported depth value, but there is no requirement that supported values are consecutive. So an implementation could support 16 and 256, for instance. Should it iterate over all depth values?

riscv-test-suite/rv64i_m/Smctr/src/smctr_m.S
csrw CSR_SCTRDEPTH, x1
csrr x14, CSR_SCTRDEPTH
beq x1, x14, try_next_depth
csrr x14, CSR_SCTRDEPTH
Copy link

Choose a reason for hiding this comment

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

nit: this read looks redundant (with line 43)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

riscv-test-suite/rv64i_m/Smctr/src/smctr_s.S

RVTEST_GOTO_LOWER_MODE Smode

# disable CTR at M mode
Copy link

Choose a reason for hiding this comment

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

nit: probably should say "disable CTR at S mode"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its enabling right. Updated to say "at S mode"

@ved-rivos ved-rivos force-pushed the smctr1 branch 3 times, most recently from d26d1ac to 8e30ec7 Compare June 11, 2024 17:37
@jamesbeyond jamesbeyond added unratified size-S Small efforts required privileged labels Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bcstrongx bcstrongx bcstrongx approved these changes

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

Assignees
No one assigned
Labels
privileged size-S Small efforts required unratified
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ved-rivos @bcstrongx @jamesbeyond