Move dbus_dat/rs2/shamt storage to bufreg2

doc/index.rst

@@ -53,6 +53,15 @@ For two-stage operations, serv_bufreg holds data between stages. This data can b
 
 .. image:: serv_bufreg_int.png
 
+serv_bufreg2
+^^^^^^^^^^^^
+
+.. image:: serv_bufreg2.png
+
+serv_bugreg2 is a 32-bit buffer register with some special features. It is used for shift operations to store the shift amount. It's used in load and store operations to store the data to be written or be read from the data bus, and it holds rs2 for the SERV extension interface. For shift and store operations, the register is shifted in from MSB when dat_en is asserted, while for loads and uses of the extension interface, the whole data word is written to when the i_load signal is asserted. Once the data is in the buffer, it is used differently depending on the operation. For stores and the extension interface the whole buffer is directly connected to the data bus as a 32-bit register. For load operations, the data is fed out serially once it has been fetched from the data bus. To better support load operations of varying sizes the buffer contains logic for reading out data serially from any of the byte LSBs of the 32-bit word. Finally, in shift mode, the 6 LSB of the register is used as a downcounter that is initialized during the init stage and then counts the remaining number of steps to shift the data and signals using sh_done and sh_done_r when finished.
+
+.. image:: serv_bufreg2_int.png
+
 serv_csr
 ^^^^^^^^
 
@@ -101,16 +110,14 @@ serv_mem_if
 
 .. image:: serv_mem_if.png
 
-serv_mem_if prepares the data to be sent out on the dbus during store operations and serializes the incoming data during loads
+serv_mem_if contains the control logic for preparing the data to be sent out on the dbus during store operations and sign-extends the incoming data from bufreg2 during loads
 
-The memory interface is centered around four byte-wide shift registers connected in series. During store operations, the `dat_en` signal is asserted long enough to shift in the data from rs2 to the right place in the shift registers and the parallel output of the shift registers is then presented to the data bus as a 32-bit word together with a byte mask. The `Data bus byte mask`_ table summarizes the logic for when the individual byte select signals are asserted depending on the two LSB of the data address together with the size (byte, halfword, word) of the write operation.
+The memory interface is centered around four serially connected byte-wide shift registers located in serv_bufreg2. During store operations, the `o_byte_valid` signal is asserted long enough to shift in the data from rs2 to the right place in the shift registers. The `Data bus byte mask`_ table summarizes the logic for when the individual byte select signals are asserted depending on the two LSB of the data address together with the size (byte, halfword, word) of the write operation.
 
-During load operations, the data from the bus is latched into the shift registers. `dat_en` is again asserted to shift out data from the registers. `i_lsb` decides from which byte stage of the shift register to tap the data, depending on the alignment of the received data. The `dat_valid` signal makes sure to only present valid data to `o_rd` and otherwise fill in with zeros or sign extension.
+During load operations, the data from the bus is read from serv_bufreg2. `dat_en` is again asserted to shift out data from the registers. `i_lsb` decides from which byte stage of the shift register to tap the data, depending on the alignment of the received data. The `dat_valid` signal makes sure to only present valid data to `o_rd` and otherwise fill in with zeros or sign extension.
 
 When SERV is built with `WITH_CSR`, there is also logic to detect misaligned accesses which asserts the o_misalign flag to the core.
 
-The shift register used for stores and loads are also used to keep track of the number of steps to shift for left/right shift operations. In this mode, the six LSB of the register is loaded with the shift amount during the init stage and the used as a down counter which raises o_sh_done and o_sh_done_r when the number of shift steps have been completed.
-
 .. image:: serv_mem_if_int.png
 
 .. _`Data bus byte mask`:

rtl/serv_bufreg2.v

@@ -0,0 +1,65 @@
+module serv_bufreg2
+  (
+   input wire 	      i_clk,
+   //State
+   input wire 	      i_en,
+   input wire 	      i_init,
+   input wire 	      i_cnt_done,
+   input wire [1:0]   i_lsb,
+   input wire 	      i_byte_valid,
+   output wire 	      o_sh_done,
+   output wire 	      o_sh_done_r,
+   //Control
+   input wire 	      i_op_b_sel,
+   input wire 	      i_shift_op,
+   //Data
+   input wire 	      i_rs2,
+   input wire 	      i_imm,
+   output wire 	      o_op_b,
+   output wire 	      o_q,
+   //External
+   output wire [31:0] o_dat,
+   input wire 	      i_load,
+   input wire [31:0]  i_dat);
+
+   reg [31:0] 	 dat;
+
+   assign o_op_b = i_op_b_sel ? i_rs2 : i_imm;
+
+   wire 	 dat_en = i_shift_op | (i_en & i_byte_valid);
+
+   /* The dat register has three different use cases for store, load and
+    shift operations.
+    store : Data to be written is shifted to the correct position in dat during
+            init by dat_en and is presented on the data bus as o_wb_dat
+    load  : Data from the bus gets latched into dat during i_wb_ack and is then
+            shifted out at the appropriate time to end up in the correct
+            position in rd
+    shift : Data is shifted in during init. After that, the six LSB are used as
+            a downcounter (with bit 5 initially set to 0) that triggers
+            o_sh_done and o_sh_done_r when they wrap around to indicate that
+            the requested number of shifts have been performed
+    */
+   wire [5:0] dat_shamt = (i_shift_op & !i_init) ?
+	      //Down counter mode
+	      dat[5:0]-1 :
+	      //Shift reg mode with optional clearing of bit 5
+	      {dat[6] & !(i_shift_op & i_cnt_done),dat[5:1]};
+
+   assign o_sh_done = dat_shamt[5];
+   assign o_sh_done_r = dat[5];
+
+   assign o_q =
+	       ((i_lsb == 2'd3) & dat[24]) |
+	       ((i_lsb == 2'd2) & dat[16]) |
+	       ((i_lsb == 2'd1) & dat[8]) |
+	       ((i_lsb == 2'd0) & dat[0]);
+
+   assign o_dat = dat;
+
+   always @(posedge i_clk) begin
+      if (dat_en | i_load)
+	dat <= i_load ? i_dat : {o_op_b, dat[31:7], dat_shamt};
+   end
+
+endmodule

rtl/serv_mem_if.v

@@ -2,34 +2,25 @@
 module serv_mem_if
   #(parameter [0:0] WITH_CSR = 1)
   (
-   input wire 	      i_clk,
+   input wire 	     i_clk,
    //State
-   input wire 	      i_en,
-   input wire 	      i_init,
-   input wire 	      i_cnt_done,
-   input wire [1:0]   i_bytecnt,
-   input wire [1:0]   i_lsb,
-   output wire 	      o_misalign,
-   output wire 	      o_sh_done,
-   output wire 	      o_sh_done_r,
+   input wire [1:0]  i_bytecnt,
+   input wire [1:0]  i_lsb,
+   output wire 	     o_byte_valid,
+   output wire 	     o_misalign,
    //Control
-   input wire 	      i_shift_op,
-   input wire 	      i_signed,
-   input wire 	      i_word,
-   input wire 	      i_half,
+   input wire 	     i_signed,
+   input wire 	     i_word,
+   input wire 	     i_half,
    //MDU
-   input wire         i_mdu_op,
+   input wire 	     i_mdu_op,
    //Data
-   input wire 	      i_op_b,
-   output wire 	      o_rd,
+   input wire 	     i_bufreg2_q,
+   output wire 	     o_rd,
    //External interface
-   output wire [31:0] o_wb_dat,
-   output wire [3:0]  o_wb_sel,
-   input wire [31:0]  i_wb_rdt,
-   input wire 	      i_wb_ack);
+   output wire [3:0] o_wb_sel);
 
    reg           signbit;
-   reg [31:0] 	 dat;
 
    /*
     Before a store operation, the data to be written needs to be shifted into
@@ -39,62 +30,29 @@ module serv_mem_if
     clever enough so the hideous expression below is used to achieve the same
     thing in a more optimal way.
     */
-   wire 	 byte_valid =
-		 (!i_lsb[0] & !i_lsb[1])         |
-		 (!i_bytecnt[0] & !i_bytecnt[1]) |
-		 (!i_bytecnt[1] & !i_lsb[1])     |
-		 (!i_bytecnt[1] & !i_lsb[0])     |
-		 (!i_bytecnt[0] & !i_lsb[1]);
-
-   wire 	 dat_en = i_shift_op | (i_en & byte_valid);
-
-   wire 	 dat_cur =
-		 ((i_lsb == 2'd3) & dat[24]) |
-		 ((i_lsb == 2'd2) & dat[16]) |
-		 ((i_lsb == 2'd1) & dat[8]) |
-		 ((i_lsb == 2'd0) & dat[0]);
+   assign o_byte_valid
+     = (!i_lsb[0] & !i_lsb[1])         |
+       (!i_bytecnt[0] & !i_bytecnt[1]) |
+       (!i_bytecnt[1] & !i_lsb[1])     |
+       (!i_bytecnt[1] & !i_lsb[0])     |
+       (!i_bytecnt[0] & !i_lsb[1]);
 
    wire dat_valid =
+	i_mdu_op |
 	i_word |
 	(i_bytecnt == 2'b00) |
 	(i_half & !i_bytecnt[1]);
 
-   assign o_rd = (dat_valid|i_mdu_op) ? dat_cur : signbit & i_signed;
+   assign o_rd = dat_valid ? i_bufreg2_q : signbit & i_signed;
 
    assign o_wb_sel[3] = (i_lsb == 2'b11) | i_word | (i_half & i_lsb[1]);
    assign o_wb_sel[2] = (i_lsb == 2'b10) | i_word;
    assign o_wb_sel[1] = (i_lsb == 2'b01) | i_word | (i_half & !i_lsb[1]);
    assign o_wb_sel[0] = (i_lsb == 2'b00);
 
-   assign o_wb_dat = dat;
-
-   /* The dat register has three different use cases for store, load and
-    shift operations.
-    store : Data to be written is shifted to the correct position in dat during
-            init by dat_en and is presented on the data bus as o_wb_dat
-    load  : Data from the bus gets latched into dat during i_wb_ack and is then
-            shifted out at the appropriate time to end up in the correct
-            position in rd
-    shift : Data is shifted in during init. After that, the six LSB are used as
-            a downcounter (with bit 5 initially set to 0) that triggers
-            o_sh_done and o_sh_done_r when they wrap around to indicate that
-            the requested number of shifts have been performed
-    */
-   wire [5:0] dat_shamt = (i_shift_op & !i_init) ?
-	      //Down counter mode
-	      dat[5:0]-1 :
-	      //Shift reg mode with optional clearing of bit 5
-	      {dat[6] & !(i_shift_op & i_cnt_done),dat[5:1]};
-
-   assign o_sh_done = dat_shamt[5];
-   assign o_sh_done_r = dat[5];
-
    always @(posedge i_clk) begin
-      if (dat_en | i_wb_ack)
-	dat <= i_wb_ack ? i_wb_rdt : {i_op_b, dat[31:7], dat_shamt};
-
       if (dat_valid)
-        signbit <= dat_cur;
+        signbit <= i_bufreg2_q;
    end
 
    /*

rtl/serv_top.v

@@ -123,6 +123,7 @@ module serv_top
    wire 	 bufreg_imm_en;
    wire 	 bufreg_clr_lsb;
    wire 	 bufreg_q;
+   wire 	 bufreg2_q;
    wire [31:0] dbus_rdt;
    wire        dbus_ack;
 
@@ -137,14 +138,16 @@ module serv_top
    wire          rs2;
    wire          rd_en;
 
-   wire          op_b_source;
+   wire          op_b;
+   wire          op_b_sel;
 
    wire          mem_signed;
    wire          mem_word;
    wire          mem_half;
    wire [1:0] 	 mem_bytecnt;
-   wire 	 mem_sh_done;
-   wire 	 mem_sh_done_r;
+   wire 	 sh_done;
+   wire 	 sh_done_r;
+   wire 	 byte_valid;
 
    wire 	 mem_misalign;
 
@@ -168,7 +171,6 @@ module serv_top
 
    wire [1:0]   lsb;
 
-   wire 	op_b = op_b_source ? rs2 : imm;
 
    serv_state
      #(.RESET_STRATEGY (RESET_STRATEGY),
@@ -196,8 +198,8 @@ module serv_top
       .o_ctrl_jump    (jump),
       .o_ctrl_trap    (trap),
       .i_ctrl_misalign(lsb[1]),
-      .i_sh_done      (mem_sh_done),
-      .i_sh_done_r    (mem_sh_done_r),
+      .i_sh_done      (sh_done),
+      .i_sh_done_r    (sh_done_r),
       .o_mem_bytecnt  (mem_bytecnt),
       .i_mem_misalign (mem_misalign),
       //Control
@@ -251,19 +253,20 @@ module serv_top
       .o_two_stage_op     (two_stage_op),
       //Extension
       .o_ext_funct3       (o_ext_funct3),
-      
+
       //To bufreg
       .o_bufreg_rs1_en    (bufreg_rs1_en),
       .o_bufreg_imm_en    (bufreg_imm_en),
       .o_bufreg_clr_lsb   (bufreg_clr_lsb),
       .o_bufreg_sh_signed (bufreg_sh_signed),
+      //To bufreg2
+      .o_op_b_source      (op_b_sel),
       //To ctrl
       .o_ctrl_jal_or_jalr (jal_or_jalr),
       .o_ctrl_utype       (utype),
       .o_ctrl_pc_rel      (pc_rel),
       .o_ctrl_mret        (mret),
       //To alu
-      .o_op_b_source      (op_b_source),
       .o_alu_sub          (alu_sub),
       .o_alu_bool_op      (alu_bool_op),
       .o_alu_cmp_eq       (alu_cmp_eq),
@@ -312,7 +315,7 @@ module serv_top
       .i_wb_en      (i_ibus_ack),
       .i_wb_rdt     (i_ibus_rdt[31:7]));
 
-   serv_bufreg 
+   serv_bufreg
       #(.MDU(MDU))
    bufreg
      (
@@ -337,6 +340,30 @@ module serv_top
       .o_dbus_adr (o_dbus_adr),
       .o_ext_rs1  (o_ext_rs1));
 
+   serv_bufreg2 bufreg2
+     (
+      .i_clk        (clk),
+      //State
+      .i_en         (cnt_en),
+      .i_init       (init),
+      .i_cnt_done   (cnt_done),
+      .i_lsb        (lsb),
+      .i_byte_valid (byte_valid),
+      .o_sh_done    (sh_done),
+      .o_sh_done_r  (sh_done_r),
+      //Control
+      .i_op_b_sel   (op_b_sel),
+      .i_shift_op   (shift_op),
+      //Data
+      .i_rs2        (rs2),
+      .i_imm        (imm),
+      .o_op_b       (op_b),
+      .o_q          (bufreg2_q),
+      //External
+      .o_dat        (o_dbus_dat),
+      .i_load       (dbus_ack),
+      .i_dat        (dbus_rdt));
+
    serv_ctrl
      #(.RESET_PC (RESET_PC),
        .RESET_STRATEGY (RESET_STRATEGY),
@@ -437,30 +464,22 @@ module serv_top
      #(.WITH_CSR (WITH_CSR))
    mem_if
      (
-      .i_clk      (clk),
+      .i_clk        (clk),
       //State
-      .i_en       (cnt_en),
-      .i_init     (init),
-      .i_cnt_done (cnt_done),
-      .i_bytecnt  (mem_bytecnt),
-      .i_lsb      (lsb),
-      .o_misalign (mem_misalign),
-      .o_sh_done   (mem_sh_done),
-      .o_sh_done_r (mem_sh_done_r),
+      .i_bytecnt    (mem_bytecnt),
+      .i_lsb        (lsb),
+      .o_byte_valid (byte_valid),
+      .o_misalign   (mem_misalign),
       //Control
-      .i_mdu_op   (mdu_op),
-      .i_shift_op (shift_op),
-      .i_signed   (mem_signed),
-      .i_word     (mem_word),
-      .i_half     (mem_half),
+      .i_mdu_op     (mdu_op),
+      .i_signed     (mem_signed),
+      .i_word       (mem_word),
+      .i_half       (mem_half),
       //Data
-      .i_op_b     (op_b),
-      .o_rd       (mem_rd),
+      .i_bufreg2_q  (bufreg2_q),
+      .o_rd         (mem_rd),
       //External interface
-      .o_wb_dat   (o_dbus_dat),
-      .o_wb_sel   (o_dbus_sel),
-      .i_wb_rdt   (dbus_rdt),
-      .i_wb_ack   (dbus_ack));
+      .o_wb_sel     (o_dbus_sel));
 
    generate
       if (WITH_CSR) begin
@@ -579,7 +598,7 @@ generate
     assign dbus_rdt = i_ext_ready ? i_ext_rd:i_dbus_rdt;
     assign dbus_ack = i_dbus_ack | i_ext_ready;
   end else begin
-    assign dbus_rdt = i_dbus_rdt;  
+    assign dbus_rdt = i_dbus_rdt;
     assign dbus_ack = i_dbus_ack;
   end
   assign o_ext_rs2 = o_dbus_dat;

serv.core

@@ -7,6 +7,7 @@ filesets:
     files:
       - "tool_verilator? (data/verilator_waiver.vlt)" : {file_type: vlt}
       - rtl/serv_bufreg.v
+      - rtl/serv_bufreg2.v
       - rtl/serv_alu.v
       - rtl/serv_csr.v
       - rtl/serv_ctrl.v