HDL Conventions

I've tried to be consistent with some conventions and rules through this HDL implementation. These both make the design easier to read and understand and reduce bugs and synthesis mismatches. I've documented them here with motivations. Some of these are relaxed for test bench code, as their main goal to ensure proper synthesis. Endnotes reference sources that give more background and justification.

General

Use logic to define nets and flops rather than reg and wire. This is cleaner, since reg and wire are somewhat inconsistent in their usage.¹¹ An implication of this is that you must use an explicit 'assign' statement for continuous assignment (unlike wire, which can combine it with the declaration):

No:
```
wire foo = a & b;
```
Yes:
```
logic foo;
assign foo = a & b;
```
Do not perform initialization in net declarations. Initialize nets in an initial block (for simulation-only testbenches) or with a reset signal (for synthesizable modules) instead. Some synthesis tools produce incorrect logic with the first form, this construct can mask reset bugs during simulation, and mixing both forms could result in unexpected behavior.

No:
```
logic burst_w = '0;
```
Yes:
```
logic burst_w;
...
initial
begin
    burst_w = '0;
```
Use interfaces and structs to group related signals.
- This makes the code more concise and intent more explicit.
- Less likely to miss flops, since a single statement can register all signals in a struct.²⁰
Prefer typedefs rather than hardcoded sizes. e.g. rather than:
```
logic[3:0] subcycle1;
logic[3:0] subcycle2;
```
Do this:
```
typedef logic[3:0] subcycle_t; // in defines.sv

subcycle_t subcycle1;
subcycle_t subcycle2;
```
- More readable, less visual noise with array indices.
- Makes code more understandable by giving context about signals.
- Avoids size mismatch bugs (for example, if I changed a signal size and missed one spot).
- Since this is an experimental design that is in flux, this makes it easier to change things.
Use enumerations wherever possible (as opposed to constant parameters or `defines).
- Tools will raise an error if code assigns a non-enumerated type to them, which can catch bugs (unfortunately, Verilator does not yet).²⁰
- Will catch if two constants have the same values
- Checks that signals are wide enough to hold all values. Less likely to have one-off size mismatches and easier to add new constants.
Avoid using 'X' values and casex. These have well known problems that can cause simulation/synthesis mismatches,² but also Verilator ignores them (as it is a two-state simulator), so tests may not catch issues that would break 4-state simulators.¹²
Instantiate memories using sram_1r1w and sram_2r1w rather than arrays.

No:
```
    logic[31:0] my_memory[4096];
```
Yes:
```
sram_1r1w #(
    .DATA_WIDTH(32),
    .SIZE(4096),
    .READ_DURING_WRITE("NEW_DATA")
) my_memory(.*);
```
- This ensures FPGA tools infer memories rather than a huge number of flops.⁷
- Avoids inadvertently synthesizing extra ports.
- These enforce proper synchronous access semantics.
- This guarantees consistent read-after-write behavior (when the same address is read and written simultaneously) and makes these requirements explicit. This behavior tends to vary in structural FPGA primitives between vendors and even between FPGAs in the same family. Using behavioral arrays may generate more logic than necessary to support unneeded semantics.
- In simulation, these modules clear themselves to avoid X propagation bugs in 4-state simulators.
- Simulation randomizes output when read enables aren't active to catch bugs.
- For ASIC synthesis, this allows using memory generator tools.
Don't use translate_off/translate on. Use ifdef SIMULATION where necessary (but only if necessary; this should be rare).^{9, 16}
- Using the preprocessor will catch missing `endifs, where there is no checking for missing translate_offs.
- But these constructs cause synthesis/simulation mismatches by design, so ensure it is something that only simulation cares about (for example, extra runtime error checking). Synthesis tools ignore most non-synthesizable constructs, so these are usually not necessary.
Use default statements for all case statements.
- Makes design more robust so it can recover from glitches
- Avoids inferring latches.⁷
- Avoids X-latching.²
Don't use full_case/parallel_case pragmas. These cause simulation and synthesis to differ and can cause subtle bugs that tests won't catch.¹⁰ Behavior can also vary between synthesis tools.

Case statements should be simple and parallel (i.e. only one case is true). Use the 'unique' keyword on all case statements to verify this with the simulator.²⁰ Use constant values for the case clauses, and prefer a single net in the case statement. This makes it easier for synthesis tools to optimize logic, and gives more consistent among vendors and between synthesis and simulation:

Yuck:

logic a;
logic b;
logic c:
case (1'b1)
    a: // do something
    b: // do something else
    c: // another thing
endcase

Better:

    unique case (dd_request_vaddr.offset[1:0])
        2'd0: byte_aligned = mem_load_lane[31:24];
        2'd1: byte_aligned = mem_load_lane[23:16];
        2'd2: byte_aligned = mem_load_lane[15:8];
        2'd3: byte_aligned = mem_load_lane[7:0];
        default: byte_aligned = '0;
    endcase

Use assertions liberally. As well as catching bugs, assertions clarify assumptions and requirements.¹⁹
- Add a comment above each assertion to describe why it should be true.
- assert should only be inside an always_ff and shouldn't be reachable when reset is high. Otherwise they may trigger incorrectly.
- Use $onehot or $onehot0 to ensure one-hot signals only have one signal set.
- Do not add assertions that duplicate RTL. Assertions should validate behavior, invariants, and inter-module dependencies.
No:
```
assign foo = bar && (baz || !boo);
...
assert(foo == bar && (baz || !boo));
```
Yes:
```
for (way_idx = 0; way_idx < `L1D_WAYS; way_idx++)
begin : hit_check_gen
    assign way_hit_oh[way_idx] = dt_request_paddr.tag == dt_tag[way_idx]
        && dt_valid[way_idx];
end

// Make sure data is not present in more than one way.
assert(!dcache_load_en || $onehot0(way_hit_oh));
```
Allow tools to infer state machines from a case statement/next_state logic rather than trying to code something like a one-hot state machine explicitly. This allows the tools to generate more efficient logic and is easier to understand (thus less error prone). ⁷
Make all internal enable signals active high. It's easier to understand and less error prone. The exception is external interfaces that are already specified (e.g. SPI or SDRAM), which should have an _n suffix if they are active low.
Avoid generating logic with functions. These are harder to reason about and can infer latches¹⁶
Avoid internal tri-state/bidirectional (inout) signals. FPGA synthesis tools emulate these with multiplexers, but they may affect the ability of tools to optimize across hierarchical boundaries.⁷ These are okay for external pins (which have I/O cells that can actually float).
When computing address widths, use $clog2 rather than hardcoding.²⁰ Use $bits where needed to determine the size of a signal.

Use explicit $signed() or signed when performing signed operations. This makes intent explicit and avoids subtle bugs.: Yes:

OP_SEXT8: lane_result = 32'($signed(lane_operand2[7:0]));

logic signed[7:0] addend;

No

OP_SEXT8: lane_result = {{24{ifd_instruction[22]}}, ifd_instruction[22:15]};

Clocks and Reset

Do not use behavioral delays. These are not supported in Verilator, nor by synthesis tools, and can cause synthesis/simulation mismatches.¹⁶
```
#4 a = b + c;
q <= #2 d;
```
Name the main clock 'clk', and make all flops positive edge triggered. Mixing positive and negative edge triggered blocks is more complex and can reduce maximum clock speed.¹⁸
Keep the number of clock domains to a minimum.⁵ There are a limited clock resources on FPGAs, and clock trees consume area on ASICs. Clock domain crossings are difficult to test and debug in simulation and can cause subtle bugs. Prefer to use flip flop enables where possible instead.
Do not use the same signal as a clock (i.e. in a @posedge/@negedge block) and as data (i.e. RHS of an expression). Same with resets. These complicate synthesis and timing analysis.⁵ Note that some external signals that have clock in their name (for example, the SPI clock) are treated strictly as enables, which is fine, as, despite the name, they are never routed into clock inputs of flops and are data signals from as far as the synthesis tools are concerned.
Use always_ff and always_comb to avoid inferred latches or sensitivity list bugs.⁶ always_comb also triggers at time 0 to avoid subtle initialization bugs.¹³

Use nonblocking assignment for sequential logic (always_ff), blocking for combinational (always_comb).¹

always_ff @(posedge clk)
    my_ff <= another_ff;

always_comb
    my_signal = another_signal;

Do not use nonblocking and blocking assignments in the same block.¹
Do not make assignments to the same variable from multiple always blocks.¹ Verilator does not flag this as an error, but many other tools do, as it creates ambiguous logic definitions.
Do not use latches. These are difficult to make glitch-free, have slower timing performance on FPGAs, and timing analysis tools cannot model them as accurately.¹⁷
There is a single reset named 'reset', which is active high. All flops are reset asynchronously. This style reduces area and increases maximum clock frequency on Altera FPGAs, as a synchronous reset adds an extra gate in front of each flop. Since this is a large design that barely fits on some FPGAs, this is important.⁷ Reset should be globally synchronized to clk. If using another clock domain, be careful to synchronize reset properly.⁴

Reset only to constant values (not to other signals) to avoid subtle reset bugs:

Bad:

    always_ff @(posedge clk, posedge reset)
    begin
        if (reset)
            my_ff <= data_value_in;

Okay:

    always_ff @(posedge clk, posedge reset)
    begin
        if (reset)
            my_ff <= '0;

If an always_ff block has a reset, make sure to reset all flops that are assigned anywhere in the block. Otherwise, synthesis tools may use reset as a data enable.⁴ Use AUTORESET to automatically generate these and reduce bugs.⁸

Only use reset if necessary. If a signal doesn't need to be reset for proper operation (data path signal), put it in a block without reset in the sensitivity list. This improves area and timing:

If a signal needs reset:

    always_ff @(posedge clk, posedge reset)
    begin
        if (reset)
            control_signal <= '0;
        else
            control_signal <= a && (!b || c);

If a signal does not need reset:

    always_ff @(posedge clk)
    begin
        data_signal <= some_other_data_signal;

Naming/Formatting

Keep the same signal name through all levels of the hierarchy⁵ (the exception is for generic modules like SRAMs). This makes it easier to find signals across different modules. It also allows implicit port connections with .*, which are less error prone and reduce redundant code and verbosity.¹⁴

No:
```
    my_module my_module(
       .foo(bar),
       .bar(baz));
```
Signal and module names use snake_case, lowercase only. Parameters and preprocessor defines use uppercase.
Make identifier names succinct, but don't abbreviate unnecessarily. Remove things that don't add information.³

Bad:
```
ins_vld, is_unaligned, load_signal
```
Better:
```
instruction_valid, unaligned, load
```

Break up complex logic by adding named signals to make it clearer:

Okay:

if (is_infinity || (add_result_exponent == 8’hFF && !is_nan))

Better:

logic add_overflow;

assign add_overflow = add_result_exponent == 8’hFF && !is_nan;
if (is_infinity || add_overflow)

Use named constants rather than hardcoded, magic values to make code clearer and easier to understand.³

Okay:
```
if (load_address == MEMORY_BASE_ADDRESS)
```
No:
```
if (load_address == 'x123456)
```
Use one definition per line, and do not separate multiple definitions with commas. This makes the code more readable, and also makes it easier to search the code for definitions.

No:
```
logic[31:0] a, b;
logic foo,
      bar;
```
Okay:
```
logic[31:0] a;
logic[31:0] b;
logic foo;
logic bar;
```
Use one module per file, and name the file the same as the module. This makes it easier to find modules when navigating source and encourages reducing coupling between modules.
Connect ports and parameters by name and not position. Otherwise it is fragile and can break in subtle ways if a module is updated.

No:
```
my_module my_module(foo, bar, baz);
```
Yes:
```
my_module my_module(
    .foo(foo),
    .bar(bar),
    .baz(baz));
```
Modules tend to have the following order to make them easier to navigate:
- Type definitions and local parameters
- Signals (define all signals at the beginning of the module. Although Verilator doesn't complain about missing forward references, many other tools will raise errors).
- Combinational logic
- Sequential logic (usually registering the output and not the input)
Define global constants as parameters in defines.sv. For constants inside a module, use localparam. When a module sets a preprocessor variable, some tools make this visible in all subsequently compiled modules, which can cause namespace collisions. Using localparam restricts the scope to a single module. Using a constant parameter in defines.sv avoids conflicts when this chip is integrated into a design with unrelated IP, because it is wrapped in a namespace.
For non-generic components, make the instance name be the same as the component name. This makes it simpler to find the source module when viewing waveform traces

Yes:
```
writeback_stage writeback_stage(
```
No:
```
writeback_stage wback(
```
Use verilog-2001 style port definitions, specifying direction and type in one definition. This makes the sources more concise and captures all information about a signal in one place. Group module ports by source/destination and add a comment for each group.¹⁵ Prefix signals that go between non-generic components with an abbreviation of the source module. This makes it easier to search sources using automated tools and is easier to understand.
```
 // From io_request_queue
 input scalar_t                ior_read_value,
 input logic                   ior_rollback_en,

 // From control registers
 input scalar_t                cr_creg_read_val,
 input thread_bitmap_t         cr_interrupt_en,
 input scalar_t                cr_fault_handler,
```

Signals often use the following suffixes:

Suffix	Meaning
_en	Use for a signal that enables some operation. Internal enables are active high.
_oh	One-hot. No more than one signal will be set, corresponding to the index
_idx	Signal is an index. Usually used when one-hot signals of the same name are also present
_t	Typedef
_gen	Generated block
_nxt	Combinational logic that generates the next value (input) for a flop. Used to distinguish the input from the output of the flop

Do not use _i or _o to designate inputs and output ports. This is implied by the rule above that port names must match net names. Nets are neither inputs or outputs, as a single net connects to both.

Use AUTOLOGIC for module interconnections to avoid incorrectly inferring single bit nets.¹³

Name all generate blocks (with begin : <label>) so their logic will appear in VCD traces.¹³

generate
    for (byte_lane = 0; byte_lane < CACHE_LINE_BYTES; byte_lane++)
    begin : lane_bypass_gen
       ...
    end
endgenerate

Comments and Documentation

Comments are as important as code. While excessive or complex comments can be signal that code can be simplified, I believe the concept of "self documenting" code is a myth, especially for something as low level as HDL. The comments augment the source by clarifying intent and giving context. The code captures only a small part of a larger abstract mental model.

Useful things to comment:
- Side effects (for example, if this modifies some global state)
- Preconditions and constraints (what values are legal and which are not)
- Describe algorithms and heuristics used (link to external documents if these are common, describe in detail if not)
- Define what a module/block is responsible for and what it is not. This helps keep coupling and cohesion explicit in a design, and is something that is not always directly clear from the code.
- What state are things left in when an error occurs. What responsibilities does the client of this code/logic/module have in this case?
- For logic, is a signal registered/expected to be registered.
- When doing something unusual, describe why (e.g. optimizations, working around a tool bug)
- Describe the gist of what a block of statements is doing. This can encourage grouping logic with a more thoughtful structure. For example, if the intent of a few statements is to push something on a stack.

Eliminate tautologies and comments that add no information. Use comments to provide background on why a piece of code exists.

No:

state = 0; // reset state

Yes:

// This code path restarts the operation as a side effect, so
// reset the state machine
state = 0;

Eliminate comments if the code can be clarified with a more descriptive identifier (but keep identifiers concise and avoid trying to cram too much information into them).

No:
```
bytes++; // increment number of bytes read
```
Yes:
```
bytes_read++;
```
A constraint or invariant can often be better expressed and testing with an assertion, but usually still requires a comment to explain why it exists.

No:
```
// only one bit of way_hit_oh should be set
```
Yes:
```
// Make sure data is not present in more than one way.
assert($onehot0(way_hit_oh));
```
Avoid boilerplate comments. While consistency is a good thing to strive for, there is usually not enough in common between things to warrant it, so most of it ends up being blank. It also encourages laziness. Think about what a reader would want or need to know.
For short inline comments, use imperative mood, as if you are telling the program what to do. This is more direct and concise. Expository comments at the top of a module can have a more conversational tone.

No:
```
// Adds values together and writes back the result
```
Yes:
```
// Add the values together and write back result
```
Prefer present tense. Consistent tense is easier to understand, and present tense is more concise. It also avoids ambiguity as to whether something hasn't been implemented yet. This more applicable to longer comments, as the rule for using imperative voice implies using present tense.

No:
```
// This will handle interrupts
```
Yes:
```
// This handles interrupts
```
Prefer singular. When there are direct objects, this avoids ambiguity about the number (if the subject is plural, the object must be plural, which makes it is unclear whether there is a one-to-one or one-to-many relationship).

No:
```
// Arithmetic instructions update destination registers
```
Yes:
```
// An arithmetic instruction updates its destination register
```
Use third person to describe actions and events. The subject should be the program/hardware. State which subsystem/function is performing the action wherever possible. Use active voice.

No:
```
// We load a value from memory
// A value is loaded from memory
```
Yes:
```
// The cache loads a value from memory
```
Eliminate meaningless words and simplify phrases. e.g.
- Note that/It should be noted -> eliminate
- It is possible for X to -> X may
- Actually/Technically -> eliminate (this is true for most adverbs). "When you catch an adverb, kill it"²¹. Doing a search for 'ly' is a good way to find these.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDL Conventions

General

Clocks and Reset

Naming/Formatting

Comments and Documentation

References

Clone this wiki locally