Rgiunti/fix indexed instr

[rgiunti]

Problem

In spatz_vlsu.sv, the data_index_width_diff signal express the relationship between data and index width when performing an indexed load or store instruction:

logic [1:0] data_index_width_diff;
data_index_width_diff = int'(mem_spatz_req.vtype.vsew) - int'(mem_spatz_req.op_mem.ew);

and it has a key role for the computation of the address offset:

// Index
unique case (mem_spatz_req.op_mem.ew)
EW_8 : offset = $signed(vrf_rdata_i[1][8 * word_index +: 8]);
EW_16: offset = $signed(vrf_rdata_i[1][8 * word_index +: 16]);
default: offset = $signed(vrf_rdata_i[1][8 * word_index +: 32]);
endcase

since word_index is defined as:

word_index = (port << (MAXEW - data_index_width_diff)) + (maxew_t'(idx_offset << data_index_width_diff) >> data_index_width_diff) + (maxew_t'(idx_offset >> (MAXEW - data_index_width_diff)) << (MAXEW - data_index_width_diff)) * NrMemPorts;

This approach works only when the width of the data represented by mem_spatz_req.vtype.vsew is greater than or equal to mem_spatz_req.op_mem.ew which express the width of the indexes. Both are of type:

typedef enum logic [2:0] {
    EW_8  = 3'b000,
    EW_16 = 3'b001,
    EW_32 = 3'b010,
    EW_64 = 3'b011
} vew_e;

This means that the whole address generation fails for a test like the following one reported in vloxei.c in which this condition is not respected:

// EEW Destination < EEW indexes
void TEST_CASE3(void) {
  volatile uint8_t ALIGNED_I8[] = {0xe0, 0xd3, 0x40, 0xd1, 0x84, 0x48, 0x89, 0x88,
    0x88, 0xae, 0x08, 0x91, 0x02, 0x59, 0x11, 0x89};

  VSET(2, e8, m1);
  VLOAD_16(v2, 1, 15);
  asm volatile("vloxei16.v v1, (%0), v2" ::"r"(&ALIGNED_I8[0]));
  VCMP_U8(7, v1, 0xd3, 0x89);

  volatile uint16_t ALIGNED_I16[] = {
        0x05e0, 0xbbd3, 0x3840, 0x8cd1, 0x9384, 0x7548, 0x3489, 0x9388,
        0x8188, 0x11ae, 0x5808, 0x4891, 0x4902, 0x8759, 0x1111, 0x1989};   

  VSET(2, e16, m1);
  VLOAD_32(v2, 2, 30);
  asm volatile("vloxei32.v v1, (%0), v2" ::"r"(&ALIGNED_I16[0]));
  VCMP_U16(8, v1, 0xbbd3, 0x1989);
}

This happens because data_index_width_diff is defined as unsigned so, following the TEST_CASE3 example, we get:

data_index_width_diff = EW_8 - EW_16 = int'(3'b000) - int'(3'b001)`= 2'b11

which of course corrupts all the following computations for the address generation.

Proposed solution

In order to solve this issue, the proposed solution consists in redefining data_index_width_diff signal as a signed number:

logic signed [2:0] data_index_width_diff;

in this way the word_index signal computation must be reinterpreted depending on the sign of data_index_width_diff:

if (data_index_width_diff >= 0) begin
  word_index = (port << (MAXEW - data_index_width_diff)) + (maxew_t'(idx_offset << data_index_width_diff) >> data_index_width_diff) + (maxew_t'(idx_offset >> (MAXEW - data_index_width_diff)) << (MAXEW - data_index_width_diff)) * NrMemPorts;
end else begin
  word_index = (port >> (MAXEW + data_index_width_diff)) + (maxew_t'(idx_offset >> -data_index_width_diff) << -data_index_width_diff) + (maxew_t'(idx_offset << (MAXEW + data_index_width_diff)) >> (MAXEW + data_index_width_diff)) * NrMemPorts;
end

In this way, when data_index_width_diff < 0, since it is impossible to make a shift of a negative quantity, all the shift operations of the original word_index computation have been reversed as well as the sign of data_index_width_diff to maintain mathematical coherence with the original operation.

Results

Test cases in vloxei.c and vsuxei.c have been used in order to observe the failures due to the bug before the modifications and verify the correctness of the results after them.

[Navaneeth-KunhiPurayil]

@rgiunti the PR looks good to me. Do we need to also push the .do file ?

[rgiunti]

Hi @Navaneeth-KunhiPurayil, thank you for your review. I've rebased the PR and removed the .do file

[rgiunti]

Hi @Navaneeth-KunhiPurayil, @anga93. Now it seems that the CI is failing for some reason but I can't access the GitLab ETH pipeline with my credentials to see where is the error. Can you help me to understand which is the problem or to provide me access to the pipeline if it's possible?

[Navaneeth-KunhiPurayil]

@rgiunti I am currently looking into it as well, but basically there is an issue in the 32b config of Spatz where these indexed memory operations fail.

[Navaneeth-KunhiPurayil]

Fixed the issue in this branch - fix with CI passing.