Combinational loops in synthesis (cv32e40px) when adding co-processor through CV-X-IF

[gonzo-sal]

Combinational loops in synthesis (cv32e40px)

I have designed a co-processor wrapper based on the CV-X-IF standard (v0.2.0). The idea is to use it as a quick template to implement custom instructions easily.
Simulation works, but synthesis still gives combinational loops and the bitstream is not generated for the cv32e40px core.
Simulation and synthesis works for cv32e40x core.

IMP: latest commit with x illegal fix is already applied.

Testing and findings:
My issue_ready signal responds in a combinational manner based on some signals from the CPU, as requested by the cv-x-if standard (v0.2.0). One of these signals is rs_valid: if any of the registers I need is still not valid according to rs_valid then issue_ready goes low, waiting for the register to be valid.
This behaviour is shown below on a custom accumulation example: second instruction has to wait the previous one to writeback the result in the X registers before continuing with the accumulation (accumulation register is in the CPU).

After several tests I have encountered If I remove the rs_valid dependency from my issue_ready signal, then synthesis works. However I miss and important functionality: either I take the risks of executing an instruction with wrong/old data by not checking the rs_valid signal or I check rs-valid and simply reject the instruction due to unavailable data if necessary (instead of putting the issue_ready signal low), but then an illegal instruction happens and program breaks.

Note: find below a code snippet from the fpu_ss repo. Driving issue_ready according to rs_valid seems to be the correct behaviour (apart from the interpretation of the cv-x-if standard).

  // Issue Interface
  // ---------------
  always_comb begin
    x_issue_ready_o = 1'b0;
    if (((prd_rsp_use_rs_i[0] & x_issue_req_rs_valid_i[0]) | !prd_rsp_use_rs_i[0])
      & ((prd_rsp_use_rs_i[1] & x_issue_req_rs_valid_i[1]) | !prd_rsp_use_rs_i[1])
      & ((prd_rsp_use_rs_i[2] & x_issue_req_rs_valid_i[2]) | !prd_rsp_use_rs_i[2])
      & in_buf_push_ready_i) begin
      x_issue_ready_o = 1'b1;
    end
  end

Component

Component:RTL: For issues in the RTL (e.g. for files in the rtl directory)

Steps to Reproduce

Please provide:

1. git hash: 15b9dd6
2. Command line: make vivado-fpga FPGA_BOARD=pynq-z2
3. Logfile and/or wave-dump info (screen shots can be useful): check above

[MikeOpenHWGroup]

Hi @gonzo-sal. What is the cv32e40px?

[pascalgouedo]

This is the wrong project.

Better to add an issue here https://github.com/esl-epfl/cv32e40px/issues or future openhwgroup cv32e40px project.

[pascalgouedo]

Hi @gonzo-sal. What is the cv32e40px?

https://github.com/openhwgroup/programs/blob/master/Project-Descriptions-and-Plans/CV32E40PX/Project_Concept_for_CV32E40PX_Nov-27_2023.md

[MikeOpenHWGroup]

Hi @gonzo-sal. What is the cv32e40px?

https://github.com/openhwgroup/programs/blob/master/Project-Descriptions-and-Plans/CV32E40PX/Project_Concept_for_CV32E40PX_Nov-27_2023.md

Yes, I know about this, but that is a Project Concept - there is not yet a formal OpenHW Group CV332E40PX because we have not yet passed Project Launch. I was unaware that EPFL had created one.

[pascalgouedo]

I think they will go to Project Launch at some time as this is needed for Tristan.
But meanwhile they already started to work so I think Davide created this new repo which is a fork of a fork of openhwgroup/cv32e40p repo...
And I think CEA is working using that epfl fork as a starting point.

[gonzo-sal]

I think I can shed some light on the problem.

When I click on "bug" here, as @pascalgouedo suggested:

I am redirected here:

Maybe due to cv32e40px being a fork of a fork of cv32e40p?
Let me know how to proceed and thanks in advance.

[pascalgouedo]

Hi @gonzo-sal

You're right it is because templates config are redirecting to cv32e40p repo.

If you directly go using this address you stay in cv32e40px.
You can replace bug by task, question or enhancement if needed.

@davideschiavone Ultimately maybe better to update esl-epfl/cv32e40px/.github/ISSUE_TEMPLATE/config.yml to point to cv32e40px...

[davideschiavone]

now it is done, thx @pascalgouedo and @gonzo-sal

[gonzo-sal]

Thanks all, moving issue to the correct address.

Sorry for any inconvenience caused.