RTL Code Reviewer — IC HDL Agent Skill

name: rtl-reviewer description: > Perform a comprehensive code review of Verilog/SystemVerilog RTL against industry-standard quality criteria. Checks for: synthesis-simulation mismatches, inferred latches, blocking/non-blocking assignment violations, reset issues, CDC problems, coding style violations, unsynthesizable constructs, potential simulation race conditions, naming convention violations, and missing comments. Produces a categorized report with severity levels, file locations, and fix recommendations. Equivalent to a senior digital design engineer's code review. category: verification level: advanced

Purpose

Review RTL code and produce a detailed, actionable report. Every issue is categorized by severity (ERROR, WARNING, INFO) and includes a specific fix. The review must catch bugs that would survive synthesis and simulation but cause failures in silicon.

Review Scope

Review every line of the provided RTL for these categories:

Category 1: Synthesis-Simulation Mismatch Risks (ERROR)

These are silicon-killing bugs. Highest severity.

Checklist:

Mixed blocking/non-blocking assignments in same always block
Blocking assignments (=) in always_ff blocks
Non-blocking assignments (<=) in always_comb blocks
Race conditions from multiple always blocks driving same signal
Read/write race conditions in same time step
#delay in synthesizable code
$display/$monitor in synthesizable modules (move to testbench)
Incomplete sensitivity lists (if not using always_comb / always @*)
Simulation-only constructs used in synthesizable code (initial, forever, fork, wait, while with non-static bound)

Category 2: Latch Inference Risks (ERROR)

Combinational always block missing else branch
Combinational always block missing default in case
Signal assigned in some branches but not all
Signal read before being assigned in the same combinational block
Recommendation: always add default assignments at top of always_comb

Category 3: Reset Issues (WARNING → ERROR if safety-critical)

Missing reset on sequential elements (is it intentional?)
Inconsistent reset strategy within same module (some sync, some async)
Reset value not specified
Reset recovery/removal timing not considered (for async reset)
Multiple reset domains not clearly separated
Reset synchronizer not implemented for async reset

Category 4: CDC (Clock Domain Crossing) Issues (ERROR)

Single-bit signal crossing domains without synchronizer
Multi-bit signal crossing domains without FIFO or handshake
Binary counter/pointer crossing domains (must be Gray-coded)
Missing CDC attributes on synchronizer flip-flops
Combinational logic between synchronizer stages

Category 5: Coding Style Violations (WARNING)

Signal naming convention violations
Missing or incomplete module header comment
Magic numbers instead of named parameters/localparams
Width mismatches in assignments (implicit truncation/extension)
Unsized constants ('b1, 'd5) — use sized (1'b1) or '0/'1
Overly deep nested if/else (>3 levels) — suggests case or refactoring
Unused signals or ports
Undriven inputs (floating)
Multi-driven nets

Category 6: Unintentional Logic (WARNING → ERROR)

Combinational loops (a = b & c; c = a | d)
Implicit latch enable from complex conditional expression
Priority encoder where parallel case expected
Unintended multiplier/divider from * or / operator
Bit-width overflow in arithmetic without protection
Signed/unsigned mixing without explicit casting

Category 7: Synthesis Optimization Issues (INFO → WARNING)

Overly deep combinational logic (>15 levels) — timing closure risk
Large case statements that could be ROM/FSM
Unregistered outputs on module boundary — timing closure risk
Missing pipeline stage opportunities on critical paths
Inefficient resource usage (e.g., / instead of >> for power-of-2)

Category 8: DFT (Design for Test) Readiness (INFO)

All flip-flops in scan chain? (Check for un-scannable FFs)
Asynchronous set/reset that blocks scan?
Clock gating cells properly inserted (not raw AND gates)?
Test clock vs functional clock separation?

Output Report Format

code

================================================================
  RTL CODE REVIEW REPORT
  Module: [module_name]
  File: [filename]
  Reviewed by: ICHDL Agent (rtl-reviewer)
================================================================

SUMMARY
  ERROR:   [N] issues — MUST FIX before tape-out
  WARNING: [N] issues — SHOULD FIX before code review sign-off
  INFO:    [N] issues — consider fixing for improved quality

----------------------------------------------------------------
  ERRORS
----------------------------------------------------------------

[E01] Mixed blocking/non-blocking in always block
  Line: 45
  Code: always_ff @(posedge clk) begin
            tmp = a + b;     // ← BLOCKING in sequential block
            result <= tmp;   // ← NON-BLOCKING
        end
  Risk: Simulation-synthesis mismatch. In simulation, 'tmp' updates
        immediately; in synthesis, 'tmp' is a intermediate wire.
  Fix: Move combinational logic to separate always_comb block:
        always_comb tmp = a + b;
        always_ff @(posedge clk) result <= tmp;

[E02] Latch inferred in combinational block
  Line: 72
  Code: always_comb begin
            if (en) y = a & b;
        end
  Risk: y retains previous value when en=0, creating a latch.
  Fix: Add default assignment: y = '0; before if statement.

----------------------------------------------------------------
  WARNINGS
----------------------------------------------------------------

[W01] Unsized constant in arithmetic
  Line: 120
  Code: count <= count + 1;  // 1 is 32-bit by default
  Risk: Width mismatch if count is not 32-bit.
  Fix: count <= count + 1'b1;

[W02] Missing module header comment
  Line: 1
  Risk: Poor documentation. Future maintainers will not understand
        the module's purpose, parameters, or interface.
  Fix: Add header comment with module description, parameters
        table, port table, and design notes.

----------------------------------------------------------------
  INFO
----------------------------------------------------------------

[I01] Consider using SystemVerilog 'logic' instead of 'reg'/'wire'
  Line: 15-22
  All ports and internal signals can use 'logic' type for clarity.

[I02] Deep combinational logic chain (estimated 18 levels)
  Line: 88-95
  Risk: May not meet timing at target frequency.
  Suggestion: Consider inserting a pipeline register at line 92.

================================================================
  REVIEW COMPLETE
================================================================

Review Severity Guidelines

| Severity | Definition | Action | |----------|-----------|--------| | ERROR | Will cause functional failure in silicon | Must fix before tape-out | | WARNING | May cause issues in corner cases or is a significant quality concern | Should fix before code review sign-off | | INFO | Style, readability, or minor optimization opportunity | Consider fixing; not blocking |

Checklist Verification

Before outputting the report, verify:

[ ] Every always block checked for blocking/non-blocking correctness
[ ] Every combinational always block checked for latch inference
[ ] Every port checked for direction and connectivity
[ ] Every signal checked for multiple drivers
[ ] Every CDC path identified and verified
[ ] Every reset path verified
[ ] Synthesis attributes checked (if present)
[ ] Naming conventions checked
[ ] All issues have specific line numbers and fix recommendations

Interaction with Other Skills

verilog-module: Apply this review to generated code before delivery
fsm-generator: FSM-specific review criteria
fifo-generator: FIFO-specific review criteria (overflow/underflow, pointer logic)
cdc-synchronizer: CDC-specific review criteria
sdc-writer: Cross-check SDC constraints against reviewed RTL
assertion-writer: Suggested assertions are listed in review INFO items

Disclaimer

This skill is provided "as is" for reference and educational purposes only. Code generated using this skill should be independently reviewed and verified before use in any production design, tape-out, or safety-critical application. The author(s) and ICHDL assume no liability for any errors, omissions, or damages arising from the use of this skill or any code generated with it.

Usage Rights

You are free to use, modify, and distribute this skill for personal or commercial purposes. Attribution to ICHDL (ichdl.com) is appreciated but not required. Redistribution of this skill in substantially unmodified form must retain this notice.