pulp-platform/common_cells

`id_queue`: Add support for multidimensional arrays

niwis opened this issue · 1 comments

niwis commented

As noted in #138, id_queue currently does not support multidimensional arrays:

On line 369, we loop over all bits in data_t, which only works for 1D vectors or packed structs

There are several options to address this:

  • Add an assertion or comment to disallow multidimensional arrays as data_t. These would need to be flattened outside the module.
  • Use a width parameter instead of a type parameter for data_t (as suggested in #138)
  • Flatten within id_queue

Allow me to leave a comment that might help with what this issue discusses.

I encountered synthesis errors regarding exists_match_bits.
The synthesis tool interpreted that exists_match_bits is multiply assigned and forming combinational loops.
This is probably because the inner genvar loop means to clone a logic of assigning to single exists_match_bits, rather than an assignment to each bit of exists_match_bits.

Considering the code would intend, I worked for a resolution as follows.
The errors have been resolved.

     // Exists Lookup
     for (genvar i = 0; i < CAPACITY; i++) begin: gen_lookup
         data_t exists_match_bits;
-        for (genvar j = 0; j < $bits(data_t); j++) begin: gen_mask
-            always_comb begin
+        always_comb begin
+            for (int j = 0; j < $bits(data_t); j++) begin: gen_mask
                 if (linked_data_q[i].free) begin
                     exists_match_bits[j] = 1'b0;
                 end else begin
                    if (!exists_mask_i[j]) begin
                        exists_match_bits[j] = 1'b1;
                    end else begin
                        exists_match_bits[j] = (linked_data_q[i].data[j] == exists_data_i[j]);
                    end
                end
            end
        end
        assign exists_match[i] = (&exists_match_bits);
    end

I hope this helps.