hw: Fix reduction routing logic and add comments to document code

Author: Lore0599

hw/floo_nw_chimney.sv

@@ -236,11 +236,8 @@ module floo_nw_chimney #(
   // Routing
   dst_t [NumNWAxiChannels-1:0] dst_id;
   dst_t narrow_aw_id_q, wide_aw_id_q;
-  id_t [NumNWAxiChannels-1:0] mask;
   id_t narrow_aw_mask_q, wide_aw_mask_q;
-  route_t [NumNWAxiChannels-1:0] route_out;
   id_t [NumNWAxiChannels-1:0] mcast_mask;
-  id_t axi_aw_mask_q;
   id_t [NumNWAxiChannels-1:0] id_out;
   id_t [NumNWAxiChannels-1:0] mask_id;
 
@@ -885,9 +882,9 @@ module floo_nw_chimney #(
     assign mcast_mask[WideR]   = wide_ar_buf_hdr_out.hdr.mask;
     assign mcast_mask[WideB]   = wide_aw_buf_hdr_out.hdr.mask;
 
-    `FFL(narrow_aw_mask_q, mask[NarrowAw], axi_narrow_aw_queue_valid_out &&
+    `FFL(narrow_aw_mask_q, mcast_mask[NarrowAw], axi_narrow_aw_queue_valid_out &&
                                            axi_narrow_aw_queue_ready_in, '0)
-    `FFL(wide_aw_mask_q, mask[WideAw], axi_wide_aw_queue_valid_out &&
+    `FFL(wide_aw_mask_q, mcast_mask[WideAw], axi_wide_aw_queue_valid_out &&
                                        axi_wide_aw_queue_ready_in, '0)
 
   end else begin: gen_no_mcast_mask

hw/floo_nw_router.sv

@@ -172,6 +172,8 @@ module floo_nw_router #(
     rsvd: '0
   });
 
+  // Enable reduction for the B response.
+  // Disable multicast for the B response.
   floo_router #(
     .NumInput         ( NumInputs               ),
     .NumOutput        ( NumOutputs              ),

hw/floo_output_arbiter.sv

@@ -4,6 +4,8 @@
 //
 // Author: Chen Wu <[email protected]>
 
+`include "common_cells/assertions.svh"
+
 module floo_output_arbiter import floo_pkg::*;
 #(
   /// Number of input ports
@@ -96,4 +98,7 @@ module floo_output_arbiter import floo_pkg::*;
 
   assign ready_o = (reduce_valid_out)? reduce_ready_out : unicast_ready_out;
 
+  // Cannot have an output valid without at least one input valid
+  `ASSERT(ValidOutInvalidIn, valid_o |-> |valid_i)
+
 endmodule

hw/floo_reduction_arbiter.sv

@@ -11,6 +11,8 @@ module floo_reduction_arbiter import floo_pkg::*;
   /// Type definitions
   parameter type         flit_t     = logic,
   parameter type         payload_t  = logic,
+  // Masks used to select which bits of the payload are part of the response,
+  // allowing extraction of relevant bits and detection of any participant errors.
   parameter payload_t    NarrowRspMask = '0,
   parameter payload_t    WideRspMask = '0,
   parameter type         id_t       = logic
@@ -67,7 +69,8 @@ module floo_reduction_arbiter import floo_pkg::*;
     // We check every input port from which we expect a response
     for (int i = 0; i < NumRoutes; i++) begin
       if(in_route_mask[i]) begin
-        // For every bit that is set in the mask, we assemble the response
+        // Select only the bits of the payload that are part of the response
+        // and check if at least one of the participants sent an error.
         automatic int j = 0;
         for (int k = 0; k < $bits(ReduceMask); k++) begin
           if (ReduceMask[k]) begin
@@ -76,6 +79,7 @@ module floo_reduction_arbiter import floo_pkg::*;
           end
         end
         // If one of the responses is an error, we return an error
+        // otherwise we return the first response
         if(resp == axi_pkg::RESP_SLVERR) begin
           data_o = data_i[i];
           break;

hw/floo_reduction_sync.sv

@@ -22,8 +22,10 @@ module floo_reduction_sync import floo_pkg::*;
 );
 
   logic [NumRoutes-1:0]  compare_same, same_and_valid;
+  logic all_reduction_srcs_valid;
 
-  // Compute the mask of expected input ports to reduce from
+  // Compute the input mask based on the selected input port's destination and mask fields.
+  // This determines which input ports are expected to participate in the reduction.
   floo_route_xymask #(
     .NumRoutes ( NumRoutes ),
     .flit_t    ( flit_t    ),
@@ -39,11 +41,18 @@ module floo_reduction_sync import floo_pkg::*;
     // Compare whether the `mask` and `dst_id` are equal to the selected input port
     assign compare_same[in] = ((data_i[in].hdr.mask == data_i[sel_i].hdr.mask) &&
                                (data_i[in].hdr.dst_id == data_i[sel_i].hdr.dst_id));
-    // TODO(fischeti): Check with Chen what is done here
+
+    // Determine if this input should be considered valid for the reduction:
+    // If we are at the dst node and the port is the local one, we don’t wait for a
+    // response/reduction since it will stay locally [NoLoopBack].
     assign same_and_valid[in] = (data_i[sel_i].hdr.dst_id == xy_id_i && in == Eject) ||
                                 (compare_same[in] & valid_i[in]);
   end
 
-  assign valid_o = (in_route_mask_o == '0)? 1'b0 : &(same_and_valid | ~in_route_mask_o);
+  // Reduction is valid only if all expected inputs [in_route_mask_o] are valid.
+  // Inputs not involved in the reduction are ignored [~(in_route_mask_o)].
+  assign all_reduction_srcs_valid = &(same_and_valid | ~in_route_mask_o);
 
+  // To have a valid output at least one input must be valid.
+  assign valid_o = (in_route_mask_o == '0)? 1'b0 : (|valid_i & all_reduction_srcs_valid);
 endmodule

hw/floo_router.sv

@@ -161,6 +161,7 @@ module floo_router
         assign in_ready[in][v] = |(masked_all_ready[in][v] & route_mask[in][v]);
       end else begin : gen_multicast
         // TODO(fischeti): Clarify with Chen
+        // Logic to fork the multicast packet into multiple ports
         assign acc_masked_ready_d[in][v] = (in_ready[in][v]) ? '0 :
                                             (acc_masked_ready_q[in][v] | masked_all_ready[in][v]);
         assign current_accumulated[in][v] = acc_masked_ready_q[in][v] | masked_all_ready[in][v];
@@ -197,6 +198,8 @@ module floo_router
           .data_o  ( out_data [out][v] )
         );
       end else begin : gen_red_arb
+        // Arbiter to be instantiated for reduction operations.
+        // Repsonses from a multicast request are also treated as reductions.
         floo_output_arbiter #(
           .NumRoutes     ( NumInput      ),
           .flit_t        ( flit_t        ),

hw/floo_wormhole_arbiter.sv

@@ -5,6 +5,7 @@
 // Michael Rogenmoser <[email protected]>
 
 `include "common_cells/registers.svh"
+`include "common_cells/assertions.svh"
 
 /// A wormhole arbiter
 module floo_wormhole_arbiter import floo_pkg::*;
@@ -75,4 +76,6 @@ module floo_wormhole_arbiter import floo_pkg::*;
   `FF(valid_q, valid_d, '0)
   `FF(last_q, last_out & ready_i, '0)
 
+  `ASSERT(InvalidCreation, valid_o |-> |valid_i)
+
 endmodule