RTL Review Checklist

27.1 closed the decisions. This gate asks whether they are actually in the code — and whether the code survives every event the architecture permits.

1. The One-Sentence Model

An RTL review asks two questions of every module: can each architecture decision be found in this code, and does this code behave correctly for every event the architecture allows — including the combinations nobody wrote a test for.

The first question is traceability. A decision recorded in a document and absent from the RTL was not implemented, whatever the document says (27.1 §16). A review that does not check this is checking style.

The second question is where the real defects live, and it has a specific shape. Almost every bug in this chapter is correct for the events the author imagined and wrong for a combination they did not — two events in the same cycle (§8), a subtraction that underflows once (§10), a state entered only after a recovery (§14). Each is invisible in the common case and permanent once it happens.

2. What This Gate Owns

Three boundaries, because this is the gate whose scope creeps most.

This gate does not re-open architecture. If the review finds that the retention policy is wrong, that is a 27.1 finding raised late, and it should be recorded as such rather than fixed quietly in RTL. A design where architecture decisions get made during RTL review has no architecture gate.

This gate does not replace verification, and the division is precise. Review finds defects that are visible in the code; DV finds defects that require state to expose. §8's lost decrement is visible in five lines. §14's unreachable state is visible in the case statement. Neither needs a simulation, and both routinely survive one.

And this gate does not chase style. Naming, formatting and structure matter for maintenance and are not why parts come back. Every item in §5–§21 is chosen because it produces silicon failures, not because it produces ugly code.

3. How to Read RTL in a Review

Line-by-line reading does not scale and does not find these bugs. Three passes do.

Three readings.

Pass 1 is the one unique to this gate, and it is fast. Take 27.1's decision list; for each, name the file and line where it is enforced. A decision with no line is either unimplemented or implemented somewhere nobody expects — and both are findings. 27.1 §20's capability record makes this mechanical: every field should map to code.

Pass 2 finds the largest class of real bugs in this chapter, and it is a mechanical question rather than an insight. For each register: list the events that write it, then ask which pairs can occur in the same cycle. If the code handles them in separate if branches, the last assignment wins and an update is lost (§8).

And pass 3 is where reviewers stop too early. "What happens at zero" catches unsigned underflow (§10). "What happens after a recovery" catches states reachable only in that path (§14). Neither is exotic; both are simply outside the scenario the author had in mind while writing.

4. Sourcing and Scope

5. Area A — Decision Traceability

Three readings.

A1 is checked by looking for an absence, which is why it is easy to miss. The correct implementation is a recovery branch that does not clear the transaction table. A reviewer scanning for what the code does will not notice what it deliberately does not do — so the review item is phrased as "show me the recovery branch and tell me what it leaves alone."

A4 asks for a comment, and that is not a style request. A parameter without its derivation cannot be re-checked when the assumption changes (27.1 §17). localparam int N_ID = 8; is unreviewable. // N_ID >= target_rate × rt_latency = 6.25 ops/ns × 500 ns is reviewable, and it fails review immediately at 8 — which is the entire point.

And A7 is checked across modules rather than within one. Retry logic in the adapter and in the protocol layer is a finding that no single-file review can produce. It is why pass 1 works from the decision list rather than from the file list.

6. Area B — Accepted-Event Semantics

The single most productive review question in this chapter: is this counting what was offered, or what was accepted?

Two readings.

B1 and B5 look similar and fail differently. B1 overcounts, which inflates every rate derived from it. B5 hides starvation — a grant into a blocked path is not service, and resetting the age on grant makes a starving requester look healthy exactly as the system gets busier (21.5 §29).

And B4 is the one that corrupts an adaptive mechanism rather than a report. Counting transport completions as work means retries count as progress (26.3 §16) — so a degrading target appears to speed up, and a service-rate-aware scheduler sends it more work. The instrument becomes the amplifier.

7. Wrong RTL — Counting the Offer

// WRONG. ILLUSTRATIVE. Counts every cycle the producer asserts valid. During
// backpressure the SAME transaction is counted once per stalled cycle. Written
// by someone who read "valid means a transaction is present" and stopped there.
always_ff @(posedge clk or negedge rst_n) begin
  if (!rst_n) txn_count_q <= '0;
  else if (in_valid) txn_count_q <= txn_count_q + 32'd1;   // BUG: not && ready
end

Architecture. One counter feeding throughput reporting and, downstream, a rate estimate.

State. txn_count_q.

Event. in_valid — an offering, not a transfer.

Contract. Every consumer of this counter assumes it counts transactions. It counts cycles of offering, and the two coincide only when ready is always high.

Failure — the arithmetic. One transaction stalled for 9 cycles before acceptance is counted 10 times. A link at 30 % acceptance reports roughly 3.3× its real transaction count — and the error grows as the system gets more congested, so the counter is least trustworthy exactly when it is most consulted.

Root cause. valid means "I have something"; valid && ready means "it moved" (21.5 §11). The bug is one token wide and inverts the meaning of every derived metric.

Corrected.

// CORRECT. Count the TRANSFER, which is the accepted event.
always_ff @(posedge clk or negedge rst_n) begin
  if (!rst_n) txn_count_q <= '0;
  else if (in_valid && in_ready) txn_count_q <= txn_count_q + 32'd1;
end
 
// MANDATORY. English: the transaction count increases only on a cycle where a
// transfer actually occurred. Catches the offered/accepted confusion directly,
// including if someone later "optimises" the ready term away.
a_count_on_transfer_only: assert property (
  @(posedge clk) disable iff (!rst_n)
    (txn_count_q != $past(txn_count_q)) |-> $past(in_valid && in_ready)
);

DV/debug. The review signature is a grep: every counter increment guarded by valid without ready is a candidate finding. This is one of the few defect classes a reviewer can find mechanically across an entire codebase in minutes.

8. Area C — Simultaneous Events

Pass 2's question (§3): for each register, which writing events can coincide? The classic failure is an increment and a decrement in the same cycle, written as separate branches.

// WRONG. ILLUSTRATIVE. Occupancy tracking with allocation and release in
// separate branches. Correct whenever they do not coincide — which is most
// cycles, in most tests, at low load.
always_ff @(posedge clk or negedge rst_n) begin
  if (!rst_n)          occ_q <= '0;
  else if (alloc_fire) occ_q <= occ_q + 16'd1;
  else if (free_fire)  occ_q <= occ_q - 16'd1;    // BUG: unreachable when both
end

Architecture. One occupancy counter gating allocation.

State. occ_q.

Event. Two events, priority-encoded by else if.

Contract. Callers assume occ_q equals live entries. It does — until the first cycle in which both fire.

Failure — the timeline. Each coincidence loses one decrement, so the count drifts upward and never recovers.

The end state is a permanent stall with an empty queue — allocation blocked by an occupancy counter that has drifted to its limit. At hour nine, in a soak test, on hardware.

Failure — the cycle timeline, at OCC_LIMIT = 64. The lethal property is that the drift is monotonic: there is no event anywhere in the design that reduces it.

First divergence: cycle 205 — the first cycle in which both events fire. Every symptom is ~10⁶ cycles later, which is far outside any trace window (21.7 §21) and long after the causal event is unrecoverable.

And the signature at the stall is diagnostic if you know to look for it: occupancy at its limit while the queue is empty. Those two facts are contradictory, and their contradiction names the bug — which is exactly what a_occupancy_conserved states, and why it fires at cycle 205 instead of at 10⁶.

Root cause. else if is a priority encoder, and these events are not prioritised — they are independent. The frequency of coincidence rises with load, so the bug accelerates exactly when the system is busiest.

Corrected.

// CORRECT. ONE next-state expression that nets both events. Cast to a signed
// width wide enough that the intermediate cannot wrap, then assign.
always_ff @(posedge clk or negedge rst_n) begin
  if (!rst_n) occ_q <= '0;
  else        occ_q <= occ_q + 16'(alloc_fire) - 16'(free_fire);
end
 
// MANDATORY. English: occupancy always equals allocations minus frees. This is
// the invariant the counter claims to represent, and it catches drift on the
// FIRST coincidence rather than at hour nine.
a_occupancy_conserved: assert property (
  @(posedge clk) disable iff (!rst_n)
    occ_q == (alloc_total_q - free_total_q)
);
 
// MANDATORY. English: allocation never occurs when occupancy is at the limit.
// Catches the gate being bypassed, which is how ids get reused while live.
a_no_alloc_when_full: assert property (
  @(posedge clk) disable iff (!rst_n)
    alloc_fire |-> (occ_q < OCC_LIMIT)
);

Contract. One register, one next-state expression is the general rule, and it is worth stating as a review standard rather than as advice. Any register written from more than one if branch is a pass-2 finding until proven otherwise.

DV/debug. a_occupancy_conserved fires on the first coincidence. Without it, the only symptom is a stall thousands of cycles later, by which point the causal event is long out of any trace window (21.7 §21).

9. Area D — Width, Overflow and Signedness

Three readings.

D2 has no universally correct answer, and the finding is the absence of a decision. Diagnostic counters should saturate — a wrapped counter produces an apparent negative rate and sends debug in the wrong direction. Sequence numbers must wrap, and their comparison must handle it. A counter whose intent is unstated is a finding regardless of which behaviour it has.

D3 is what makes saturation honest. A silently pinned counter is a diagnostic that lies while looking healthy (26.4 §12). One sticky overflow bit for a whole counter block is enough.

And D4 is the highest-severity item in this area, because unsigned underflow does not degrade gracefully — it jumps to the maximum value in one cycle (§10).

10. Wrong RTL — Unsigned Underflow

// WRONG. ILLUSTRATIVE. Available credits computed by subtraction. Correct
// whenever the invariant holds — and this code is what turns a ONE-CYCLE
// invariant violation into a PERMANENT wedge.
logic [7:0] credits_avail;
 
assign credits_avail = credit_limit_q - credits_used_q;   // BUG: unsigned
assign can_send      = (credits_avail != 8'd0);

Architecture. A subtraction feeding a send gate.

State. Two registers; the subtraction is combinational.

Event. Evaluated continuously.

Contract. It assumes credits_used_q <= credit_limit_q always. That is exactly the invariant the credit logic is supposed to maintain — so this code is correct precisely when nothing has gone wrong, and catastrophic the moment something has.

Failure — the arithmetic. If credits_used_q reaches 5 while credit_limit_q is 4 — one double-count, or one credit return applied twice — then 4 - 5 in 8-bit unsigned is 255, not −1.

The design transitions from "one credit over-counted" to "unlimited sending" in a single cycle — and the receiver's buffer overruns, which is data loss rather than a stall.

Failure — the second variant. If the sense were reversed and a counter underflowed toward zero, the value pins at maximum and can_send never deasserts. Both directions of this bug produce the worst available outcome, which is why the review item is unconditional.

Root cause. An invariant was used as a precondition without being enforced. The subtraction is a correct implementation of a statement that is only true while the rest of the design is correct.

Corrected.

// CORRECT. Never rely on an invariant you have not enforced. Clamp explicitly,
// and make the violation VISIBLE rather than absorbing it silently.
logic [7:0] credits_avail;
logic       credit_invariant_violated_q;   // sticky — see the assertion below
 
assign credits_avail = (credits_used_q >= credit_limit_q)
                     ? 8'd0                                  // clamp, never wrap
                     : (credit_limit_q - credits_used_q);
assign can_send      = (credits_avail != 8'd0);
 
always_ff @(posedge clk or negedge rst_n) begin
  if (!rst_n) credit_invariant_violated_q <= 1'b0;
  else if (credits_used_q > credit_limit_q)
    credit_invariant_violated_q <= 1'b1;    // sticky: it happened at least once
end
 
// MANDATORY. English: credits in use never exceed the configured limit. This
// is the credit-conservation invariant from 27.1 area D2, and it catches the
// ROOT cause one cycle after it occurs — rather than the overrun that follows.
a_credit_invariant: assert property (
  @(posedge clk) disable iff (!rst_n)
    (credits_used_q <= credit_limit_q)
);
 
// MANDATORY. English: a send never occurs without an available credit.
// Catches the §10 wedge directly, and stays valid even if the clamp is removed.
a_no_send_without_credit: assert property (
  @(posedge clk) disable iff (!rst_n)
    send_fire |-> (credits_avail != 8'd0)
);

Contract. The clamp prevents the catastrophe; the sticky bit and assertion prevent the silence. Clamping alone would be worse than the bug in one specific way — it would hide a credit-accounting error indefinitely, and the design would run slightly wrong forever with no evidence.

DV/debug. credit_invariant_violated_q readable from silicon converts "the link wedged" into "credit accounting broke, at least once, before it wedged" — which is a different and far shorter investigation (21.7 §14).

11. Area E — Reset

Three readings.

E1 and E2 are the highest-consequence items, and they are checked by reading the sensitivity list and the reset expression together. if (!rst_n || !link_rst_n) covering a transaction table is 26.5 §9's corruption, visible in one line — and it looks tidy, which is why it survives.

E4 is the item most often assumed rather than checked. Asynchronous assertion with synchronised release is standard practice; a release that is not synchronised into each clock domain can let different flops in the same domain leave reset on different cycles, producing a startup state no test constructs and no analysis predicts.

And E6 inverts the usual question in a way reviewers find unnatural. "Is anything reset that should persist?" catches a sticky first-fault register cleared by a link reset — which means the reset destroys the evidence for the fault that caused it (21.7 §14).

12. Area F — FSM Hygiene

// WRONG. ILLUSTRATIVE. A link-control FSM with three realistic defects.
// It simulates correctly, and it is one upset away from a permanent hang.
typedef enum logic [2:0] { L_RESET, L_TRAIN, L_ACTIVE, L_RECOVER } lnk_e;
lnk_e lnk_q;
 
always_ff @(posedge clk or negedge rst_n) begin
  if (!rst_n) lnk_q <= L_RESET;
  else begin
    // BUG 1: 3-bit enum, 4 defined values — FOUR encodings are undefined, and
    //        there is no default, so the FSM holds an illegal state forever.
    case (lnk_q)
      L_RESET:   if (start)        lnk_q <= L_TRAIN;
      L_TRAIN:   if (train_ok)     lnk_q <= L_ACTIVE;
      L_ACTIVE:  if (err)          lnk_q <= L_RECOVER;
      // BUG 2: no timeout on TRAIN or RECOVER — a peer that never responds
      //        produces an unbounded, unreported wait (26.2 §10).
      L_RECOVER: if (recover_done) lnk_q <= L_ACTIVE;
      // BUG 3: no default. Synthesis may infer a latch-like hold, and an
      //        illegal state has no path back to L_RESET.
    endcase
  end
end

Architecture. Four states, one transition each.

State. lnk_q, three bits, eight encodings, four defined.

Event. Each transition has exactly one trigger and no escape.

Contract. Callers assume the FSM always makes progress or reports why it cannot. It does neither in three cases.

Failure. BUG 2 is the one that reaches silicon most often. A peer that never completes recovery leaves the FSM in L_RECOVER forever, with no error and no timeout — indistinguishable from a slow link, for as long as anyone is willing to look (26.2 §10). BUG 1 and BUG 3 together mean any upset into an undefined encoding is permanent.

Corrected.

// CORRECT. Bounded waits with NAMED failures, a total case, and a defined
// recovery path from any illegal encoding.
typedef enum logic [2:0] {
  L_RESET, L_TRAIN, L_ACTIVE, L_RECOVER, L_TRAIN_FAIL, L_RECOVER_FAIL
} lnk_e;
 
lnk_e        lnk_q;
logic [15:0] wait_q;
 
always_ff @(posedge clk or negedge rst_n) begin
  if (!rst_n) begin
    lnk_q  <= L_RESET;
    wait_q <= '0;
  end else begin
    // unique case flags overlapping/missing coverage in simulation, and the
    // default guarantees a path home from ANY encoding, defined or not.
    unique case (lnk_q)
      L_RESET:   if (start) begin lnk_q <= L_TRAIN; wait_q <= '0; end
 
      L_TRAIN:   if (train_ok)                 lnk_q  <= L_ACTIVE;
                 else if (wait_q == TRAIN_TO)  lnk_q  <= L_TRAIN_FAIL;
                 else                          wait_q <= wait_q + 16'd1;
 
      L_ACTIVE:  if (err) begin lnk_q <= L_RECOVER; wait_q <= '0; end
 
      L_RECOVER: if (recover_done)             lnk_q  <= L_ACTIVE;
                 else if (wait_q == RECOV_TO)  lnk_q  <= L_RECOVER_FAIL;
                 else                          wait_q <= wait_q + 16'd1;
 
      L_TRAIN_FAIL,
      L_RECOVER_FAIL: if (diag_clear)          lnk_q  <= L_RESET;
 
      default:                                 lnk_q  <= L_RESET;   // BUG 3 fix
    endcase
  end
end
 
// MANDATORY. English: the FSM never remains in a wait state longer than its
// configured bound. Catches BUG 2 — the unbounded, unreported wait.
a_train_bounded: assert property (
  @(posedge clk) disable iff (!rst_n)
    (lnk_q == L_TRAIN) |-> ##[1:TRAIN_TO+1] (lnk_q != L_TRAIN)
);
 
// MANDATORY. English: the FSM is only ever in a defined state. Catches BUG 1
// in simulation, and is a natural formal target.
a_fsm_state_legal: assert property (
  @(posedge clk) disable iff (!rst_n)
    (lnk_q inside {L_RESET, L_TRAIN, L_ACTIVE, L_RECOVER,
                   L_TRAIN_FAIL, L_RECOVER_FAIL})
);

Contract. The timeout values must come from the peer's stated worst case, not from measurement (26.2 §12) — a timeout raised each time it fires is a timeout that cannot detect the bug it exists for (21.6 §29).

DV/debug. The failure states are named and distinct, so a stuck link reports L_TRAIN_FAIL or L_RECOVER_FAIL rather than being silent. diag_clear rather than a functional reset returns them (E7), so the fault record survives re-arming.

13. Area G — Clock Domain Crossings in Code

Three readings.

G3 is the item where well-intentioned code is most often wrong. Synchronising a multi-bit bus flop by flop feels safer than not synchronising it. It is the actual bug — independent synchronisers can resolve on different cycles, producing a value that was never sent. The correct structure holds the payload stable by protocol and synchronises only the control signal.

G4 is a one-line finding with a real consequence. Edge detection on sync1 samples a potentially metastable value. The extra stage exists solely so that edge detection operates on something stable, and removing it is a natural-looking optimisation.

And G6 catches the bug that only appears under load. Without waiting for the acknowledgement to drop, two back-to-back events merge into one at a slow destination (26.5 §12) — invisible at low rates, and a lost event at high ones.

14. Area H — States Reachable Only After Recovery

Pass 3's question (§3). The most under-reviewed region of any link design is the state space entered only after an error.

Three readings.

H3 is a single-line check with a corruption-class consequence. An allocator reset by a link event restarts at zero while the peer still holds the old identities — and the reuse window is every link reset (26.5 §9).

H5 is asked because the answer is usually "that can't happen". A second error during recovery is exactly the case that occurs on a marginal link — the condition under which recovery is happening at all. If the second event is dropped, the design is unrecovered and believes it is recovering.

And H6 requires the code to be consistent with A1's documented answer, not merely self-consistent. A design that retains obligations but silently discards late completions has implemented neither policy — and that mixture is not in anyone's specification.

15. Area I — Parameters and Their Derivations

Two readings.

I5 is the defect that survives every functional test at the default configuration and breaks the first derivative product. localparam N_CH = 16; beside logic [3:0] ch_sel; works — until someone sets N_CH = 32 and the selector silently truncates. Widths must be derived ($clog2(N_CH)), and a hardcoded width beside a parameter is a finding on sight.

And I6 is cheap insurance that reviewers rarely ask for. A compile-time $error for illegal parameter combinations turns a subtle runtime misbehaviour into a build failure — and it costs three lines.

16. Area J — Assertion Review

27.1 commissioned a list (27.1 §21). This gate checks the assertions exist, say what they mean, and can actually fail.

Three readings.

J3 is the item that turns an assertion count into an assertion review. A property whose antecedent never occurs passes forever and proves nothing. The check is a coverage question — cover property on the antecedent — and it belongs beside every non-trivial assertion.

J4 is the quietest defect in this area. disable iff (!rst_n || any_error) is common and often disables the assertion exactly when the interesting behaviour happens. The legitimate use is disabling a property while the design is deliberately in a state where it does not apply (26.3 §14) — and the review question is whether the disable term is that, or a way to stop a failing assertion from failing.

And J7 is the acceptance test for the whole area. An assertion that cannot be traced to a specific bug it would catch is decoration, and decoration costs simulation time and review attention without buying anything.

17. Wrong SVA — Three Realistic Assertion Defects

// WRONG #1. Vacuous. `req_special` is a mode never enabled in this product.
// The property passes in every regression and has never been evaluated.
a_special_response: assert property (
  @(posedge clk) disable iff (!rst_n)
    req_special |-> ##[1:8] resp_valid
);
// Fix: pair every non-trivial assertion with a cover on its antecedent, so
// "never failed" is distinguishable from "never ran".
c_special_seen: cover property (@(posedge clk) req_special);

// WRONG #2. Over-broad disable. The intent was to ignore the property during
// reset. As written it is also disabled during every error condition — which
// is precisely when the retention behaviour matters (§14).
a_obligation_retained: assert property (
  @(posedge clk) disable iff (!rst_n || err_active)      // BUG: err_active
    (recovery_done && txn_live_q[chk]) |=> txn_live_q[chk]
);
// Fix: disable only for reset. If the property genuinely does not apply in
// some state, encode that in the ANTECEDENT where it is visible and reviewable.
a_obligation_retained_fixed: assert property (
  @(posedge clk) disable iff (!rst_n)
    (recovery_done && txn_live_q[chk]) |=> txn_live_q[chk]
);

// WRONG #3. Unbounded consequent. This can never fail in a finite simulation:
// the tool waits forever for the response, and the test ends first.
a_response_eventually: assert property (
  @(posedge clk) disable iff (!rst_n)
    req_fire |-> ##[1:$] resp_fire                       // BUG: unbounded
);
// Fix: bound it with the architecture's stated maximum. A bound that must be
// raised each time it fires is not a bound (21.6 §29) — if the number is
// unknown, that is a 27.1 finding, not an assertion style choice.
a_response_bounded: assert property (
  @(posedge clk) disable iff (!rst_n)
    req_fire |-> ##[1:MAX_RESP_LATENCY] resp_fire
);

Architecture. Three properties that a reviewer counting assertions would score as three. A reviewer applying J3, J4 and J6 scores them as zero.

State. None added — which is why these defects are free to accumulate.

Event. #1 never triggers, #2 is disabled when it matters, #3 never completes.

Contract. Each claims to enforce something. None does, and their presence actively reduces scrutiny elsewhere because the property is "covered".

Failure. #2 is the most dangerous of the three: it is the assertion for 26.5 §21's retention policy, disabled during exactly the error conditions under which recovery occurs. The one bug it exists to catch is the one it cannot see.

DV/debug. The general review practice is worth stating as a rule: every non-trivial assertion needs a matching cover on its antecedent, and an assertion that has never failed in development is suspicious rather than reassuring. Deliberately breaking the RTL to confirm a property fires is the cheapest way to know it works (25.8 §22).

18. Area K — X-Propagation and Simulation/Synthesis Mismatch

The last review area, and the one whose defects are invisible in RTL simulation by construction: the places where simulation is more forgiving than silicon, or more punishing.

Three readings, and K2 and K3 are the pair that most reliably surprises people.

K2 — X-optimism means simulation can be kinder than silicon. If a condition evaluates to X, a simulator's if takes the else branch and continues, producing plausible behaviour. Silicon has no X — it has a real, definite value that may take the other branch. So a design with an uninitialised control signal can simulate cleanly for a year and take a different path in a real part. The review question is K1's: is every state element that is read before being written actually reset?

K3 is the mirror image — synthesis can be kinder than simulation, and then silicon is not. unique case tells the tool that the cases are mutually exclusive and complete. Simulation checks that claim and warns; synthesis believes it and optimises accordingly, removing the priority logic that would have made an unexpected combination safe. If the claim is false in a state no test reached, the simulator would have told you and the silicon will not. So unique is a promise, and the review item is "what makes this promise true?" — not "does it simulate?"

And K6 is the classic, worth one grep rather than one debate. A combinational always_comb with a path that assigns nothing infers a latch. always_comb will warn where always @(*) may not, which is one concrete reason the review standard should require always_comb — not for style, but because it makes an entire defect class visible to the tool.

The general principle behind the whole area, and the reason it belongs at this gate rather than to DV: a defect that RTL simulation cannot express will not be found by more simulation. These seven items are found by reading, by lint, and by gate-level or X-pessimistic simulation — and the review is the cheapest of the three.

19. The Consolidated Review Sheet

And the ordering is deliberate. A first, because a decision missing from the code makes every other area moot. B and C next, because they are mechanical, fast, and corrupt everything downstream. D through H are the correctness core. J and K last — reviewing assertions is only meaningful once you know what the code is supposed to do, and K's defects are the ones no amount of RTL simulation will surface (§18).

20. How the RTL Review Itself Fails

Three readings.

The last row is the one that catches experienced teams. A diff-based review is efficient and structurally blind to §8's defect class: adding a new event that can coincide with an existing one breaks code that did not change. The affected lines are not in the diff, and the reviewer never looks at them.

Style findings are not wrong; they are miscategorised. A review that produces forty naming comments and no event-coincidence findings has spent its attention badly — and the author, reasonably, treats the whole review as low-value.

And "reviewing files" misses precisely the items that matter most across a system. A7's "is retry implemented exactly once?" cannot be answered from any single file — which is why pass 1 works from the decision list rather than the file list (§3).

21. Red Flags

Two readings.

Every row is greppable or visually obvious, which is the point. An RTL review's leverage comes from pattern recognition, not from comprehension — you cannot hold a large module in your head, and you do not need to.

And two rows deserve the most patience, because both are frequently correct: "that path can't be taken" and the two-term reset expression. A reset that legitimately covers two scopes exists. The review question is not skepticism but specificity — prevented by what, and what survives? (27.1 §23).

22. Common Misconceptions

"An RTL review checks code quality." §1: it checks that architecture decisions are present in the code and that the code survives every permitted event. Quality matters and is not why parts come back.

"Read it line by line to be thorough." §3, §20: line-by-line reading runs out of attention before the state machines. Three targeted passes — traceability, event enumeration, extremes — find more in less time.

"if (valid) is fine, ready is implied." §7: it counts cycles of offering. At 30 % acceptance the count is 3.3× too high, and the error grows with congestion.

"An else if is just a coding style." §8: it is a priority encoder. On independent events it silently loses an update, drifts upward, and wedges at hour nine.

"Underflow just wraps, it's a small error." §10: 4 - 5 in 8-bit unsigned is 255. The design goes from correct backpressure to unlimited sending in one cycle — data loss, not a stall.

"Clamping the subtraction fixes it." §10 Contract: clamping alone hides a credit-accounting error indefinitely. The clamp needs a sticky violation bit and an assertion, or the design runs slightly wrong forever with no evidence.

"A default case is defensive clutter." §12: without it an upset into an undefined encoding is permanent, with no path back to reset.

"Synchronise the bus to be safe." §13, G3: independent synchronisers can resolve on different cycles, producing a value that was never sent. Hold the payload stable by protocol; synchronise only the control signal.

"We have 200 assertions." §17: three properties can be vacuous, over-disabled and unbounded — a count of three and a value of zero. cover on the antecedent is what makes the count mean anything.

"If it simulates cleanly it is fine." §18: simulation has X and silicon does not. An uninitialised control signal can take the else branch for a year in simulation and the other branch in a real part — and unique case is a promise synthesis believes, not a check it performs.

"Reviewing the diff is enough." §20: adding an event that can coincide with an existing one breaks unchanged code, and those lines are not in the diff.

23. Understanding Check

24. Summary

Seven things.

Two questions, asked of every module (§1): can each architecture decision be found in this code, and does it survive every event the architecture permits — including combinations nobody tested.

Three passes, not line-by-line (§3): traceability against 27.1's decision list, event enumeration per register, and extremes — zero, max, reset, post-recovery.

Accepted events and simultaneous events are the mechanical wins (§6–§8). valid without ready inflates every derived metric; an else if on independent events loses an update and wedges at hour nine.

Unsigned underflow is the highest-severity single line (§10). 4 - 5 is 255 — correct backpressure to unlimited sending in one cycle, with data loss at the far end. Clamp, make it sticky, and assert the invariant.

FSMs need bounded waits, a total case and named failures (§12); post-recovery is the least-reviewed state space in any link design (§14), and ID reuse while the peer still holds an identity is corruption on every recovery.

And two areas close the gate. X-propagation and sim/synth mismatch (§18) — simulation is X-optimistic where silicon is definite, and unique is a promise the tool believes; a defect RTL simulation cannot express will not be found by more simulation. And assertions are reviewed for effect, not counted (§16–§17). Vacuous, over-disabled, unbounded — three properties, zero coverage. The acceptance test is which bug does this catch, and can it fire?