Zbb extension integration + opcode restructuring#260
Conversation
|
for ci.yaml workflow: - name: Run RISCOF
- run: |
- if make riscof_run UTOSS_RISCV_CONFIG=${{ matrix.config }}; then
- :
- else
- case "${{ matrix.config }}" in
- *B*)
- echo "RISCOF failed for B-containing config; allowing failure."
- ;;
- *)
- exit 1
- ;;
- esac
- fi
+ run: make riscof_run UTOSS_RISCV_CONFIG=${{ matrix.config }} |
ci.yaml workflow
ci.yaml workflow edit
| //verilator lint_off UNUSEDSIGNAL | ||
|
|
||
| //verilator lint_on UNUSEDSIGNAL |
There was a problem hiding this comment.
can probably remove those
| //verilator lint_off UNUSEDSIGNAL | |
| //verilator lint_on UNUSEDSIGNAL |
| localparam bit [6:0] FUNCT7_ZBA = 7'b0010000; | ||
| localparam bit [6:0] FUNCT7_ZBB__LOGICAL = 7'b0100000; | ||
| localparam bit [6:0] FUNCT7_ZBB__MINMAX = 7'b0000101; | ||
| localparam bit [6:0] FUNCT7_ZBB__SEXT = 7'b0110000; |
There was a problem hiding this comment.
why are we removing this one? i guess its not technically and operations since its either .B or .H, but i think we have the same patter for other instructions that get grouped by opcode
There was a problem hiding this comment.
ohh i think i understand why you added rs2 to the main decoder, lemme take a closer look at that and get back to you. this is a bit confusing to me since clz for example is an I-type instruction and is not supposed to have an rs2, but this is where the instruction designation is stored.
TheDeepestSpace
changed the title
| , AUIPC = 7'b0010111 //U-type | ||
| , LUI = 7'b0110111 //U-type | ||
| , MISC_MEM = 7'b0001111 //FENCE | ||
| } opcode_name_t; |
There was a problem hiding this comment.
| } opcode_name_t; | |
| } opcode_t; |
There was a problem hiding this comment.
removed //typedef logic [6:0] opcode_t; from src/headers/types.svh
in Instruction_Decode.sv: +assign opcode = opcode_t'(instr[6:0]);
| { OP = 7'b0110011 //R-type | ||
| , OP_IMM = 7'b0010011 //I-type logic | ||
| , LOAD = 7'b0000011 //I-type load | ||
| , STORE = 7'b0100011 //S-type | ||
| , BRANCH = 7'b1100011 //B-type | ||
| , JAL = 7'b1101111 //J-type | ||
| , JALR = 7'b1100111 //I-type jump | ||
| , AUIPC = 7'b0010111 //U-type | ||
| , LUI = 7'b0110111 //U-type | ||
| , MISC_MEM = 7'b0001111 //FENCE | ||
| } opcode_name_t; |
There was a problem hiding this comment.
I'd prepend each entiry in the enaum with OPCODE_ for consistency with the way we define other enums (to avoid collisions) even though we will have OPCODE_OP which sounds tautological.
| `ifdef UTOSS_RISCV_ENABLE_B_EXT | ||
| assign id_to_ex.b_alu_control = b_alu_control; | ||
| `endif |
There was a problem hiding this comment.
nitpick: could move that to the bottom of the assignment list for consistency
| `ifdef UTOSS_RISCV_ENABLE_B_EXT | ||
| zbb u_zbb | ||
| ( .a ( alu_input_a ) | ||
| , .b ( alu_input_b ) | ||
| , .b_alu_control ( id_to_ex.b_alu_control ) | ||
| , .out ( zbb_result ) | ||
| , .zeroE ( zbb_zero_flag ) | ||
| ); | ||
| `endif |
There was a problem hiding this comment.
also a nitpick: i'd move this instantiation logic above the combinational block that decides on the final alu result
There was a problem hiding this comment.
lets use our new enum here!
| , input opcode_t opcode |
enum comment changes
enum comment changes
changed from decoder c
added system
moved enum def
2 participants
No description provided.