-
Notifications
You must be signed in to change notification settings - Fork 706
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
Zcmp extension support #1779
Zcmp extension support #1779
Conversation
❌ failed run, report available here. |
cva6/docs/verif-approval: add file describing task approval process
Hi @arshadrohan5, there's an other PR for the Zcmp instruction for the Specification |
Thanks @arshadrohan5 for this nice (and very wanted feature to reduce code size) contribution. Up to now the CO is failed. DO you have enough information to fix it ? |
No, I don't have enough information to fix it. |
For of all, I would propose you to run the smoke-tests on your local env fllowing the README.md Let me know |
As the push/pop instructions are transformed into a series of store/load instructions, what is the expected behaviour when an interruption occurs during the execution of the series of store/load instructions? Is the series interrupted? Is the whole series executed and the interruption handled afterwards? Similar questions exist also when an exception occurs. These behaviours should be described to know how to verify this extension. |
1038667
to
008bab6
Compare
❌ failed run, report available here. |
6 similar comments
❌ failed run, report available here. |
❌ failed run, report available here. |
❌ failed run, report available here. |
❌ failed run, report available here. |
❌ failed run, report available here. |
❌ failed run, report available here. |
As the push/pop instructions are transformed into a series of instructions and as these instructions are going through the issue, the execute and the commit stages, |
In the RVFI trace generated by the CVA6, each retired (committed) instruction has to be traced. With this current Zcmp implementation, the trace will contain multiple instructions (store/load, add) instead of one (push/pop). |
core/zcmp_decoder.sv
Outdated
if (riscv::XLEN == 64) begin | ||
if (issue_ack_i && is_zcmp_instr_i && zcmp_instr_type == PUSH) begin | ||
// addi sp, sp, stack_adj | ||
instr_o_reg = {itype_inst.imm, 5'h2, 3'h0, 5'h2, riscv::OpcodeOpImm}; | ||
end else begin | ||
if (issue_ack_i) begin | ||
instr_o_reg = {stack_adj, 5'h2, 3'h0, 5'h2, riscv::OpcodeOpImm}; | ||
end | ||
end | ||
if (issue_ack_i && is_zcmp_instr_i && (zcmp_instr_type == POPRETZ || zcmp_instr_type == POPRET)) begin | ||
state_d = POPRETZ_1; | ||
fetch_stall_o = 1; | ||
end else begin | ||
if (issue_ack_i) begin | ||
state_d = IDLE; | ||
fetch_stall_o = 0; | ||
end else begin | ||
fetch_stall_o = 1; | ||
end | ||
end | ||
end else begin | ||
if (issue_ack_i && is_zcmp_instr_i && zcmp_instr_type == PUSH) begin | ||
// addi sp, sp, stack_adj | ||
instr_o_reg = {itype_inst.imm, 5'h2, 3'h0, 5'h2, riscv::OpcodeOpImm}; | ||
end else begin | ||
if (issue_ack_i) begin | ||
instr_o_reg = {stack_adj, 5'h2, 3'h0, 5'h2, riscv::OpcodeOpImm}; | ||
end | ||
end | ||
if (issue_ack_i && is_zcmp_instr_i && (zcmp_instr_type == POPRETZ || zcmp_instr_type == POPRET)) begin | ||
state_d = POPRETZ_1; | ||
fetch_stall_o = 1; | ||
end else begin | ||
if (issue_ack_i) begin | ||
state_d = IDLE; | ||
fetch_stall_o = 0; | ||
end else begin | ||
fetch_stall_o = 1; | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the code for RV64 is the same as the one for RV32.
Can you remove the "rename" file. |
@@ -38,6 +38,8 @@ module commit_stage | |||
input scoreboard_entry_t [CVA6Cfg.NrCommitPorts-1:0] commit_instr_i, | |||
// Acknowledge that we are indeed committing - ISSUE_STAGE | |||
output logic [CVA6Cfg.NrCommitPorts-1:0] commit_ack_o, | |||
// Acknowledge that we are indeed committing - CSR_REGFILE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why CSR_REGFILE
and not ISSUE_STAGE
as destination of commit_macro_ack_o
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal was to increment the CSR register only once for a macro instruction. We still want to be able to Issue and Execute the decoded sequence of instructions. Please note here that commit_macro_ack_o
is asserted only when we execute the last decoded 32bit instruction in the macro definition, otherwise it behaves exactly like commit_ack_o
.
❌ failed run, report available here. |
1 similar comment
❌ failed run, report available here. |
FYI the Dashboard service is down, I keep you in touch |
The dashboard is up and running again, but the report for the fpga build job is "failed" due to a previous Gitlab Runner issue, I started the job again. |
It seems there is still formatting issues. See this. I just take a look at the verilator build log. A few lint warnings are reported.
|
❌ failed run, report available here. |
1 similar comment
❌ failed run, report available here. |
a273ea8
to
599253b
Compare
❌ failed run, report available here. |
✔️ successful run, report available here. |
Thanks for your patience, let's merge. |
Zcmp Extension
Introduction
This PR is adding a set of instructions(cm.push, cm.pop, cm.popret, cm.popretz, cm.mvsa01, and cm.mva01s) that may be executed as a series of existing 32-bit RISC-V instructions. The objective is to reduce the overall code size for embedded platforms.
Adding Support in CVA6
Since the new set of instructions is executed as a series of existing 32-bit instructions we need a special sequential decoder that stalls the fetching of a new instruction until all the corresponding instructions are generated (one instruction in every cycle) and issued.
This new decoder extends the existing compressed decoder and the main decoder in the instruction decode stage.
Our new zcmp_decoder has the following important features:
It has an input signal to indicate that the current instruction is a zcmp instruction and needs to be decoded into a series of 32-bit instructions.
It has an input signal from the Issue Stage indicating the last issued instruction has been acknowledged. So, we can decode and send the next instruction in the series.
It has an output signal to indicate that the zcmp decoder is busy decoding the current instruction into a series of instructions and instruction fetching should be stalled as long as it is busy.
Design Changes
compressed_decoder
Updated the compressed decoder to indicate that the current compressed instruction is of zcmp extension.
id_stage
As mentioned we need to instantiate and connect our new zcmp_decoder between the compressed decoder and the main larger decoder when the zcmp extension is enabled in the cva6 configuration. Furthermore, we need to stall the fetching of new instructions until our zcmp_decoder is busy.
Test Plan
Please find the tests under CVA6_REPO_DIR/verif/tests/custom/Zcmp/ along with the command to build the test binary file.