Alitariq4589/issue#300 revisited

[alitariq4589]

Description

This PR is the continuation of the PR #365, where there were only tests generated for most-negative-number divided by -1 for RV64 architecture. In this PR, I have also added the generated tests for same condition for RV32 architecture.

Note: One thing to note about coverage is that, there are two conditions in dataset.cgf: One case of division of most negative number divided by -1 for RV32 and same case for RV64. dataset.cgf shares the same set of conditions in both the architectures (i.e. RV64IM as well as RV32IM), so the condition for RV64IM will not work on RV32IM and hence will not be covered in coverage and the condition for RV32IM will not work for RV64IM and will not be covered in its coverage which is totally an expected behavior, other than that, the coverage is a 100% hit.

Related Issues

#300

Ratified/Unratified Extensions

• Ratified
• Unratified

List Extensions

RV32IM

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): Folder Link
• Link to PR in RISCV-ISAC from which the reports were generated : riscv_isac version 0.18.0 (pip)
• Changelog entry created with a minor patch

Optional Checklist:

• RISCV-V CTG PR link if tests were generated using it : PR#87
• 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.

[allenjbaum]

You regenerated all the tests, so it's hard to actually figure out what changed. But, there should be a test report generated that shows all the cover points that were seen.
Could you point me at it?

[alitariq4589]

@allenjbaum You are right. There is a test report generated in html format but I did not include it in this PR since I needed to just add the tests and I did not see a test report added in the repo previously. Do you want me to separately attach the html report for both tests (RV32IM and RV64IM) in the comment here ?

P.S. I think there should also be a requirement in PR checklist for attaching the test report in case the tests are regenerated completely again.

[allenjbaum]

The coverage report (and other artifacts) was supposed to be filed in google drive : for risc-v members > Workgroups > ACT Test Devlopment Artifacts > Artifacts-PR : https://drive.google.com/drive/folders/1C70-DJPSV2HxNPbHg6qL9wvVadlmAB2h Look at the readme in the folder

…

On Wed, Nov 1, 2023 at 11:31 PM Ali Tariq ***@***.***> wrote: @allenjbaum <https://github.com/allenjbaum> You are right. There is a test report generated in html format but I did not include it in this PR since I needed to just add the tests and I did not see a test report added in the repo previously. Do you want me to separately attach the html report for both tests (RV32IM and RV64IM) in the comment here ? P.S. I think there should also be a requirement in PR checklist for attaching the test report in case the tests are regenerated completely again. — Reply to this email directly, view it on GitHub <#404 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJQCCU2JQ2YHZGQSAEDYCM45TAVCNFSM6AAAAAA6XHRXKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJQGE2DQMZRGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[alitariq4589]

Thanks for pointing. I had already added the coverage reports. I will also include the test reports in the respective zipped archives in this folder.

[alitariq4589]

@allenjbaum I have re-uploaded the zip files with report folders named as test_reports inside rv32i_m/M.zip and rv64i_m/M.zip. There is report.html in that folder containing the reports of tests.

[allenjbaum]

I see the specific case(s) in the test report for rem and div, but not for divu or remu. Since there is no "largest negative number" of unsigned values, that's OK, though

[allenjbaum]

Seems to still want the Changelog to be updated

[alitariq4589]

Seems to still want the Changelog to be updated

I had updated the changelog.md with version 3.8.1 in this PR

[alitariq4589]

Oh. There are some changelog file conflicts. I will resolve them

[alitariq4589]

@allenjbaum I have resolved the conflicts and added a changelog entry. Its good to be merged.

[allenjbaum]

There was a merge that went through in the meantime, so you'll need to do it again. This is pretty annoying (and not your fault) - we need a better way to manage this.

…

On Mon, Nov 13, 2023 at 11:50 PM Ali Tariq ***@***.***> wrote: @allenjbaum <https://github.com/allenjbaum> I have resolved the conflicts and added a changelog entry. Its good to be merged. — Reply to this email directly, view it on GitHub <#404 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJXSY6ELYSLDS3CF3G3YEMPEVAVCNFSM6AAAAAA6XHRXKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBZGY4TINZRGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>