vproc / vicuna

RISC-V Zve32x Vector Coprocessor
Other
162 stars 47 forks source link

Asynchronous reset sensitivity list issue #3

Closed stevobailey closed 3 years ago

stevobailey commented 3 years ago

In several places, the sensitivity list for reset fails linting. For example, in proc_lsu.sv, line 413:

always_ff @(posedge clk_i or negedge async_rst_n) begin : vproc_lsu_stage_vreg
                if (~async_rst_n | (~ASYNC_RESET & ~rst_ni)) begin
                    state_vreg_q <= '{busy: 1'b0, default: 'x};
                end

Having async_rst_n with other signals in the sensitivity list is the problem. I suggest you find a linting tool. Verilator has one, though I'm not sure if it catches this.

This page shows you how to fix the issue: https://www.intel.com/content/www/us/en/programmable/quartushelp/13.0/mergedProjects/msgs/msgs/evrfx_veri_if_condition_does_not_match_sensitivity_list_edge.htm

michael-platzer commented 3 years ago

Thanks for pointing this out, I wasn't aware that this is an issue in Quartus as it synthesized without problems using Vivado.

I have restructured all blocks involving reset signals as shown on the page you linked to, and generally tried to improve handling of asynchronous and synchronous resets.

Please let me know if this fixes the issue.

Also, I am experimenting with verible as a linting tool, though unfortunately it does not catch this issue either.

stevobailey commented 3 years ago

Yeah not sure if this is a Verilog standard or just good practice / necessary for certain synthesis tools. So I'm not sure what linters would catch this. But I don't see it anymore as of commit bbfd0d8. Thanks!