Update tests for B extension

[ved-rivos]

Description

Add B extension to ISA strings
B == Zba + Zbb + Zbs

Related Issues

N/A

Ratified/Unratified Extensions

• Ratified
• [] Unratified

List Extensions

https://github.com/riscv/riscv-b

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 : Add unratified B extension riscv-software-src/riscv-ctg#100
• 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.

[davidharrishmc]

For reasons Allen and I don't entirely understand, some riscof installations require XLEN in the RVTEST_CASE macro to select the correct set of tests to build. Some tests such as the I suite already have this, but I've been having to add the XLEN check to many of the newer test suites. Would it be possible to modify your script to generate tests that include the XLEN check?

For example, in rv32i_m/B/src/andn-01.S

please change

RVTEST_CASE(0,"//check ISA:=regex(.*I.B.);def TEST_CASE_1=True;",andn)
to
RVTEST_CASE(0,"//check ISA:=regex(.32.);check ISA:=regex(.*I.B.);def TEST_CASE_1=True;",andn)

[allenjbaum]

The RVTEST_CASE macro is part of the template that is generated by the CTG (compatibility Test Generator) program (when tests aren't hand generated, as many priv and non-ISA oriented tests seem to be moving to) So, this change sounds like it could be part of either RISCOF, but more likely part of CTG, not part of arch-test; I would file this issue in that repo. I'm having the template part of the CTG code updated to be much much simpler (and moving the complexity to existing arch-test macros) and all the tests re-generated, and this could be part of it.

…

On Mon, Jan 29, 2024 at 5:05 AM David Harris ***@***.***> wrote: For reasons Allen and I don't entirely understand, some riscof installations require XLEN in the RVTEST_CASE macro to select the correct set of tests to build. Some tests such as the I suite already have this, but I've been having to add the XLEN check to many of the newer test suites. Would it be possible to modify your script to generate tests that include the XLEN check? For example, in rv32i_m/B/src/andn-01.S please change RVTEST_CASE(0,"//check ISA:=regex(.*I.*B.*);def TEST_CASE_1=True;",andn) to RVTEST_CASE(0,"//**check ISA:=regex(.*32.*);**check ISA:=regex(.*I.*B.*);def TEST_CASE_1=True;",andn) — Reply to this email directly, view it on GitHub <#427 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJRCBOL3M7TWK5KBKZTYQ6NBHAVCNFSM6AAAAABB2KKHFCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJUGY2TONRXGU> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[ved-rivos]

This gets generated from the string in the cgf - "check ISA:=regex(.*I.B.)". Fixing this will need the cgf file to be updated to add the xlen filter and then regenerate the files. My PR to add B just got merged into dev branch of CTG so I cannot update that PR unfortunately. A new PR would be needed.

require XLEN in the RVTEST_CASE macro to select the correct set of tests to build

I have noticed it will generate a warning of by selecting more tests into the test data base but nothing bad besides that. I am not sure why we cannot just fix riscof to look at both the test ISA and the ISA string for doing the test selection.

But seems like Allen has a better fix in mind.

[davidharrishmc]

Thank you. Let's defer to Allen's recommendation.

[allenjbaum]

1. I guess I don't understand why earlier tests were generated "correctly," and these new ones weren't.
   That's the first step - I don't want this to happen again.

2. David: is this actually an issue? The report says he tested it with Spike vs. Sail, and it passed.
   IS this necessary (in this case) or just nice to have?
   Presumably the tests won't be run if the RVTEST_ISA string doesn't match

3. As far as I can tell, the changes to this test were only in those boilerplate macros; the tests themselves haven't changed.
   It's really hard to review this when all the diffs have a "Large diffs are not rendered by default" message.
   The shouldn't be for a change like this.
   The right way to do this (IMHO, of course) is to use sed to just replace/add the change comments and RVTEST_ISA/RVTEST_CASE macros.

I think we need a "best practices" document...
If this is actually a functional problem, then let's fix it by doing what I suggest above, either by taking the current tests in the repo and running sed of them, replacing this PR, or by a new commit using sed on this PR.