VLSU Fix

hw/ip/spatz/src/spatz_doublebw_vlsu.sv

@@ -666,6 +666,23 @@ module spatz_doublebw_vlsu
 
   logic vrf_commit_bypass;
 
+  /////////////////////////
+  //  Coalescing Buffer  //
+  /////////////////////////
+
+  // Per-interface coalescing buffers: accumulate partial wbe writes before
+  // committing to the VRF, preventing premature chaining on partial VRF words.
+  vrf_req_t [NrInterfaces-1:0] coalesce_d, coalesce_q;
+  logic     [NrInterfaces-1:0] coalesce_valid_d, coalesce_valid_q;
+  logic     [NrInterfaces-1:0] coalesce_commit;
+
+  // Both interfaces must be ready before either commits (normal path). This
+  // prevents the faster interface from draining its buffer while the slower
+  // one is still accumulating, which would leave the slower interface with
+  // rsp_valid data it can never flush (no &coalesce_valid_q, no vrf_wvalid_i[0]).
+  logic both_commit;
+  assign both_commit = coalesce_commit[0] & coalesce_commit[1];
+
   for (genvar intf = 0; intf < NrInterfaces; intf++) begin : gen_vrf_req_register_intf
     spill_register #(
       .T(vrf_req_t)
@@ -680,9 +697,16 @@ module spatz_doublebw_vlsu
       .ready_i(vrf_req_ready_q[intf])
     );
 
-    assign vrf_waddr_o[intf]     = vrf_req_q[intf].waddr;
-    assign vrf_wdata_o[intf]     = vrf_req_q[intf].wdata;
-    assign vrf_wbe_o[intf]       = vrf_req_q[intf].wbe;
+    `FF(coalesce_q[intf], coalesce_d[intf], '0)
+    `FF(coalesce_valid_q[intf], coalesce_valid_d[intf], '0)
+
+    // Commit the coalescing buffer when the VRF word is fully assembled or
+    // this is the last write of the instruction.
+    assign coalesce_commit[intf] = coalesce_valid_q[intf] && (&coalesce_q[intf].wbe || coalesce_q[intf].rsp_valid);
+
+    assign vrf_waddr_o[intf] = coalesce_q[intf].waddr;
+    assign vrf_wdata_o[intf] = coalesce_q[intf].wdata;
+    assign vrf_wbe_o[intf]   = coalesce_q[intf].wbe;
     // Ensure simpler synchronization for commits from both interfaces
     // Writeback:
     // For interface 1, check if interface 0 commit can go through
@@ -691,14 +715,55 @@ module spatz_doublebw_vlsu
     // If Interface 1, is resp interface (usually the default)
     // If Interface 0, is resp interface (if interface 0 has more vector elements), then ensure interface 1 has nothing in buffer
     // to avoid retiring before interface 1 commits to the VRF
-    assign vrf_we_o[intf]        = ((&vrf_req_valid_q) | ((intf==0) ? vrf_req_valid_q[0] & (vrf_commit_bypass | vrf_commit_waiting_q[1]) & vlsu_buf_empty_i : 1'b0)) &
-                                   ((intf==1) ? (vrf_wvalid_i[0] & (vrf_req_q[1].rsp.id == vrf_req_q[0].rsp.id)) : 1'b1) &
-                                   !vlsu_buf_full_i;
-    assign vrf_id_o[intf]        = {vrf_req_q[intf].rsp.id, mem_spatz_req.id, commit_insn_q.id};
-    assign vrf_req_ready_q[intf] = vrf_wvalid_i[intf];
+    // Normal path: both coalescing buffers must be commit-ready (both_commit)
+    // before either fires, so no interface drains ahead of the other.
+    // Bypass path (intf 0 only): interface 1 has nothing to write (small vl).
+    assign vrf_we_o[intf] = (both_commit |
+                             ((intf==0) ? coalesce_commit[0] & (vrf_commit_bypass | vrf_commit_waiting_q[1]) & vlsu_buf_empty_i : 1'b0)) &
+                            ((intf==1) ? (vrf_wvalid_i[0] & (coalesce_q[1].rsp.id == coalesce_q[0].rsp.id)) : 1'b1) &
+                            !vlsu_buf_full_i;
+    assign vrf_id_o[intf] = {coalesce_q[intf].rsp.id, mem_spatz_req.id, commit_insn_q.id};
+    // The spill register may be popped when the coalescing buffer can accept:
+    //   - buffer is empty, OR
+    //   - buffer is still accumulating (not yet commit-ready), OR
+    //   - buffer is committing this cycle (VRF accepting, freeing a slot)
+    assign vrf_req_ready_q[intf] = !coalesce_valid_q[intf] || !coalesce_commit[intf] || vrf_wvalid_i[intf];
 
     `FF(vrf_commit_intf_valid_q[intf], vrf_commit_intf_valid[intf], 1'b0)
     `FF(vrf_commit_waiting_q[intf], vrf_commit_waiting_d[intf], 1'b0)
+
+    always_comb begin : coalesce_proc
+      coalesce_d[intf]       = coalesce_q[intf];
+      coalesce_valid_d[intf] = coalesce_valid_q[intf];
+
+      // When the VRF accepts the coalesced write, clear the buffer.
+      if (coalesce_commit[intf] && vrf_wvalid_i[intf]) begin
+        coalesce_d[intf]       = '0;
+        coalesce_valid_d[intf] = 1'b0;
+      end
+
+      // Accept a new partial write from the spill-register output.
+      if (vrf_req_valid_q[intf] && vrf_req_ready_q[intf]) begin
+        if (coalesce_valid_q[intf] && !(coalesce_commit[intf] && vrf_wvalid_i[intf])) begin
+          // Merge into the existing accumulation (same VRF word in progress):
+          // OR the byte enables and splice in only the newly-valid data bytes.
+          for (int unsigned i = 0; i < N_FU*ELENB; i++)
+            if (vrf_req_q[intf].wbe[i])
+              coalesce_d[intf].wdata[8*i +: 8] = vrf_req_q[intf].wdata[8*i +: 8];
+          coalesce_d[intf].wbe = coalesce_d[intf].wbe | vrf_req_q[intf].wbe;
+          // Carry the rsp metadata from the last write of the instruction.
+          if (vrf_req_q[intf].rsp_valid) begin
+            coalesce_d[intf].rsp       = vrf_req_q[intf].rsp;
+            coalesce_d[intf].rsp_valid = 1'b1;
+          end
+        end else begin
+          // Buffer was empty or just committed — start a fresh accumulation.
+          // Copy the full struct so no field (e.g. commit_vl) is left at 0.
+          coalesce_d[intf]       = vrf_req_q[intf];
+          coalesce_valid_d[intf] = 1'b1;
+        end
+      end
+    end : coalesce_proc
   end
 
   //////////////////////////////////////
@@ -711,11 +776,11 @@ module spatz_doublebw_vlsu
     vrf_commit_waiting_d = vrf_commit_waiting_q;
 
     // To track if the second interface is committing or not for small vector lengths
-    vrf_commit_bypass = vrf_req_valid_q[0] ? ((vrf_req_q[0].commit_vl <= ( SpatzMemBytes / 2)) ? 1'b1 : 1'b0) : 1'b0;
+    vrf_commit_bypass = coalesce_valid_q[0] ? ((coalesce_q[0].commit_vl <= (SpatzMemBytes / 2)) ? 1'b1 : 1'b0) : 1'b0;
 
     for (int intf = 0; intf < NrInterfaces; intf++) begin
-      // We have a final resp to write to VRF
-      vrf_valid_rsp[intf] = (vrf_req_valid_q[intf] & vrf_req_q[intf].rsp_valid);
+      // We have a final resp ready to commit to VRF (coalesced word is complete and is the last write)
+      vrf_valid_rsp[intf] = (coalesce_valid_q[intf] & coalesce_q[intf].rsp_valid & coalesce_commit[intf]);
 
       // Track if the final resp has already been written to the VRF
       vrf_commit_intf_valid[intf] = ((vrf_valid_rsp[intf] & vrf_wvalid_i[intf]) | vrf_commit_waiting_q[intf]) | (intf == 1 ? vrf_commit_bypass : 1'b0);
@@ -739,13 +804,13 @@ module spatz_doublebw_vlsu
 
   // Check is both interfaces has reached to a completion and if the last write to the VRF is also done
   // Assign the instruction id from the interface that completes the last
-  assign vlsu_rsp_o = &vrf_commit_intf_valid && |vrf_req_valid_q ? vrf_req_q[resp_intf].rsp   : '{id: commit_insn_q.id, default: '0};
+  assign vlsu_rsp_o = &vrf_commit_intf_valid && |coalesce_valid_q ? coalesce_q[resp_intf].rsp : '{id: commit_insn_q.id, default: '0};
 
   // Send response back to the controller to indicate end of request
   // Check if both the interfaces have completed request and have a valid response to send
   // Check if atleast one interface has a valid (interfaces can send responses asynchronously to the VRF)
-  // Set reponse high if one of the interfaces has a ready indicating the response has been written to the VRF
-  assign vlsu_rsp_valid_o = &vrf_commit_intf_valid && |vrf_req_valid_q ? |vrf_req_ready_q : vlsu_finished_req && !commit_insn_q.is_load;
+  // Set response high when both interfaces have committed and the VRF accepts the final write
+  assign vlsu_rsp_valid_o = &vrf_commit_intf_valid && |coalesce_valid_q ? |vrf_wvalid_i : vlsu_finished_req && !commit_insn_q.is_load;
 
   //////////////
   // Counters //

hw/ip/spatz/src/spatz_vlsu.sv

@@ -632,16 +632,74 @@ module spatz_vlsu
     .ready_i(vrf_req_ready_q)
   );
 
-  assign vrf_waddr_o     = vrf_req_q.waddr;
-  assign vrf_wdata_o     = vrf_req_q.wdata;
-  assign vrf_wbe_o       = vrf_req_q.wbe;
-  assign vrf_we_o        = vrf_req_valid_q;
-  assign vrf_id_o        = {vrf_req_q.rsp.id, mem_spatz_req.id, commit_insn_q.id};
-  assign vrf_req_ready_q = vrf_wvalid_i;
-
-  // Ack when the vector store finishes, or when the vector load commits to the VRF
-  assign vlsu_rsp_o       = vrf_req_q.rsp_valid && vrf_req_valid_q ? vrf_req_q.rsp   : '{id: commit_insn_q.id, default: '0};
-  assign vlsu_rsp_valid_o = vrf_req_q.rsp_valid && vrf_req_valid_q ? vrf_req_ready_q : vlsu_finished_req && !commit_insn_q.is_load;
+  /////////////////////////
+  //  Coalescing Buffer  //
+  /////////////////////////
+
+  // Coalesce the data write to VRF after it construct a full vector word
+  vrf_req_t coalesce_d, coalesce_q;
+  logic     coalesce_valid_d, coalesce_valid_q;
+
+  `FF(coalesce_q, coalesce_d, '0)
+  `FF(coalesce_valid_q, coalesce_valid_d, '0)
+
+  // The coalescing buffer is ready to commit when the accumulated byte-enable
+  // covers the full VRF word, or when this is the last write of the instruction.
+  logic coalesce_commit;
+  assign coalesce_commit = coalesce_valid_q && (&coalesce_q.wbe || coalesce_q.rsp_valid);
+
+  // Drive VRF outputs from the coalescing buffer, not the raw spill-register.
+  assign vrf_waddr_o = coalesce_q.waddr;
+  assign vrf_wdata_o = coalesce_q.wdata;
+  assign vrf_wbe_o   = coalesce_q.wbe;
+  assign vrf_we_o    = coalesce_commit;
+  assign vrf_id_o    = {coalesce_q.rsp.id, mem_spatz_req.id, commit_insn_q.id};
+
+  // The spill register may be popped when the coalescing buffer can accept:
+  //   - buffer is empty, OR
+  //   - buffer is still accumulating (not yet commit-ready), OR
+  //   - buffer is committing this cycle (VRF accepting, freeing a slot)
+  assign vrf_req_ready_q = !coalesce_valid_q || !coalesce_commit || vrf_wvalid_i;
+
+  // Ack when the vector store finishes, or when the load's coalesced write commits to the VRF
+  assign vlsu_rsp_o       = coalesce_q.rsp_valid && coalesce_commit ? coalesce_q.rsp : '{id: commit_insn_q.id, default: '0};
+  assign vlsu_rsp_valid_o = coalesce_q.rsp_valid && coalesce_commit ? vrf_wvalid_i   : vlsu_finished_req && !commit_insn_q.is_load;
+
+  always_comb begin : coalesce_proc
+    coalesce_d       = coalesce_q;
+    coalesce_valid_d = coalesce_valid_q;
+
+    // When the VRF accepts the coalesced write, clear the buffer.
+    if (coalesce_commit && vrf_wvalid_i) begin
+      coalesce_d       = '0;
+      coalesce_valid_d = 1'b0;
+    end
+
+    // Accept a new partial write from the spill-register output.
+    if (vrf_req_valid_q && vrf_req_ready_q) begin
+      if (coalesce_valid_q && !(coalesce_commit && vrf_wvalid_i)) begin
+        // Merge into the existing accumulation (same VRF word in progress):
+        // OR the byte enables and splice in only the newly-valid data bytes.
+        for (int unsigned i = 0; i < N_FU*ELENB; i++)
+          if (vrf_req_q.wbe[i])
+            coalesce_d.wdata[8*i +: 8] = vrf_req_q.wdata[8*i +: 8];
+        coalesce_d.wbe = coalesce_d.wbe | vrf_req_q.wbe;
+        // Carry the rsp metadata from the last write of the instruction.
+        if (vrf_req_q.rsp_valid) begin
+          coalesce_d.rsp       = vrf_req_q.rsp;
+          coalesce_d.rsp_valid = 1'b1;
+        end
+      end else begin
+        // Buffer was empty or just committed — start a fresh accumulation.
+        coalesce_d.waddr     = vrf_req_q.waddr;
+        coalesce_d.wdata     = vrf_req_q.wdata;
+        coalesce_d.wbe       = vrf_req_q.wbe;
+        coalesce_d.rsp       = vrf_req_q.rsp;
+        coalesce_d.rsp_valid = vrf_req_q.rsp_valid;
+        coalesce_valid_d     = 1'b1;
+      end
+    end
+  end : coalesce_proc
 
   //////////////
   // Counters //

sw/riscvTests/CMakeLists.txt

@@ -33,8 +33,9 @@ add_snitch_test(vadd  isa/rv64uv/vadd.c)
 add_snitch_test(vsub  isa/rv64uv/vsub.c)
 add_snitch_test(vrsub isa/rv64uv/vrsub.c)
 
-add_snitch_test(vls    isa/rv64uv/vls.c)
-add_snitch_test(vloxei isa/rv64uv/vloxei.c)
+add_snitch_test(vls       isa/rv64uv/vls.c)
+add_snitch_test(vloxei    isa/rv64uv/vloxei.c)
+add_snitch_test(vls_chain isa/rv64uv/vls_chain.c)
 add_snitch_test(vsuxei isa/rv64uv/vsuxei.c)
 
 add_snitch_test(vwadd  isa/rv64uv/vwadd.c)

sw/riscvTests/isa/rv64uv/vls_chain.c

@@ -0,0 +1,102 @@
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+// Author: Diyou Shen
+
+// Chaining correctness tests for strided and indexed EW_8/EW_16 loads.
+//
+// These instruction types cause the VLSU to issue partial VRF writes (wbe !=
+// all-ones) before the full VRF word is coherent. A dependent vector
+// instruction chained on the load destination must not read stale data.
+//
+// Each test case performs:
+//   1. A strided or indexed load into v1 (partial-wbe writes).
+//   2. An immediately chained vadd reading v1.
+//   3. VCMP on the vadd result.
+//
+// Without the VLSU coalescing buffer, the controller enables chaining after
+// the first partial write, so vadd reads a partially-written VRF word and
+// produces wrong results.
+
+#include "vector_macros.h"
+
+// EW_8 strided load chained with vadd.vv
+// stride=3 forces single-element commits; 8 elements span 2 VRF write cycles.
+void TEST_CASE1(void) {
+  volatile uint8_t INP1[] = {
+    0x9f, 0xe4, 0x19, 0x20, 0x8f, 0x2e, 0x05, 0xe0,
+    0xf9, 0xaa, 0x71, 0xf0, 0xc3, 0x94, 0xbb, 0xd3,
+    0x9f, 0xe4, 0x19, 0x20, 0x8f, 0x2e, 0x05, 0xe0,
+    0xf9, 0xaa, 0x71, 0xf0, 0xc3, 0x94, 0xbb, 0xd3};
+  uint64_t stride = 3;
+
+  // stride=3: offsets 0,3,6,9,12,15,18,21
+  // v1 = {0x9f, 0x20, 0x05, 0xaa, 0xc3, 0xd3, 0x19, 0x2e}
+  VSET(8, e8, m1);
+  VLOAD_8(v3, 1, 1, 1, 1, 1, 1, 1, 1);
+  asm volatile("vlse8.v v1, (%0), %1" ::"r"(INP1), "r"(stride));
+  asm volatile("vadd.vv v2, v1, v3");
+  VCMP_U8(1, v2, 0xa0, 0x21, 0x06, 0xab, 0xc4, 0xd4, 0x1a, 0x2f);
+}
+
+// EW_16 strided load chained with vadd.vv
+// stride=4 bytes (skips every other element); 8 elements span 2 VRF write cycles.
+void TEST_CASE2(void) {
+  volatile uint16_t INP1[] = {
+    0x9fe4, 0x1920, 0x8f2e, 0x05e0, 0xf9aa, 0x71f0, 0xc394, 0xbbd3,
+    0xa11a, 0x9384, 0xa716, 0x3840, 0x9999, 0x1348, 0xa9f3, 0x8cd1};
+  uint64_t stride = 4;
+
+  // stride=4 bytes: byte offsets 0,4,8,12,16,20,24,28
+  // v1 = {0x9fe4, 0x8f2e, 0xf9aa, 0xc394, 0xa11a, 0xa716, 0x9999, 0xa9f3}
+  VSET(8, e16, m1);
+  VLOAD_16(v3, 1, 1, 1, 1, 1, 1, 1, 1);
+  asm volatile("vlse16.v v1, (%0), %1" ::"r"(INP1), "r"(stride));
+  asm volatile("vadd.vv  v2, v1, v3");
+  VCMP_U16(2, v2, 0x9fe5, 0x8f2f, 0xf9ab, 0xc395, 0xa11b, 0xa717, 0x999a,
+           0xa9f4);
+}
+
+// EW_8 indexed load (vloxei8.v) chained with vadd.vv
+void TEST_CASE3(void) {
+  volatile uint8_t INP1[] = {
+    0xe0, 0xd3, 0x40, 0xd1, 0x84, 0x48, 0x89, 0x88,
+    0x88, 0xae, 0x08, 0x91, 0x02, 0x59, 0x11, 0x89};
+
+  // byte offsets {1,2,3,4,5,7,8,9}
+  // v1 = {0xd3, 0x40, 0xd1, 0x84, 0x48, 0x88, 0x88, 0xae}
+  VSET(8, e8, m1);
+  VLOAD_8(v2, 1, 2, 3, 4, 5, 7, 8, 9);
+  VLOAD_8(v3, 1, 1, 1, 1, 1, 1, 1, 1);
+  asm volatile("vloxei8.v v1, (%0), v2" ::"r"(INP1));
+  asm volatile("vadd.vv   v4, v1, v3");
+  VCMP_U8(3, v4, 0xd4, 0x41, 0xd2, 0x85, 0x49, 0x89, 0x89, 0xaf);
+}
+
+// EW_16 indexed load (vloxei16.v) chained with vadd.vv
+void TEST_CASE4(void) {
+  volatile uint16_t INP1[] = {
+    0x05e0, 0xbbd3, 0x3840, 0x8cd1, 0x9384, 0x7548, 0x3489, 0x9388,
+    0x8188, 0x11ae, 0x5808, 0x4891, 0x4902, 0x8759, 0x1111, 0x1989};
+
+  // byte offsets {2,4,6,8,10,12,14,16}
+  // v1 = {0xbbd3, 0x3840, 0x8cd1, 0x9384, 0x7548, 0x3489, 0x9388, 0x8188}
+  VSET(8, e16, m1);
+  VLOAD_16(v2, 2, 4, 6, 8, 10, 12, 14, 16);
+  VLOAD_16(v3, 1, 1, 1, 1, 1, 1, 1, 1);
+  asm volatile("vloxei16.v v1, (%0), v2" ::"r"(INP1));
+  asm volatile("vadd.vv    v4, v1, v3");
+  VCMP_U16(4, v4, 0xbbd4, 0x3841, 0x8cd2, 0x9385, 0x7549, 0x348a, 0x9389,
+           0x8189);
+}
+
+int main(void) {
+  INIT_CHECK();
+  enable_vec();
+
+  TEST_CASE1();
+  TEST_CASE2();
+  TEST_CASE3();
+  TEST_CASE4();
+
+  EXIT_CHECK();
+}