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 unratified Zabha extension #1491

Merged
merged 6 commits into from
Nov 2, 2023
Merged

Conversation

ved-rivos
Copy link
Contributor

Specification: https://github.com/riscv/riscv-zabha
Tested with RISC-V arch. tests and checked against sail.

@@ -0,0 +1,3 @@
require_extension('A');
require_extension(EXT_ZABHA);
WRITE_RD((sreg_t)(int8_t)(MMU.amo<uint8_t>(RS1, [&](uint8_t lhs) { return lhs + RS2; })));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason we can't simplify this to the following:

WRITE_RD(sreg_t(MMU.amo<int8_t>(RS1, [&](int8_t lhs) { return lhs + RS2; })));

If not, let's do the same for everything in Zabha.

Copy link
Contributor Author

@ved-rivos ved-rivos Nov 2, 2023

Choose a reason for hiding this comment

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

The type cast to int8_t was to do the sign extension and is required. I have updated it to:
WRITE_RD(sreg_t((int8_t)MMU.amo<int8_t>(RS1, [&](int8_t lhs) { return lhs + RS2; })));

@aswaterman

Copy link
Collaborator

Choose a reason for hiding this comment

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

Understood that sign extension is the reason, but you can delete the int8_t cast, because after this change, the function’s return type is already int8_t.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aswaterman - I missed that part. I have updated to remove the explicit cast.

riscv/insns/amoadd_b.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

OK with me, after my comments are addressed. Thanks!

@ved-rivos ved-rivos force-pushed the zabha branch 2 times, most recently from 3cc203a to d244d2c Compare November 2, 2023 16:24
Copy link
Collaborator

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks.

@aswaterman aswaterman merged commit 5a11457 into riscv-software-src:master Nov 2, 2023
3 checks passed
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.

2 participants