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

Add Test suites for Zcb sub extension from Code Size Reduction Extension (Zce) #364

Merged
merged 7 commits into from
Dec 4, 2023

Conversation

Abdulwadoodd
Copy link
Contributor

@Abdulwadoodd Abdulwadoodd commented Jun 20, 2023

Description

Code Size Reduction is the superset of the standard Compressed (C) extension. It consists of six sub-extensions:

  • Zca
  • Zcf
  • Zcd
  • Zcb
  • Zcmp
  • Zcmt

Out of these six sub-extensions zca, zcf, zcd contains the instructions which are part of the standard C extension. But zcb, zcmp, zcmt sub extensions add new instructions.

💡 Currently, this repository has test suites only for zca instructions.

Changes

  • This PR adds the test suites for zcb instructions, which is one of six sub-extensions of RISC-V Code Size Reduction.
  • Added a test macro that uses a single operand.

Related Pull Requests

  • RISC-V CTG: PR #64 adds the test template and cover group format (cgf) to generate the tests suites for zcb.
  • RISC-V Config: PR #128 adds the ISA strings for Code Size Reduction extension, which is used by RISCOF.

Related Issues

Issue #334 and #6

Ratified/Unratified Extensions

  • Ratified

Reference Model Used

  • Spike

⚠️ Support of Code Size Reduction Extension (Zcb, Zcmp, Zcmt) is not yet added to SAIL.

Mandatory Checklist:

  • All tests are compliant with the test-format spec present in this repo
  • Ran the new tests on RISCOF with Spike as a reference model successfully?
  • Ran the new tests on RISCOF in coverage mode
  • Link to Google-Drive folder containing the new coverage reports.
  • Link to PR in RISCV-ISAC from which the reports were generated.
  • Changelog entry created with a minor patch

@allenjbaum
Copy link
Collaborator

This needs the following checklist items completed:

  • Ran the new tests on RISCOF in coverage mode
  • Link to Google-Drive folder containing the new coverage reports.
    You'll probably get a conflict on the changlog because there are a bunch of inflight mreges, so be warned.

@davidharrishmc
Copy link
Contributor

I would love to use these tests on CORE-V Wally. It would be easiest when they are integrated into riscv-arch-test. @Abdulwadoodd, do you have an anticipated schedule about completing the checklist items to finish the PR?

@Abdulwadoodd
Copy link
Contributor Author

Hello prof. @davidharrishmc , generating coverage reports is a bit challenging for me due to the apparent lack of zcb support in ISAC's internal decoder. There were some alternatives highlighted in one of the sig-arch-test meetings. I will look to look into this and try to make it work by the end of the upcoming week.

@YazanHussnain-10x
Copy link

YazanHussnain-10x commented Nov 8, 2023

Coverage reports for the zcb instruction test suites (32/64) are available here

@Abdulwadoodd
Copy link
Contributor Author

Abdulwadoodd commented Nov 8, 2023

@allenjbaum ,
With the available coverage reports and the corresponding PR in RISCV ISAC by @YazanHussnain-10x, mandatory checklist has been completed and this PR is good to go (unless there is any change required).

@Abdulwadoodd
Copy link
Contributor Author

hello @allenjbaum, pinging for a review on this PR

Signed-off-by: Abdul Wadood <abdulwadood.afzal88@gmail.com>
Signed-off-by: Abdul Wadood <abdulwadood.afzal88@gmail.com>
Signed-off-by: Abdul Wadood <abdulwadood.afzal88@gmail.com>
Signed-off-by: Abdul Wadood <abdulwadood.afzal88@gmail.com>
Signed-off-by: Abdul Wadood <abdulwadood.afzal88@gmail.com>
Signed-off-by: Abdul Wadood <abdulwadood.afzal88@gmail.com>
modified version to avoid stale version number

Signed-off-by: Allen Baum <31423142+allenjbaum@users.noreply.github.com>
Copy link
Collaborator

@allenjbaum allenjbaum left a comment

Choose a reason for hiding this comment

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

manually updated version number to avoid duplicate

@allenjbaum allenjbaum merged commit e00d217 into riscv-non-isa:main Dec 4, 2023
1 check failed
@Abdulwadoodd
Copy link
Contributor Author

Thanks, @allenjbaum for approving the PR and pulling it in.
In addition to "manually updating the version number", it would have also required a "git rebase" to avoid the conflict and make a stable version release through CI. CHANGELOG.md isn't chronologically correct at the moment.

@allenjbaum
Copy link
Collaborator

allenjbaum commented Dec 4, 2023 via email

@leo-thomas-8390
Copy link

@Abdulwadoodd @allenjbaum @YazanHussnain-10x hello ,

I am trying to understand zcmp extension ( specifically cm.push and cm.pop instruction ) it would very helpful if you can share the config files and arch test files for the same
thank you

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.

5 participants