Add support for the Zicond ISA extension #321

cmuellner · 2023-03-21T19:59:15Z

This patch introduces a coverage model and the generated test cases using riscv_ctg.

The tests have been generated using the following command:

  riscv_ctg.py \
    --cgf coverage/dataset.cgf \
    --cgf coverage/zicond.cgf \
    -bi rv32i \
    -d tests/
  riscv_ctg.py \
    --cgf coverage/dataset.cgf \
    --cgf coverage/zicond.cgf \
    -bi rv64i \
    -d tests64/

The resulting tests have been copied to the target directory:

  cp tests/*S riscv-test-suite/rv32i_m/Zicond/src/
  cp tests64/*S riscv-test-suite/rv64i_m/Zicond/src/

This PR depends on Zicond support in the riscv-ctg repo:
riscv-software-src/riscv-ctg#59

Open topics:

I have not filled out the form below because I could not find information on how to create the requested information (e.g. the coverage reports; running the ACT against SAIL, Spike or other emulators; are generated tests complying with the TestFormatSpec.adoc?)
I have ignored the generated arch_test.h, because there is zero documentation on what to do with that, and it differs fundamentally from ./riscv-test-suite/env/arch_test.h
I have ignored the generated encoding.h because there is zero documentation on what to do with that, and it differs strangely from ./riscv-test-suite/env/encoding.h (which has three more CSRs...)

Guidance on these open topics would be very welcome.

Description

Provide a detailed description of the changes performed by the PR.

Related Issues

Please list all the issues related to this PR. Use NA if no issues exist

Ratified/Unratified Extensions

Ratified
Unratified

List Extensions

List the extensions that your PR affects. In case of unratified extensions, please provide a link to the spec draft that was referred to make this PR.

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.

charmitro · 2023-04-06T21:03:17Z

Link for test results using RISCOF can be found here.

UmerShahidengr · 2023-08-18T09:56:38Z

Zicond extension has no support in either Sail or gcc compiler. Can you give any pointer on which compiler toolchain have you used to build these tests and how have you run these tests on riscof?

cmuellner · 2023-08-21T07:47:28Z

Zicond extension has no support in either Sail or gcc compiler. Can you give any pointer on which compiler toolchain have you used to build these tests and how have you run these tests on riscof?

Zicond support for Binutils is upstream for quite some time: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b625eff8a2346fe1107aa4ab7bbf4302f2c2136e

GCC support is not needed for this repository.

The changes for CTG are linked in the description of this PR (riscv-software-src/riscv-ctg#59).

Zicond support for Sail has been merged a few months ago: riscv/sail-riscv#206

The exact commands to generate the tests can be found in the description of the PR.

charmitro · 2023-08-21T08:34:02Z

Zicond extension has no support in either Sail or gcc compiler. Can you give any pointer on which compiler toolchain have you used to build these tests and how have you run these tests on riscof?

To execute the tests, ensure you have the appropriate versions of Binutils, Spike, and SAIL installed. Once verified, you can initiate the tests using:

$ riscof testlist --config config.ini --suite riscv-arch-test/riscv-test-suite/rv64i_m/Zicond/ --env=riscv-arch-test/riscv-test-suite/env
[ ... ]
$ riscof run --config config.ini --suite riscv-arch-test/riscv-test-suite/rv64i_m/Zicond/ --env=riscv-arch-test/riscv-test-suite/env --no-browser
    INFO | TEST NAME                                          : COMMIT ID                                : STATUS
    INFO | /home/cmitrodimas/repositories/ACTs/riscv-arch-test/riscv-test-suite/rv64i_m/Zicond/src/czero.eqz-01.S : -                                        : Passed
    INFO | /home/cmitrodimas/repositories/ACTs/riscv-arch-test/riscv-test-suite/rv64i_m/Zicond/src/czero.nez-01.S : -                                        : Passed
    INFO | Test report generated at /home/cmitrodimas/repositories/ACTs/riscof_work/report.html.

UmerShahidengr · 2023-08-22T16:17:09Z

Thanks. I will make it work at my end.

UmerShahidengr · 2023-08-30T09:46:03Z

@charmitro I have updated binutils but upon compilation, it is giving unrecognized opcode error on czero.nez.
Can you share the binaries and/or exact versions of your riscv toolchain, binutils, and riscof please?

cmuellner · 2023-08-30T10:46:04Z

@charmitro I have updated binutils but upon compilation, it is giving unrecognized opcode error on czero.nez. Can you share the binaries and/or exact versions of your riscv toolchain, binutils, and riscof please?

What is the exact error message?
Which version of Binutils are you using?
How are you invoking Binutils (did you enable zicond?)?

UmerShahidengr · 2023-09-01T08:11:19Z

@cmuellner The exact error message is Error: unrecognized opcode `czero.eqz x31,x30,x29'
It is because I have not enabled Zicond and I am having trouble enabling it as I am not an expert in using binary utlities. It will be best if you send me the binary files of these tests.

UmerShahidengr · 2023-09-05T08:47:23Z

@charmitro , @cmuellner kind reminder. Can I get the binaries of Zicond tests?

UmerShahidengr · 2023-11-17T05:23:59Z

CHANGELOG.md

@@ -1,5 +1,8 @@
 # CHANGELOG

+## [3.6.3] - 2023-03-21


You need to change this Changelog entry as there are a lot of PRs merged since this update, and this version number is taken.

UmerShahidengr

Update the Changelog file, rest of the files are ready to go.

cmuellner · 2023-11-17T09:03:37Z

Updating the Changelog of all PRs after each new commit on master does not scale.
There is also no "update the Changelog file". Instead a rebase is required.
And rebasing after 8 (!) months usually requires re-testing.
How can I justify these efforts, when at the end of the day the PR might just be ignored for at least another 8 months?

Also, it seems you solved the issues with your toolchain. It would be great to share your solution, so that others which face the same issue can learn from your solution.

UmerShahidengr · 2023-11-17T09:14:09Z

Updating the Changelog of all PRs after each new commit on master does not scale.
There is also no "update the Changelog file". Instead a rebase is required.
And rebasing after 8 (!) months usually requires re-testing.
How can I justify these efforts, when at the end of the day the PR might just be ignored for at least another 8 months?

You are absolutely right. Some moderator/writer will be assigned soon by sig-arch-tests team who will be managing this Changelog issue in future to make it scalable. @allenjbaum and co are working on our governance policies. For this time only, in order to remove the backlog, you are requested to update the file manually as nobody else the write access to this PR. Once you resolve the conflict, this PR will be merged right away.

UmerShahidengr · 2023-11-17T09:15:05Z

Also, it seems you solved the issues with your toolchain. It would be great to share your solution, so that others which face the same issue can learn from your solution.

Yes I resolved the issue, I found the gcc patch online which compiled the tests and I executed it successfully

This patch intoduces a coverage model and the generated test cases using riscv_ctg. The tests have been generated using the following command: riscv_ctg.py \ --cgf coverage/dataset.cgf \ --cgf coverage/zicond.cgf \ -bi rv32i \ -d tests/ riscv_ctg.py \ --cgf coverage/dataset.cgf \ --cgf coverage/zicond.cgf \ -bi rv64i \ -d tests64/ The resulting tests have been copied to the target directory: cp tests/*S riscv-test-suite/rv32i_m/Zicond/src/ cp tests64/*S riscv-test-suite/rv64i_m/Zicond/src/ This PR depends on Zicond support in the riscv-ctg repo: riscv-software-src/riscv-ctg#59 Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>

cmuellner · 2023-11-17T09:29:10Z

PR has been rebase and Changelog has been updated (new version and today's date).

UmerShahidengr

Good to go now.

cmuellner · 2023-11-17T09:37:05Z

Also note, that the Changelog contains a merge-artifact: version 3.6.6-3.7.1 are duplicated.

Personally, I don't like the Changelog file approach. Especially if each patchset needs to bump a release number.
A much better scaling approach would be to enforce proper commit headings. Then the changelog can be generated by git log.

Putting aside the (subjective) don't-like aspect, there is still something that can be improved to make this approach scaling by looking at the GCC project.
There they require the changelog to be part of the commit message.
From there a nightly job collects copies them into the actual changelog file.

cmuellner · 2023-11-30T17:05:38Z

Once you resolve the conflict, this PR will be merged right away.

@UmerShahidengr: We already ran into a new conflict, since other PRs were merged since you requested my update. And the request-for-update and the update were made two weeks ago.

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

allenjbaum

resolved merge conflicts in changelog & updated the date

cmuellner · 2023-12-01T08:43:03Z

Thanks, @allenjbaum, @UmerShahidengr, and @charmitro!

Perfect timing, as Zicond's ratification has just been announced.

cmuellner force-pushed the zicond branch from 27df189 to 8fd3f2b Compare March 21, 2023 20:08

cmuellner mentioned this pull request Mar 28, 2023

Add support for the Zicond ISA extension riscv-software-src/riscv-ctg#59

Merged

charmitro mentioned this pull request Apr 7, 2023

Riscof coverage command treats warnings as errors #331

Open

ZenithalHourlyRate mentioned this pull request Apr 23, 2023

Implement Zicond extension chipsalliance/rocket-chip#3329

Merged

fatimasaleem mentioned this pull request Jun 19, 2023

ZiCond Extension openhwgroup/cva6#1262

Closed

fatimasaleem mentioned this pull request Jul 6, 2023

Added ZiCond Extension Support openhwgroup/cva6#1299

Closed

UmerShahidengr reviewed Nov 17, 2023

View reviewed changes

UmerShahidengr requested changes Nov 17, 2023

View reviewed changes

cmuellner force-pushed the zicond branch from 8fd3f2b to 594d483 Compare November 17, 2023 09:26

UmerShahidengr approved these changes Nov 17, 2023

View reviewed changes

Merge branch 'main' into zicond

b8bd7a6

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

allenjbaum self-assigned this Dec 1, 2023

allenjbaum approved these changes Dec 1, 2023

View reviewed changes

allenjbaum merged commit ff42b13 into riscv-non-isa:main Dec 1, 2023
1 check passed

stnolting mentioned this pull request Dec 2, 2023

Add Zicond tests stnolting/neorv32-riscof#187

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for the Zicond ISA extension #321

Add support for the Zicond ISA extension #321

cmuellner commented Mar 21, 2023 •

edited

Loading

charmitro commented Apr 6, 2023

UmerShahidengr commented Aug 18, 2023

cmuellner commented Aug 21, 2023

charmitro commented Aug 21, 2023 •

edited

Loading

UmerShahidengr commented Aug 22, 2023

UmerShahidengr commented Aug 30, 2023

cmuellner commented Aug 30, 2023 •

edited

Loading

UmerShahidengr commented Sep 1, 2023 •

edited

Loading

UmerShahidengr commented Sep 5, 2023

UmerShahidengr Nov 17, 2023

UmerShahidengr left a comment

cmuellner commented Nov 17, 2023

UmerShahidengr commented Nov 17, 2023

UmerShahidengr commented Nov 17, 2023

cmuellner commented Nov 17, 2023

UmerShahidengr left a comment

cmuellner commented Nov 17, 2023

cmuellner commented Nov 30, 2023

allenjbaum left a comment

cmuellner commented Dec 1, 2023

Add support for the Zicond ISA extension #321

Add support for the Zicond ISA extension #321

Conversation

cmuellner commented Mar 21, 2023 • edited Loading

Description

Related Issues

Ratified/Unratified Extensions

List Extensions

Reference Model Used

Mandatory Checklist:

Optional Checklist:

charmitro commented Apr 6, 2023

UmerShahidengr commented Aug 18, 2023

cmuellner commented Aug 21, 2023

charmitro commented Aug 21, 2023 • edited Loading

UmerShahidengr commented Aug 22, 2023

UmerShahidengr commented Aug 30, 2023

cmuellner commented Aug 30, 2023 • edited Loading

UmerShahidengr commented Sep 1, 2023 • edited Loading

UmerShahidengr commented Sep 5, 2023

UmerShahidengr Nov 17, 2023

Choose a reason for hiding this comment

UmerShahidengr left a comment

Choose a reason for hiding this comment

cmuellner commented Nov 17, 2023

UmerShahidengr commented Nov 17, 2023

UmerShahidengr commented Nov 17, 2023

cmuellner commented Nov 17, 2023

UmerShahidengr left a comment

Choose a reason for hiding this comment

cmuellner commented Nov 17, 2023

cmuellner commented Nov 30, 2023

allenjbaum left a comment

Choose a reason for hiding this comment

cmuellner commented Dec 1, 2023

cmuellner commented Mar 21, 2023 •

edited

Loading

charmitro commented Aug 21, 2023 •

edited

Loading

cmuellner commented Aug 30, 2023 •

edited

Loading

UmerShahidengr commented Sep 1, 2023 •

edited

Loading