Fix DivSqrt lanes synchronization (#90)

* Fix DivSqrt lanes synchronization
* Tie further unused signals in fpnew_opgroup_multifmt to zero

Author: lucabertaccini

src/fpnew_divsqrt_multi.sv

@@ -38,6 +38,7 @@ module fpnew_divsqrt_multi #(
   input  TagType                      tag_i,
   input  logic                        mask_i,
   input  AuxType                      aux_i,
+  input  logic                        vectorial_op_i,
   // Input Handshake
   input  logic                        in_valid_i,
   output logic                        in_ready_o,
@@ -95,6 +96,7 @@ module fpnew_divsqrt_multi #(
   TagType                [0:NUM_INP_REGS]                       inp_pipe_tag_q;
   logic                  [0:NUM_INP_REGS]                       inp_pipe_mask_q;
   AuxType                [0:NUM_INP_REGS]                       inp_pipe_aux_q;
+  logic                  [0:NUM_INP_REGS]                       inp_pipe_vec_op_q;
   logic                  [0:NUM_INP_REGS]                       inp_pipe_valid_q;
   // Ready signal is combinatorial for all stages
   logic [0:NUM_INP_REGS] inp_pipe_ready;
@@ -107,8 +109,9 @@ module fpnew_divsqrt_multi #(
   assign inp_pipe_tag_q[0]      = tag_i;
   assign inp_pipe_mask_q[0]     = mask_i;
   assign inp_pipe_aux_q[0]      = aux_i;
+  assign inp_pipe_vec_op_q[0]   = vectorial_op_i;
   assign inp_pipe_valid_q[0]    = in_valid_i;
-  // Input stage: Propagate pipeline ready signal to updtream circuitry
+  // Input stage: Propagate pipeline ready signal to upstream circuitry
   assign in_ready_o = inp_pipe_ready[0];
   // Generate the register stages
   for (genvar i = 0; i < NUM_INP_REGS; i++) begin : gen_input_pipeline
@@ -130,6 +133,7 @@ module fpnew_divsqrt_multi #(
     `FFL(inp_pipe_tag_q[i+1],      inp_pipe_tag_q[i],      reg_ena, TagType'('0))
     `FFL(inp_pipe_mask_q[i+1],     inp_pipe_mask_q[i],     reg_ena, '0)
     `FFL(inp_pipe_aux_q[i+1],      inp_pipe_aux_q[i],      reg_ena, AuxType'('0))
+    `FFL(inp_pipe_vec_op_q[i+1],   inp_pipe_vec_op_q[i],   reg_ena, AuxType'('0))
   end
   // Output stage: assign selected pipe outputs to signals for later use
   assign operands_q = inp_pipe_operands_q[NUM_INP_REGS];
@@ -173,27 +177,45 @@ module fpnew_divsqrt_multi #(
   logic op_starting;            // high in the cycle a new operation starts
   logic out_valid, out_ready;   // output handshake with downstream
   logic unit_busy;              // valid data in flight
+  logic simd_synch_done;
   // FSM states
   typedef enum logic [1:0] {IDLE, BUSY, HOLD} fsm_state_e;
   fsm_state_e state_q, state_d;
 
-  // Ready synch with other lanes
-  // Bring the FSM-generated ready outside the unit, to synchronize it with the other lanes
-  assign divsqrt_ready_o = in_ready;
-  // Upstream ready comes from sanitization FSM, and it is synched among all the lanes
-  assign inp_pipe_ready[NUM_INP_REGS] = simd_synch_rdy_i;
+  // Valids are gated by the FSM ready. Invalid input ops run a sqrt to not lose illegal instr.
+  assign div_valid   = in_valid_q & (op_q == fpnew_pkg::DIV) & in_ready & ~flush_i;
+  assign sqrt_valid  = in_valid_q & (op_q != fpnew_pkg::DIV) & in_ready & ~flush_i;
+  assign op_starting = div_valid | sqrt_valid;
+
+  // Hold additional information while the operation is in progress
+  logic result_is_fp8_q;
+  TagType result_tag_q;
+  logic result_mask_q;
+  AuxType result_aux_q;
+  logic result_vec_op_q;
+
+  // Fill the registers everytime a valid operation arrives (load FF, active low asynch rst)
+  `FFL(result_is_fp8_q, input_is_fp8,                 op_starting, '0)
+  `FFL(result_tag_q,    inp_pipe_tag_q[NUM_INP_REGS], op_starting, '0)
+  `FFL(result_mask_q,   inp_pipe_mask_q[NUM_INP_REGS],op_starting, '0)
+  `FFL(result_aux_q,    inp_pipe_aux_q[NUM_INP_REGS], op_starting, '0)
+  `FFL(result_vec_op_q, inp_pipe_vec_op_q[NUM_INP_REGS], op_starting, '0)
+
+  // Wait for other lanes only if the operation is vectorial
+  assign simd_synch_done = simd_synch_done_i || ~result_vec_op_q;
 
   // Valid synch with other lanes
   // When one divsqrt unit completes an operation, keep its done high, waiting for the other lanes
   // As soon as all the lanes are over, we can clear this FF and start with a new operation
-  `FFLARNC(unit_done_q, unit_done, unit_done, simd_synch_done_i, 1'b0, clk_i, rst_ni);
+  `FFLARNC(unit_done_q, unit_done, unit_done, simd_synch_done, 1'b0, clk_i, rst_ni);
   // Tell the other units that this unit has finished now or in the past
-  assign divsqrt_done_o = unit_done_q | unit_done;
+  assign divsqrt_done_o = (unit_done_q | unit_done) & result_vec_op_q;
 
-  // Valids are gated by the FSM ready. Invalid input ops run a sqrt to not lose illegal instr.
-  assign div_valid   = in_valid_q & (op_q == fpnew_pkg::DIV) & in_ready & ~flush_i;
-  assign sqrt_valid  = in_valid_q & (op_q != fpnew_pkg::DIV) & in_ready & ~flush_i;
-  assign op_starting = div_valid | sqrt_valid;
+  // Ready synch with other lanes
+  // Bring the FSM-generated ready outside the unit, to synchronize it with the other lanes
+  assign divsqrt_ready_o = in_ready;
+  // Upstream ready comes from sanitization FSM, and it is synched among all the lanes
+  assign inp_pipe_ready[NUM_INP_REGS] = result_vec_op_q ? simd_synch_rdy_i : in_ready;
 
   // FSM to safely apply and receive data from DIVSQRT unit
   always_comb begin : flag_fsm
@@ -215,13 +237,13 @@ module fpnew_divsqrt_multi #(
       BUSY: begin
         unit_busy = 1'b1; // data in flight
         // If all the lanes are done with processing
-        if (simd_synch_done_i) begin
+        if (simd_synch_done_i || (~result_vec_op_q && unit_done)) begin
           out_valid = 1'b1; // try to commit result downstream
           // If downstream accepts our result
           if (out_ready) begin
             state_d = IDLE; // we anticipate going back to idling..
+            in_ready = 1'b1; // we acknowledge the instruction
             if (in_valid_q && unit_ready) begin // ..unless new work comes in
-              in_ready = 1'b1; // we acknowledge the instruction
               state_d  = BUSY; // and stay busy with it
             end
           // Otherwise if downstream is not ready for the result
@@ -258,18 +280,6 @@ module fpnew_divsqrt_multi #(
   // FSM status register (asynch active low reset)
   `FF(state_q, state_d, IDLE)
 
-  // Hold additional information while the operation is in progress
-  logic result_is_fp8_q;
-  TagType result_tag_q;
-  logic result_mask_q;
-  AuxType result_aux_q;
-
-  // Fill the registers everytime a valid operation arrives (load FF, active low asynch rst)
-  `FFL(result_is_fp8_q, input_is_fp8,                 op_starting, '0)
-  `FFL(result_tag_q,    inp_pipe_tag_q[NUM_INP_REGS], op_starting, '0)
-  `FFL(result_mask_q,   inp_pipe_mask_q[NUM_INP_REGS],op_starting, '0)
-  `FFL(result_aux_q,    inp_pipe_aux_q[NUM_INP_REGS], op_starting, '0)
-
   // -----------------
   // DIVSQRT instance
   // -----------------
@@ -298,9 +308,9 @@ module fpnew_divsqrt_multi #(
   // Adjust result width and fix FP8
   assign adjusted_result = result_is_fp8_q ? unit_result >> 8 : unit_result;
 
-  // Hold the result when one lane has finished execution, except when all the lanes finish together
-  // and the result can be accepted downstream
-  assign hold_en = unit_done & (~simd_synch_done_i | ~out_ready);
+  // Hold the result when one lane has finished execution, except when all the lanes finish together,
+  // or the operation is not vectorial, and the result can be accepted downstream
+  assign hold_en = unit_done & (~simd_synch_done_i | ~out_ready) & ~(~result_vec_op_q & out_ready);
   // The Hold register (load, no reset)
   `FFLNR(held_result_q, adjusted_result, hold_en, clk_i)
   `FFLNR(held_status_q, unit_status,     hold_en, clk_i)

src/fpnew_opgroup_multifmt_slice.sv

@@ -137,6 +137,8 @@ or set Features.FpFmtMask to support only FP32");
   // CONV passes one operand for assembly after the unit: opC for cpk, opB for others
   if (OpGroup == fpnew_pkg::CONV) begin : conv_target
     assign conv_target_d = dst_is_cpk ? operands_i[2] : operands_i[1];
+  end else begin : not_conv_target
+    assign conv_target_d = '0;
   end
 
   // For 2-operand units, prepare boxing info
@@ -302,6 +304,7 @@ or set Features.FpFmtMask to support only FP32");
             .tag_i,
             .mask_i           ( simd_mask_i[lane]   ),
             .aux_i            ( aux_data            ),
+            .vectorial_op_i   ( vectorial_op        ),
             .in_valid_i       ( in_valid            ),
             .in_ready_o       ( lane_in_ready[lane] ),
             .divsqrt_done_o   ( divsqrt_done[lane]  ),
@@ -373,7 +376,13 @@ or set Features.FpFmtMask to support only FP32");
     end else begin : inactive_lane
       assign lane_out_valid[lane] = 1'b0; // unused lane
       assign lane_in_ready[lane]  = 1'b0; // unused lane
-      assign local_result         = '{default: lane_ext_bit[0]}; // sign-extend/nan box
+      assign lane_aux[lane]       = 1'b0; // unused lane
+      assign lane_masks[lane]     = 1'b1; // unused lane
+      assign lane_tags[lane]      = 1'b0; // unused lane
+      assign divsqrt_done[lane]   = 1'b0; // unused lane
+      assign divsqrt_ready[lane]  = 1'b0; // unused lane
+      assign lane_ext_bit[lane]   = 1'b1; // NaN-box unused lane
+      assign local_result         = {(LANE_WIDTH){lane_ext_bit[0]}}; // sign-extend/nan box
       assign lane_status[lane]    = '0;
       assign lane_busy[lane]      = 1'b0;
     end
@@ -420,10 +429,17 @@ or set Features.FpFmtMask to support only FP32");
       assign fmt_slice_result[fmt][Width-1:NUM_LANES*FP_WIDTH] = '{default: lane_ext_bit[0]};
   end
 
-  // Mute int results if unused
-  for (genvar ifmt = 0; ifmt < NUM_INT_FORMATS; ifmt++) begin : int_results_disabled
+  for (genvar ifmt = 0; ifmt < NUM_INT_FORMATS; ifmt++) begin : extend_or_mute_int_result
+    // Mute int results if unused
     if (OpGroup != fpnew_pkg::CONV) begin : mute_int_result
       assign ifmt_slice_result[ifmt] = '0;
+
+    // Extend slice result if needed
+    end else begin : extend_int_result
+      // Set up some constants
+      localparam int unsigned INT_WIDTH = fpnew_pkg::int_width(fpnew_pkg::int_format_e'(ifmt));
+      if (NUM_LANES*INT_WIDTH < Width)
+        assign ifmt_slice_result[ifmt][Width-1:NUM_LANES*INT_WIDTH] = '0;
     end
   end
 
@@ -465,6 +481,7 @@ or set Features.FpFmtMask to support only FP32");
     assign {result_vec_op, result_is_cpk} = byp_pipe_aux_q[NumPipeRegs];
   end else begin : no_conv
     assign {result_vec_op, result_is_cpk} = '0;
+    assign conv_target_q = '0;
   end
 
   if (PulpDivsqrt) begin