Reviewing Verification Code

Overview

Verification code review has a different failure cost than design review: a sloppy RTL block fails in simulation, but a sloppy testbench fails by passing — it stops checking, and nobody notices until silicon. That asymmetry is why senior DV engineers treat review as defect prevention for the checking machinery itself, not as style enforcement. The questions that matter are: can this constraint set ever hide a legal-but-nasty case? Can this scoreboard ever compare nothing and report green? Can this assertion ever pass without firing?

This topic gives you artifact-specific checklists — what to hunt for in transactions and constraints, in drivers/monitors/scoreboards, and in assertions and covergroups — each anchored by a worked review of genuinely flawed code. It closes with the process side (how to phrase comments that land, what to block on) and a consolidated printable master checklist.

Sub-topics

1. Reviewing Transactions & Constraints — over-constrained bases, legality vs typicality, unchecked randomize, copy/compare gaps; worked review of a flawed transaction class.

2. Reviewing TB Components — wrong-edge drivers, driver-snooping monitors, drain handling, fork hygiene; worked review of a flawed driver.

3. Reviewing Assertions & Coverage — assert/cover pairing, vacuity, reset gating, bin intent; worked review of a flawed SVA + covergroup file.

4. Review Process & Communication — scope discipline, comment phrasing, block-vs-note judgment, mentoring through review.

5. The Consolidated Checklist — the full master list by artifact type, with rationale, and the 30-minute review method.

Legend: [TXN] [COMP] [SVA/COV] [PROCESS]

  REVIEW PASSES — order matters: architecture before syntax

  pull request arrives
       │
       ▼
  PASS 1 — INTENT [PROCESS]
  what is this change supposed to do? does the approach fit
  the env architecture? (wrong approach → stop, talk, not 40 nits)
       │
       ▼
  PASS 2 — CHECKING INTEGRITY
  ┌──────────────┬──────────────────┬─────────────────────┐
  │ [TXN]        │ [COMP]           │ [SVA/COV]           │
  │ constraints  │ driver timing    │ vacuity, reset      │
  │ rand checked │ monitor source   │ gating, assert/     │
  │ copy/compare │ drain, forks     │ cover pairing, bins │
  └──────────────┴──────────────────┴─────────────────────┘
       │
       ▼
  PASS 3 — MAINTAINABILITY
  naming, duplication, magic numbers, comments that lie
       │
       ▼
  verdict: approve / approve-with-notes / request changes
  (block ONLY on correctness and architecture; note the rest)

Key takeaways

• Testbench bugs fail by passing — review verification code as defect prevention for the checking machinery.

• Review in passes: intent and architecture first, checking integrity second, style last.

• Every checklist item exists because a real bug class escaped without it.

• Block only on correctness and architecture; everything else is a note.