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 Zacas extension #492

Closed
wants to merge 8 commits into from
Closed

Conversation

ved-rivos
Copy link

@ved-rivos ved-rivos commented Jul 14, 2023

This PR adds tests for the Zacas extension that introduces the following instructions:

  • amocas.w for RV32 and RV64
  • amocas.d for RV32 and RV64
  • amocas.q for RV64 only
    The specification is here: https://github.com/riscv/riscv-zacas
    The amocas instructions are emitted using a .word directive pending assembler support.

@aswaterman
Copy link
Collaborator

@ved-rivos did you make sure these can run on Spike?

@ved-rivos
Copy link
Author

I have tested them on sail but not on spike.

@aswaterman
Copy link
Collaborator

Good enough for me.

@aswaterman
Copy link
Collaborator

Actually, good enough for me to believe the test cases are correct, but not good enough for being able to merge this :) We need it to be the case that executing make run in the isa directory succeeds. I think that means you need to add _zacas to the ISA strings in isa/Makefile so that the extension will be enabled for Spike. After doing that and making sure they run on Spike, we can merge.

@ved-rivos
Copy link
Author

Thanks. These and all other tests pass on spike but the breakpoint.S test seems to be failing. I am not sure if I caused it (I most likely did not since I only added amocas tests :) ). I will update the PR after that debug that to be sure.

@ved-rivos
Copy link
Author

Updated PR to extend the ISA string (breakpoint.S failure seems unrelated to addition of these tests).
FYI - SAIL PR: riscv/sail-riscv#285

@aswaterman
Copy link
Collaborator

Thanks, @ved-rivos. I’m sure you’re right, and I’ll look into the breakpoint test failure myself.

@ved-rivos
Copy link
Author

ved-rivos commented Jul 18, 2023

@aswaterman - Found some time to look at this today. Seems like the test was not factoring in the case that when tcontrol is not implemented, mstatus.mie must be 1 for trigger with action=0 to match. I have sent PR #494 with a fix.

@ved-rivos ved-rivos changed the title Add unratified Zacas extension Add Zacas extension Feb 3, 2024
@ved-rivos ved-rivos closed this Oct 3, 2024
@aswaterman
Copy link
Collaborator

@ved-rivos sorry we forgot to finish this up. If it's fully working now, LMK.

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