Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[prim,rtl] Rewrite a case statement to avoid unreachable case #26072

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[prim,rtl] Rewrite a case statement to avoid unreachable case #26072

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3dc603
[prim,rtl] Rewrite a case as if/else to avoid unreachable case
rswarbrick Jan 30, 2025
f47aaa8
[prim,rtl] Get rid of a trivial combinatorial variable
rswarbrick Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 27 additions & 44 deletions hw/ip/prim/rtl/prim_reg_cdc_arb.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,6 @@ module prim_reg_cdc_arb #(
SelHwReq
} req_sel_e;

typedef enum logic {
StIdle,
StWait
} state_e;


// Only honor the incoming destination update request if the incoming
// value is actually different from what is already completed in the
Expand All @@ -106,16 +101,15 @@ module prim_reg_cdc_arb #(
logic dst_update_ack;
req_sel_e id_q;

state_e state_q, state_d;
logic idle_q, idle_d;
always_ff @(posedge clk_dst_i or negedge rst_dst_ni) begin
if (!rst_dst_ni) begin
state_q <= StIdle;
idle_q <= 1'b1;
end else begin
state_q <= state_d;
idle_q <= idle_d;
end
end

logic busy;
logic dst_req_q, dst_req;
always_ff @(posedge clk_dst_i or negedge rst_dst_ni) begin
if (!rst_dst_ni) begin
Expand All @@ -126,15 +120,15 @@ module prim_reg_cdc_arb #(
// dst_lat_d is safe to used here because dst_req_q, if set,
// always has priority over other hardware based events.
dst_req_q <= '0;
end else if (dst_req_i && busy) begin
end else if (dst_req_i && !idle_q) begin
// if destination request arrives when a handshake event
// is already ongoing, hold on to request and send later
dst_req_q <= 1'b1;
end
end

// dst_req_q will be 0 when dst_req_i is set, this assertion checks the conditional branch
// (dst_req_i && !dst_req_q && busy) can be simplified to avoid conditional coverage
// (dst_req_i && !dst_req_q && !idle_q) can be simplified to avoid conditional coverage
// holes
`ASSERT(Not_Dst_req_q_while_dst_req_i_A, dst_req_i |-> !dst_req_q,
clk_dst_i, !rst_dst_ni)
Expand Down Expand Up @@ -177,7 +171,7 @@ module prim_reg_cdc_arb #(

// if a destination update is received when the system is idle and there is no
// software side request, hw update must be selected.
`ASSERT(DstUpdateReqCheck_A, ##1 dst_update & !dst_req & !busy |=> id_q == SelHwReq,
`ASSERT(DstUpdateReqCheck_A, ##1 dst_update & !dst_req & idle_q |=> id_q == SelHwReq,
clk_dst_i, !rst_dst_ni)

// if hw select was chosen, then it must be the case there was a destination update
Expand All @@ -190,49 +184,38 @@ module prim_reg_cdc_arb #(

// send out prim_subreg request only when proceeding
// with software request
assign dst_req_o = ~busy & dst_req;
assign dst_req_o = idle_q & dst_req;

logic dst_hold_req;
always_comb begin
state_d = state_q;
idle_d = idle_q;
dst_hold_req = '0;

// depending on when the request is received, we
// may latch d or q.
dst_lat_q = '0;
dst_lat_d = '0;

busy = 1'b1;

unique case (state_q)
StIdle: begin
busy = '0;
if (dst_req) begin
// there's a software issued request for change
state_d = StWait;
dst_lat_d = 1'b1;
end else if (dst_update) begin
state_d = StWait;
dst_lat_d = 1'b1;
end else if (dst_qs_o != dst_qs_i) begin
// there's a direct destination update
// that was blocked by an ongoing transaction
state_d = StWait;
dst_lat_q = 1'b1;
end
end

StWait: begin
dst_hold_req = 1'b1;
if (dst_update_ack) begin
state_d = StIdle;
end
if (idle_q) begin
if (dst_req) begin
// there's a software issued request for change
idle_d = 1'b0;
dst_lat_d = 1'b1;
end else if (dst_update) begin
idle_d = 1'b0;
dst_lat_d = 1'b1;
end else if (dst_qs_o != dst_qs_i) begin
// there's a direct destination update
// that was blocked by an ongoing transaction
idle_d = 1'b0;
dst_lat_q = 1'b1;
end

default: begin
state_d = StIdle;
end else begin
dst_hold_req = 1'b1;
if (dst_update_ack) begin
idle_d = 1'b1;
end
endcase // unique case (state_q)
end
end // always_comb

assign dst_update_req = dst_hold_req | dst_lat_d | dst_lat_q;
Expand Down Expand Up @@ -270,7 +253,7 @@ module prim_reg_cdc_arb #(
async_flag <= '0;
end else if (src_update_o) begin
async_flag <= '0;
end else if (dst_update && !dst_req_o && !busy) begin
end else if (dst_update && !dst_req_o && idle_q) begin
async_flag <= 1'b1;
end
end
Expand Down
Loading