unique case statements consistently triggers warnings/non-fatal errors during reset

[silabs-hfegran]

always_comb begin
  unique case ({count_up, count_down})
    2'b00 : begin
      next_cnt = cnt_q;
    end
    2'b01 : begin
      next_cnt = cnt_q - 1'b1;
    end
    2'b10 : begin
      next_cnt = cnt_q + 1'b1;
    end
    2'b11 : begin
      next_cnt = cnt_q;
    end
  endcase
end 

Code such as the above consistently triggers warnings/non-fatal error at the start of simulation due to no case item matching. This clutters logs and generates unnecessary warnings. It would be good if we could rephrase such statements to avoid said warnings, e.g. by using defaults, unique0 or any other appropriate construction.

Source: cv32e40x/rtl/cv32e40x_div.sv:117 cv32e40x/rtl/cv32e40x_load_store_unit.sv:496

[Silabs-ArjanB]

Hi @silabs-hfegran The use of 'unique' is on purpose and will guarantee us that we handled all cases. A violation here is only possible when the CPU has not been reset yet.

If you have a case where you get similar warnings/errors after reset, can you provide a waveform form that please?

[silabs-hfegran]

Thanks @Silabs-ArjanB, All cases I have seen appears to be related to reset (at t>0 with rst asserted or t = 0) - I will close this issue and open a similar one on core-v-verif to investigate possible solutions on the testbench side of things

[Silabs-ArjanB]

Re-open until the t>0 issue is understood. Might 'fix' the RTL just to get rid of these warnings

[silabs-hfegran]

For reference, metrics log:

[Silabs-ArjanB]

Hi @silabs-hfegran So it seems that reset only becomes asserted at 2 ns, which is why you get warnings at that time as well. Is that being done on purpose?

[Silabs-ArjanB]

I think it will be easiest if we simply remove the unique keyword in the two reported cases.

[silabs-hfegran]

Hi @silabs-hfegran So it seems that reset only becomes asserted at 2 ns, which is why you get warnings at that time as well. Is that being done on purpose?

By default, the environment is soft-constrained to not assert reset until 50 clock cycles after clock start.

I think it will be easiest if we simply remove the unique keyword in the two reported cases.

Agreed, if the initial purpose of the unique keyword was for synthesis optimization, we might wanna replace unique with unique0 to avoid inferring any new logic. From a simulation point of view, I am fine with both removal and using unique0 instead.

[silabs-oysteink]

@silabs-hfegran Is this still an issue? If not, could you please close it?

[silabs-hfegran]

still seeing the issue here

[silabs-oysteink]

This should be fixed by PR #452 on cv32e40s.

[Silabs-ArjanB]

@silabs-hfegran Can you please close this if indeed fixed?

[silabs-hfegran]

Appears to be fixed now, thanks!