openhwgroup / core-v-xif

RISC-V eXtension interface that provides a generalized framework suitable to implement custom coprocessors and ISA extensions
https://docs.openhwgroup.org/projects/openhw-group-core-v-xif/en
Other
53 stars 23 forks source link

Make dualread and dualwrite easier to be enabled #143

Closed LuigiGiuffrida98 closed 1 month ago

LuigiGiuffrida98 commented 4 months ago

Context

Dualread requires to expose on the C-V-XIF register interface a number of operands that depends on the value of X_DUALREAD, while dualwrite requires to duplicate the dimension of data field of result interface.

Actual status

The parameters to be modified are many and not well specified, e.g. to enable dualread X_DUALREAD has to be set, then to actually use the feature, either the X_NUM_RS or the X_RFR_WIDTH parameters have to be modified. To make it easier to use these features could be more convenient to simply set X_DUALREAD and/or X_DUALWRITE parameters.

Proposed solution (dualread)

First proposed implementation (compliant with the current status of the documentation):

This approach permits to enable the dualread feature by setting X_DUALREAD to a value higher than 0 and the rs width is modified internally


 localparam RS_WIDTH = (X_DUALREAD == 0) ? X_RFR_WIDTH : X_RFR_WIDTH*2);

  typedef struct packed {
    hartid_t hartid;  // Identification of the hart offloading the instruction
    id_t id;  // Identification of the offloaded instruction
    /* verilator lint_off UNPACKED */
    logic [RS_WIDTH-1:0] rs[X_NUM_RS-1:0];  // Register file source operands for the offloaded instruction. If  X_DUALREAD is different from 0, the rs in register interface is increase by the X_DUALREAD value.
    readregflags_t rs_valid; // Validity of the register file source operand(s).
  } x_register_t;

Second proposed implementation:

This approach permits to enable the dualread functionality by editing only the X_DUALREAD parameter. The number of elements of rs is modified accordingly to the value of X_DUALREAD.

  typedef struct packed {
    hartid_t hartid;  // Identification of the hart offloading the instruction
    id_t id;  // Identification of the offloaded instruction
    /* verilator lint_off UNPACKED */
    logic [X_RFR_WIDTH-1:0] rs[X_NUM_RS+X_DUALREAD-1:0];  // Register file source operands for the offloaded instruction. If  X_DUALREAD is different from 0, the rs in register interface is increase by the X_DUALREAD value.
    readregflags_t rs_valid; // Validity of the register file source operand(s).
  } x_register_t;

The difference between the two implementations is basically that the former requires to set to 0 the unused bits of the elements of rs if the dualread feature is not enabled on all the rs, the latter adds to rs the required elements according to the number of operands that admit the dualread feature. The main drawback of the second proposed implementation is that the meaning of elements added to rs has to be defined and specified. An example of how the extra registers can be mapped is:

rs1     -> rs[0]
rs2     -> rs[1]
rs3     -> rs[2]
rs1 + 1 -> rs[3]
rs2 + 1 -> rs[4]
rs3 + 1 -> rs[5]

Proposed solution (dualwrite)

TO-DO

christian-herber-nxp commented 3 months ago

@Luigi2898 are you intending to work on this? Otherwise I would suggest to close the PR.

LuigiGiuffrida98 commented 3 months ago

@christian-herber-nxp yes, I had to stop working for few weeks, but in next days I will return active on this.

LuigiGiuffrida98 commented 3 months ago

@christian-herber-nxp I update the code of the reference implementation, in my opinion, this updated implementation restores the original meaning of X_DUALREAD parameter. I made some changes in the docs too, can you please check them and let me know if I need to update the docs to explicitly state how to enable DUALREAD/DUALWRITE.

christian-herber-nxp commented 1 month ago

Stale and incomplete. Closing PR. Feel free to reopen once its active and best-case complete.