Compressed Extension for SERV #79
Conversation
There was a problem hiding this comment.
Very good! On the technical side it looks fine. As you mention, there are some optimizations that can be done but I think it's better to pull this in and make those optimizations later.
Most comments are more stylistic things like making sure to not change indentation of signals. I know the current indentation is a bit weird unless you use Emacs, but let's try to keep it as it is. Maybe the easiest way is that I fix the indentation once all the other things are fixed (parameters, license header, etc)
rtl/serv_aligner.v
Outdated
| module serv_aligner | ||
| #(parameter ALIGN = 0) | ||
| ( | ||
| input clk,rst, |
There was a problem hiding this comment.
Put rst on a separate line. Use "input wire" on both signals. Most tools are ok without specifying wire, but Vivado complains when `default_nettype none is set
rtl/serv_aligner.v
Outdated
| @@ -0,0 +1,93 @@ | |||
| module serv_aligner | |||
| #(parameter ALIGN = 0) | |||
There was a problem hiding this comment.
Use parameter [0:0] for one-bit parameters. Some SystemVerilog compilers complains otherwise
rtl/serv_aligner.v
Outdated
| input wire [31:0] i_wb_ibus_rdt, | ||
| input wire i_wb_ibus_ack | ||
| ); | ||
| parameter fetch_align = 2'b00; |
There was a problem hiding this comment.
Define these as localparam instead of parameter. We only want to use parameter for values that we intend to change from the outside
rtl/serv_compdec.v
Outdated
| @@ -0,0 +1,238 @@ | |||
| module serv_compdec | |||
| #(parameter COMPRESSED=0) | |||
rtl/serv_rf_top.v
Outdated
| /* COMPRESSED=1: Enable the compressed decoder and allowed misaligned jump of pc | ||
| COMPRESSED=0: Disable the compressed decoder and does not allow the misaligned jump of pc | ||
| */ | ||
| parameter COMPRESSED = 0, |
rtl/serv_rf_top.v
Outdated
| ALIGN = 1: Fetch the aligned instruction by making two bus transactions if the misaligned address | ||
| is given to the instruction bus. | ||
| */ | ||
| parameter ALIGN = 0, |
rtl/serv_rf_top.v
Outdated
| @@ -91,6 +100,7 @@ module serv_rf_top | |||
| wire [RF_L2D-1:0] raddr; | |||
| wire [RF_WIDTH-1:0] rdata; | |||
|
|
|||
|
|
|||
rtl/serv_top.v
Outdated
| ( | ||
| input wire clk, | ||
| input wire i_rst, | ||
| input wire i_timer_irq, | ||
| `ifdef RISCV_FORMAL | ||
| output reg rvfi_valid = 1'b0, |
There was a problem hiding this comment.
Avoid reindendation of ports here. The current indentation is done by the Verilog mode in Emacs. I'm aware it has some strange rules about indentation but I don't want to change that as a part of this PR
There was a problem hiding this comment.
I didn't notice that my reindentation using VScode made a mess to the indentation on GitHub. Apologies!
rtl/serv_compdec.v
Outdated
| endcase | ||
| end | ||
| reg comp1; | ||
| always @(negedge i_ack) begin |
There was a problem hiding this comment.
We don't want to clock any logic on negative edges. It can be very problematic for timing
|
Alright! I think we have merged everything now and we can close this! |
This project is the part of spring '22 LFX Mentorship program. RISC-V Compressed extension support is added to SERV core. All the required compliance tests are passing.
ToDo:
OPTIMIZATIONS!!!