lowRISC / lowRISC/opentitan

prim_reg_cdc_arb unreachable conditional coverage

Open
#26,379 0 comments 0 reactions 1 assignee Claimed by @antmarzam View on GitHub
IP:prim
Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

[Prim_reg_cdc_arb has an always statement](https://github.com/lowRISC/opentitan/blob/0449b4b3387c7ba647d07a9445233f5c10813dfe/hw/ip/prim/rtl/prim_reg_cdc_arb.sv#L155-L167) which doesn't allow to hit conditional coverage.

```
always_ff @(posedge clk_dst_i or negedge rst_dst_ni) begin
if (!rst_dst_ni) begin
id_q <= SelSwReq;
end else if (dst_update_ack) begin
id_q <= SelSwReq;
end else if (dst_req && dst_lat_d) begin
id_q <= SelSwReq;
end else if (!dst_req && dst_lat_d) begin
id_q <= SelHwReq;
end else if (dst_lat_q) begin
id_q <= SelHwReq;
end
end
```

There is unreachable conditional coverage in the third `else if` for the condition "1 && 1" (or `!dst_req == 0 && dst_lat_d== 1`).
This can't occur because the second `else if` covers this scenario.

The proposal is to re-write the RTL to avoid waiving conditional coverage in multiple instances so it would look like:

```
always_ff @(posedge clk_dst_i or negedge rst_dst_ni) begin
if (!rst_dst_ni) begin
id_q <= SelSwReq;
end else if (dst_update_ack) begin
id_q <= SelSwReq;
end else if (dst_lat_d) begin
id_q <= dst_req ? SelSwReq : SelHwReq;
end else if (!dst_req && dst_lat_d) begin
id_q <= SelHwReq;
end else if (dst_lat_q) begin
id_q <= SelHwReq;
end
end
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.