Hi, thanks for the great work on SERV!
While reviewing rtl/serv_ctrl.v, I noticed that the registers pc_plus_offset_cy_r and pc_plus_4_cy_r are used in carry feedback paths without being explicitly reset. This may result in undefined behavior when their values are read before being initialized.
Relevant Code Snippets:
For pc_plus_offset_cy_r:
pc_plus_offset_cy_r <= i_pc_en & pc_plus_offset_cy;
assign {pc_plus_offset_cy, pc_plus_offset} = offset_a + offset_b + pc_plus_offset_cy_r_w;
assign pc_plus_offset_cy_r_w[0] = pc_plus_offset_cy_r;
For pc_plus_4_cy_r:
pc_plus_4_cy_r <= i_pc_en & pc_plus_4_cy;
assign {pc_plus_4_cy, pc_plus_4} = pc + plus_4 + pc_plus_4_cy_r_w;
assign pc_plus_4_cy_r_w[0] = pc_plus_4_cy_r;
Both registers are used in self-referential expressions via the *_cy_r_w wires, and their initial values influence the first cycle of computation.
If they are not explicitly reset, their startup values can be undefined (X), potentially causing unpredictable results.
Suggestion
Add reset logic to assign a known initial value (1'b0) to both registers. For example:
if (i_rst) begin
pc_plus_offset_cy_r <= 1'b0;
pc_plus_4_cy_r <= 1'b0;
end else if (i_pc_en) begin
pc_plus_offset_cy_r <= pc_plus_offset_cy;
pc_plus_4_cy_r <= pc_plus_4_cy;
end
For RESET_STRATEGY == "NONE", you might consider using an initial block or extending the always @(posedge clk) block accordingly.
Why it Matters:
-
Prevents undefined/X propagation during startup
-
Ensures predictable and deterministic simulation results
-
Aligns with standard RTL design practice of initializing all stateful elements
Hi, thanks for the great work on SERV!
While reviewing
rtl/serv_ctrl.v, I noticed that the registerspc_plus_offset_cy_randpc_plus_4_cy_rare used in carry feedback paths without being explicitly reset. This may result in undefined behavior when their values are read before being initialized.Relevant Code Snippets:
For
pc_plus_offset_cy_r:For
pc_plus_4_cy_r:Both registers are used in self-referential expressions via the
*_cy_r_wwires, and their initial values influence the first cycle of computation.If they are not explicitly reset, their startup values can be undefined (X), potentially causing unpredictable results.
Suggestion
Add reset logic to assign a known initial value (1'b0) to both registers. For example:
For
RESET_STRATEGY == "NONE", you might consider using aninitialblock or extending thealways @(posedge clk)block accordingly.Why it Matters:
Prevents undefined/X propagation during startup
Ensures predictable and deterministic simulation results
Aligns with standard RTL design practice of initializing all stateful elements