diff --git a/MANUAL.md b/MANUAL.md index 17e8d89b..4628fa10 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -155,7 +155,7 @@ prefix_label = "lab_" style_indent = true [syntaxrules] -non_ansi_module = true +module_nonansi_forbidden = true keyword_forbidden_wire_reg = true ``` @@ -665,6 +665,100 @@ The most relevant clauses of IEEE1800-2017 are: +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `blocking_assignment_in_always_at_edge` + +### Hint + +Do not use blocking assignments within edge-sensitive `always`. + +### Reason + +Blocking assignment in `always_ff` may cause undefined event ordering. + +### Pass Example (1 of 3) +```systemverilog +module M; + always @(posedge clk) q <= d; +endmodule +``` + +### Pass Example (2 of 3) +```systemverilog +module M; + always @(negedge clk) q <= d; +endmodule +``` + +### Pass Example (3 of 3) +```systemverilog +module M; + always @(edge clk) q <= d; +endmodule +``` + +### Fail Example (1 of 3) +```systemverilog +module M; + always @(posedge clk) q = d; +endmodule +``` + +### Fail Example (2 of 3) +```systemverilog +module M; + always @(negedge clk) q = d; +endmodule +``` + +### Fail Example (3 of 3) +```systemverilog +module M; + always @(edge clk) q = d; +endmodule +``` + +### Explanation + +Simulator event ordering between blocking and non-blocking assignments +is undefined, so observed behavior is simulator-dependent. +Edge-sensitive (usually clocked) processes like, `always @(posedge clk)` should +only contain non-blocking assignments in order for sampling and variable +evaluation to operate in a defined order, e.g. `q <= d;`, not `q = d;`. + +For SystemVerilog (IEEE1800) code, the keyword `always_ff` (or `always_latch`) +should be used instead of the general purpose `always` to take advantage of +extra compile-time checks. +For code which must be compatible with Verilog (IEEE1364), `always` is the only +option. +Therefore, this rule `reg` assignments to be compatible with Verilog like this +(in conjunction with **non_blocking_assignment_in_always_no_edge**): +```verilog +always @(posedge clk) q <= d; // Clocked to reg (flip-flop) +always @* a = b + c; // Combinational to reg (logic gates) +assign d = e + f; // Combinational to wire (logic gates) +``` + +See also: +- **non_blocking_assignment_in_always_no_edge** - Useful companion rule. +- **blocking_assignment_in_always_ff** - Similar rule, suggested as alternative + for SystemVerilog code, but not Verilog. +- **blocking_assignment_in_always_latch** - Useful companion rule for + SystemVerilog, but not Verilog. +- **non_blocking_assignment_in_always_comb** - Useful companion rule for + SystemVerilog, but not Verilog. + +The most relevant clauses of IEEE1800-2017 are: +- 4.9.3 Blocking assignment +- 4.9.4 Non-blocking assignment +- 9.4.2 Event control +- 10.4.1 Blocking procedural assignments +- 10.4.2 Nonblocking procedural assignments +- 16.5.1 Sampling + + + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * ## Syntax Rule: `blocking_assignment_in_always_ff` @@ -691,11 +785,7 @@ endmodule ### Fail Example (1 of 1) ```systemverilog module M; -/* svlint off blocking_assignment_in_always_ff */ -always_ff @(posedge clk) q1 = d; // Control comments avoid failure. -/* svlint on blocking_assignment_in_always_ff */ - -always_ff @(posedge clk) q2 = d; // Failure. + always_ff @(posedge clk) q = d; // Failure. endmodule ``` @@ -853,7 +943,7 @@ When `foo` is non-zero, this example may be interpreted in at least two ways: See also: - **explicit_case_default** - Useful companion rule. - **explicit_if_else** - Useful companion rule. -- **legacy_always** - Useful companion rule. +- **keyword_forbidden_always** - Useful companion rule. - **sequential_block_in_always_comb** - Useful companion rule. The most relevant clauses of IEEE1800-2017 are: @@ -1115,7 +1205,7 @@ This rule only applies to event expressions in `always_ff` processes. See also: - **eventlist_or** - Mutually exclusive rule. - **blocking_assignment_in_always_ff** - Useful companion rule. -- **level_sensitive_always** - Useful companion rule. +- **general_always_no_edge** - Useful companion rule. - **style_keyword_1space** - Useful companion rule. The most relevant clauses of IEEE1800-2017 are: @@ -1242,7 +1332,7 @@ processes. See also: - **eventlist_comma_always_ff** - Mutually exclusive rule. - **blocking_assignment_in_always_ff** - Useful companion rule. -- **level_sensitive_always** - Useful companion rule. +- **general_always_no_edge** - Useful companion rule. - **style_keyword_commaleading** - Useful companion rule. The most relevant clauses of IEEE1800-2017 are: @@ -1309,7 +1399,7 @@ endmodule ### Explanation -The reasoning behind this rule are different between combinatial constructs +The reasoning behind this is are different between combinatial constructs (`always_comb`, `always @*`) vs sequential constructs (`always_ff`, `always_latch`). The reasoning behind this rule is equivalent to that of **explicit_if_else**. @@ -1330,12 +1420,12 @@ and clear through some useful redundancy. NOTE: The legacy keyword `always` can infer both combinational and sequential constructs in the same block, which can be confusing and should be avoided. -Use of the legacy keyword can be detected with the rule **legacy_always**. +Use of the legacy keyword can be detected with the rule **keyword_forbidden_always**. See also: - **case_default** - Useful companion rule. - **explicit_if_else** - Useful companion rule. -- **legacy_always** - Useful companion rule. +- **keyword_forbidden_always** - Useful companion rule. - **sequential_block_in_always_comb** - Useful companion rule. - **sequential_block_in_always_ff** - Useful companion rule. - **sequential_block_in_always_latch** - Useful companion rule. @@ -1414,11 +1504,11 @@ and clear through some useful redundancy. NOTE: The legacy keyword `always` can infer both combinational and sequential constructs in the same block, which can be confusing and should be avoided. -Use of the legacy keyword can be detected with the rule **legacy_always**. +Use of the legacy keyword can be detected with the rule **keyword_forbidden_always**. See also: - **explicit_case_default** - Useful companion rule. -- **legacy_always** - Useful companion rule. +- **keyword_forbidden_always** - Useful companion rule. - **sequential_block_in_always_comb** - Useful companion rule. - **sequential_block_in_always_ff** - Useful companion rule. - **sequential_block_in_always_latch** - Useful companion rule. @@ -1586,6 +1676,177 @@ The most relevant clauses of IEEE1800-2017 are: +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `general_always_level_sensitive` + +### Hint + +Replace general-purpose `always @(...no edge...)` with `always @*`. + +### Reason + +General-purpose `always` cannot detect combinatorial/stateful mistakes. + +### Pass Example (1 of 2) +```systemverilog +module M; + always @* // Sensitive to `b` and `c`. + a = b + c; +endmodule +``` + +### Pass Example (2 of 2) +```systemverilog +module M; + always @(posedge clk) // Sensitive to edges of `clk`. + q <= d; +endmodule +``` + +### Fail Example (1 of 2) +```systemverilog +module M; + always @(b) // Missing sensitivity to `c`. + a = b + c; +endmodule +``` + +### Fail Example (2 of 2) +```systemverilog +module M; + always @(a or b) // Correct sensitivity list, but error prone. + a = b + c; +endmodule +``` + +### Explanation + +This rule is specific to code which must be compatible with Verilog, not +only SystemVerilog. + +In Verilog (IEEE1364), there are two language constructs which can be used to +model combinatorial logic: +1. Continuous assignment to `wire` signals is specified with the `assign` + keyword. +2. `reg` signals are assigned to with an `always` block, which is evaluated + whenever anything in the sensitivity list changes value. + +To ensure that the process correctly sensitive to changes on all driving +signals, `always @*` should be used instead of providing an explicit +sensitivity list like `always @(a or b or c)`. +The `always` keyword can also be used for modelling sequential logic by +including the edge of a signal in the sensitivity list. +Providing an explicit sensitivity list is prone to two mistakes: +1. Forgetting to add a driver to the list, e.g. `always @(b) a = b + c;` + instead of `always @(b or c) a = b + c;`. +2. Forgetting to add and edge specifier, e.g. `always @(clk) q <= d;` instead + of `always @(posedge clk) q <= d;`. + That makes the process level-sensitive, instead of the edge-sensitive. + +This rule requires that general-purpose `always` blocks with an explicit +sensitivity list which include at least one edge. +Combinational logic should use the Kleen-star notation, +e.g. `always @* a = b + c;` + +See also: +- **keyword_forbidden_always** - Related rule forbidding general-purpose + `always`, only applicable for SystemVerilog code. +- **general_always_no_edge** - Related rule forbidding purely combinational + logic in `always` processes. + While this is straightforward to use with SystemVerilog, this might be overly + restrictive for Verilog because all combinational variables must be driven + with `assign`. + +The most relevant clauses of IEEE1800-2017 are: +- 9.2.2 Always procedures +- 9.5 Process execution threads + + + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `general_always_no_edge` + +### Hint + +Replace general-purpose `always` with `always_comb`. + +### Reason + +General-purpose `always` cannot detect combinatorial/stateful mistakes. + +### Pass Example (1 of 1) +```systemverilog +module M; + always_comb begin + end + always @(posedge a) begin + end +endmodule +``` + +### Fail Example (1 of 2) +```systemverilog +module M; + always @* begin // No sensitivity list. + end +endmodule +``` + +### Fail Example (2 of 2) +```systemverilog +module M; + always @(a or b) begin // No sensitivity to posedge, negedge, or edge. + end +endmodule +``` + +### Explanation + +In Verilog (IEEE1364), there are two language constructs which can be used to +model combinatorial logic: +1. Continuous assignment to `wire` signals is specified with the `assign` + keyword. +2. `reg` signals are assigned to with an `always` block, which is evaluated + whenever anything in the sensitivity list changes value. + +The `always` keyword can also be used for modelling sequential logic by +including the edge of a signal in the sensitivity list. + +The semantics of these keywords in SystemVerilog are compatible with Verilog, +but additional keywords (`always_comb`, `always_ff`, and `always_latch`) should +be used to clarify intent of digital designs. +The `always_*` keywords have slightly different semantics which are beneficial +for synthesizable designs: +1. `always_*` processes require compiler checks that any signals driven on the + LHS are not driven by any other process, i.e. `always_*` cannot infer + multi-driven or tri-state logic. +2. `always_comb` processes require a compiler check that the process does not + infer state. +3. `always_ff` processes require a compiler check that the process does infer + state. + +This rule requires that general-purpose `always` blocks have an explicit +sensitivity list which includes at least one edge, thus forcing the use of +`assign` or `always_comb` to specify combinatorial logic. +It is possible to construct a full-featured testbench where all `always` blocks +meet that requriment. +The alternative rule **keyword_forbidden_always** has similar reasoning but is +more strict, completely forbidding the use of general-purpose `always` blocks. +It is appropriate to use **keyword_forbidden_always** on synthesizable design +code, but on verification code use **general_always_no_edge** instead. + +See also: +- **keyword_forbidden_always** - Alternative rule. +- **general_always_no_edge** - Similar rule that allows `always @*`. + +The most relevant clauses of IEEE1800-2017 are: +- 9.2.2 Always procedures +- 9.5 Process execution threads + + + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * ## Syntax Rule: `genvar_declaration_in_loop` @@ -1946,7 +2207,7 @@ requires that each interface port includes a modport identifier. See also: - **inout_with_tri** - Useful companion rule. - **input_with_var** - Useful companion rule. -- **non_ansi_module** - Useful companion rule. +- **module_nonansi_forbidden** - Useful companion rule. - **output_with_var** - Useful companion rule. The most relevant clauses of IEEE1800-2017 are: @@ -2015,17 +2276,18 @@ process is a valid and useful way of scheduling events. Therefore, this rule is intended only for synthesizable design code, not for testbench code. -The alternative rule **level_sensitive_always** has similar reasoning but is +The alternative rule **general_always_no_edge** has similar reasoning but is slightly relaxed, requiring that `always` blocks have an explicit sensitivity list including an edge. It is possible to construct a full-featured testbench where all `always` blocks meet that requriment. Therefore, it is appropriate to use **keyword_forbidden_always** on synthesizable design code, but on verification code use -**level_sensitive_always** instead. +**general_always_no_edge** instead. See also: -- **level_sensitive_always** - Alternative rule. +- **general_always_no_edge** - Alternative rule. +- **general_always_level_sensitive** - Alternative rule. - **sequential_block_in_always_comb** - Useful companion rule. - **sequential_block_in_always_if** - Useful companion rule. - **sequential_block_in_always_latch** - Useful companion rule. @@ -2038,115 +2300,339 @@ The most relevant clauses of IEEE1800-2017 are: * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -## Syntax Rule: `keyword_forbidden_generate` +## Syntax Rule: `keyword_forbidden_always_comb` ### Hint -Remove `generate`/`endgenerate` keywords. +Use `always @*` instead of `always_comb`. ### Reason -Keywords `generate`/`endgenerate` do not change semantics of generate blocks. +Only SystemVerilog, not Verilog, has `always_comb`. ### Pass Example (1 of 1) ```systemverilog module M; + always @* z = x + y; endmodule ``` ### Fail Example (1 of 1) ```systemverilog module M; - generate - endgenerate + always_comb z = x + y; endmodule ``` ### Explanation -The `generate`/`endgenerate` keywords may be used in a module, interface, -program, or checker to define a generate region. -A generate region is a textual span in the module description where generate -constructs may appear. -Use of generate regions is optional. -There is no semantic difference in the module when a generate region is used. -A parser may choose to recognize the generate region to produce different error -messages for misused generate construct keywords. - -As the semantics of generate blocks are unchanged by the -`generate`/`endgenerate` keywords, the keywords can be argued to be visual -noise, simply distracting the reader. -Therefore, this rule is designed to detect and forbid their use. - -NOTE: Some non-compliant tools may require the use of these keywords, which -provides an argument against this rule. +The keywords `always_comb`, `always_ff`, and `always_latch` were added to +SystemVerilog (IEEE1800) to require extra safety checks at compile time. +Verilog (IEEE1364) only has `always`, which can describe equivalent behavior +but without the compile-time checks. +This rule requires `always @*` to be used instead of `always_comb` for +backwards compatibility with Verilog. See also: -- **keyword_required_generate** - Opposite reasoning. +- **keyword_forbidden_always_ff** - Suggested companion rule. +- **keyword_forbidden_always_latch** - Suggested companion rule. +- **keyword_forbidden_logic** - Suggested companion rule. +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **operator_incdec** - Suggested companion rule. +- **operator_self_assignment** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 9.9 Structured procedures The most relevant clauses of IEEE1800-2017 are: -- 27.3 Generate construct syntax +- 9.2 Structured procedures * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -## Syntax Rule: `keyword_forbidden_priority` +## Syntax Rule: `keyword_forbidden_always_ff` ### Hint -Remove `priority` keyword, perhaps replace with an assertion. +Use `always @(posedge clk)` instead of `always_ff @(posedge clk)`. ### Reason -Priority-case/if constructs may mismatch between simulation and synthesis. +Only SystemVerilog, not Verilog, has `always_ff`. ### Pass Example (1 of 1) ```systemverilog module M; - initial - case (a) - default: b = 1; - endcase -endmodule -``` - -### Fail Example (1 of 2) -```systemverilog -module M; - initial - priority case (a) - default: b = 1; - endcase + always @(posedge clk) + d <= q; endmodule ``` -### Fail Example (2 of 2) +### Fail Example (1 of 1) ```systemverilog module M; - initial - priority if (a) - b = 1; - else if (a) - b = 2; - else - b = 3; + always_ff @(posedge clk) + d <= q; endmodule ``` ### Explanation -The keyword `priority` may be used on `if`/`else` or `case` statements to -enable *violation checks* in simulation, and describe design intent for -synthesis. +The keywords `always_comb`, `always_ff`, and `always_latch` were added to +SystemVerilog (IEEE1800) to require extra safety checks at compile time. +Verilog (IEEE1364) only has `always`, which can describe equivalent behavior +but without the compile-time checks. +This rule requires something like `always @(posedge clk)` to be used instead of +`always_ff @(posedge clk)` for backwards compatibility with Verilog. -A `priority if` statement without an explicit `else` clause will produce a -*violation report* in simulation if the implicit `else` condition is matched. -A `priority if` statement with an explicit `else` clause cannot produce a -violation report. -In synthesis, the `priority` keyword makes no difference to an `if`/`else` -statement, because the semantics of bare `if`/`else` statements already imply -priority logic. +See also: +- **keyword_forbidden_always_comb** - Suggested companion rule. +- **keyword_forbidden_always_latch** - Suggested companion rule. +- **keyword_forbidden_logic** - Suggested companion rule. +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **operator_incdec** - Suggested companion rule. +- **operator_self_assignment** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 9.9 Structured procedures + +The most relevant clauses of IEEE1800-2017 are: +- 9.2 Structured procedures + + + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `keyword_forbidden_always_latch` + +### Hint + +Use `always @*` or `always @(en)` instead of `always_latch`. + +### Reason + +Only SystemVerilog, not Verilog, has `always_latch`. + +### Pass Example (1 of 2) +```systemverilog +module M; + always @* + if (en) + d <= q; +endmodule +``` + +### Pass Example (2 of 2) +```systemverilog +module M; + always @(en) + if (en) + d <= q; +endmodule +``` + +### Fail Example (1 of 1) +```systemverilog +module M; + always_latch + if (en) + d <= q; +endmodule +``` + +### Explanation + +The keywords `always_comb`, `always_ff`, and `always_latch` were added to +SystemVerilog (IEEE1800) to require extra safety checks at compile time. +Verilog (IEEE1364) only has `always`, which can describe equivalent behavior +but without the compile-time checks. +This rule requires `always @*` or something like `always @(en)` to be used +instead of `always_latch` for backwards compatibility with Verilog. + +See also: +- **keyword_forbidden_always_comb** - Suggested companion rule. +- **keyword_forbidden_always_ff** - Suggested companion rule. +- **keyword_forbidden_logic** - Suggested companion rule. +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **operator_incdec** - Suggested companion rule. +- **operator_self_assignment** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 9.9 Structured procedures + +The most relevant clauses of IEEE1800-2017 are: +- 9.2 Structured procedures + + + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `keyword_forbidden_generate` + +### Hint + +Remove `generate`/`endgenerate` keywords. + +### Reason + +Keywords `generate`/`endgenerate` do not change semantics of generate blocks. + +### Pass Example (1 of 1) +```systemverilog +module M; +endmodule +``` + +### Fail Example (1 of 1) +```systemverilog +module M; + generate + endgenerate +endmodule +``` + +### Explanation + +The `generate`/`endgenerate` keywords may be used in a module, interface, +program, or checker to define a generate region. +A generate region is a textual span in the module description where generate +constructs may appear. +Use of generate regions is optional. +There is no semantic difference in the module when a generate region is used. +A parser may choose to recognize the generate region to produce different error +messages for misused generate construct keywords. + +As the semantics of generate blocks are unchanged by the +`generate`/`endgenerate` keywords, the keywords can be argued to be visual +noise, simply distracting the reader. +Therefore, this rule is designed to detect and forbid their use. + +NOTE: Some non-compliant tools may require the use of these keywords, which +provides an argument against this rule. + +See also: +- **keyword_required_generate** - Opposite reasoning. + +The most relevant clauses of IEEE1800-2017 are: +- 27.3 Generate construct syntax + + + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `keyword_forbidden_logic` + +### Hint + +Replace `logic` keywords with `wire` or `reg`. + +### Reason + +Only SystemVerilog, not Verilog, has `logic`. + +### Pass Example (1 of 1) +```systemverilog +module M; + wire a; + reg b; +endmodule +``` + +### Fail Example (1 of 1) +```systemverilog +module M; + logic a; +endmodule +``` + +### Explanation + +The datatype `logic` was added to SystemVerilog (IEEE1800) to clarify +designer's intent, mostly replacing `wire` and fully replacing `reg`. +Verilog (IEEE1364) only has the `reg` bit-vector variable (and the various type +of nets). +This rule forbids `logic` for backwards compatibility with Verilog. + +See also: +- **keyword_forbidden_always_comb** - Suggested companion rule. +- **keyword_forbidden_always_ff** - Suggested companion rule. +- **keyword_forbidden_always_latch** - Suggested companion rule. +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **operator_incdec** - Suggested companion rule. +- **operator_self_assignment** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 3.2 Nets and variables +- 3.3 Vectors +- 3.7 Nets types +- 3.8 regs + +The most relevant clauses of IEEE1800-2017 are: +- 6.5 Nets and variables +- 6.5 Vector declarations +- 6.11 Integer data types + + + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `keyword_forbidden_priority` + +### Hint + +Remove `priority` keyword, perhaps replace with an assertion. + +### Reason + +Priority-case/if constructs may mismatch between simulation and synthesis. + +### Pass Example (1 of 1) +```systemverilog +module M; + initial + case (a) + default: b = 1; + endcase +endmodule +``` + +### Fail Example (1 of 2) +```systemverilog +module M; + initial + priority case (a) + default: b = 1; + endcase +endmodule +``` + +### Fail Example (2 of 2) +```systemverilog +module M; + initial + priority if (a) + b = 1; + else if (a) + b = 2; + else + b = 3; +endmodule +``` + +### Explanation + +The keyword `priority` may be used on `if`/`else` or `case` statements to +enable *violation checks* in simulation, and describe design intent for +synthesis. + +A `priority if` statement without an explicit `else` clause will produce a +*violation report* in simulation if the implicit `else` condition is matched. +A `priority if` statement with an explicit `else` clause cannot produce a +violation report. +In synthesis, the `priority` keyword makes no difference to an `if`/`else` +statement, because the semantics of bare `if`/`else` statements already imply +priority logic. A `priority case` statement without a `default` arm will produce a violation report in simulation if the `default` condition is matched. @@ -2510,139 +2996,57 @@ The most relevant clauses of IEEE1800-2017 are: * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -## Syntax Rule: `level_sensitive_always` +## Syntax Rule: `localparam_explicit_type` ### Hint -Replace level-sensitive `always` with `always_comb`. +Provide an explicit type in `localparam` declaration. ### Reason -Level-sensitive `always` cannot detect combinatorial/stateful (non-)blocking mistakes. +Explicit parameter types clarify intent and improve readability. ### Pass Example (1 of 1) ```systemverilog module M; - always_comb begin - end - always @(posedge a) begin - end -endmodule -``` - -### Fail Example (1 of 2) -```systemverilog -module M; - always @* begin // No sensitivity list. - end + localparam int A = 0; endmodule ``` -### Fail Example (2 of 2) +### Fail Example (1 of 1) ```systemverilog module M; - always @ (a or b) begin // No sensitivity to posedge, negedge, or edge. - end + localparam L = 0; endmodule ``` ### Explanation -In Verilog (IEEE1364), there are two language constructs which can be used to -model combinatorial logic: -1. Continuous assignment to `wire` signals is specified with the `assign` - keyword. -2. `reg` signals are assigned to with an `always` block, which is evaluated - whenever anything in the sensitivity list changes value. +The type of a parameter is more fundmental to express intent than its value. +By analogy, asking a shopkeeper for "5 oranges" is more likely to be correctly +understood than simply asking for "5" without clarification. +This rule requires that authors consider and specify the type of each +`localparam` elaboration-time constant. +Explicit types help readers to understand exactly what effects the constant +might have, thus reducing the effort they need to expend reading how the +parameter is used. -The `always` keyword can also be used for modelling sequential logic by -including the edge of a signal in the sensitivity list. +Without an explicit type, a localparam will take a type compatible with its +constant expression. +Implict types can thereby introduce discrepencies between what the author +intends and how tools interpret the code. +For example, interactions between the default datatype `logic`, constant +functions, and case expressions can result in mismatches between simulation and +synthesis. +A detailed investigation into the semantics of implicit vs explicit types +on SystemVerilog `parameter` and `localparam`s can be found in a tutorial +paper here: + -The semantics of these keywords in SystemVerilog are compatible with Verilog, -but additional keywords (`always_comb`, `always_ff`, and `always_latch`) should -be used to clarify intent of digital designs. -The `always_*` keywords have slightly different semantics which are beneficial -for synthesizable designs: -1. `always_*` processes require compiler checks that any signals driven on the - LHS are not driven by any other process, i.e. `always_*` cannot infer - multi-driven or tri-state logic. -2. `always_comb` processes require a compiler check that the process does not - infer state. -3. `always_ff` processes require a compiler check that the process does infer - state. - -This rule requires that general-purpose `always` blocks have an explicit -sensitivity list which includes at least one edge, thus forcing the use of -`assign` or `always_comb` to specify combinatorial logic. -It is possible to construct a full-featured testbench where all `always` blocks -meet that requriment. -The alternative rule **keyword_forbidden_always** has similar reasoning but is -more strict, completely forbidding the use of general-purpose `always` blocks. -It is appropriate to use **keyword_forbidden_always** on synthesizable design -code, but on verification code use **level_sensitive_always** instead. - -See also: -- **keyword_forbidden_always** - Alternative rule. - -The most relevant clauses of IEEE1800-2017 are: -- 9.2.2 Always procedures -- 9.5 Process execution threads - - - -* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * - -## Syntax Rule: `localparam_explicit_type` - -### Hint - -Provide an explicit type in `localparam` declaration. - -### Reason - -Explicit parameter types clarify intent and improve readability. - -### Pass Example (1 of 1) -```systemverilog -module M; - localparam int A = 0; -endmodule -``` - -### Fail Example (1 of 1) -```systemverilog -module M; - localparam L = 0; -endmodule -``` - -### Explanation - -The type of a parameter is more fundmental to express intent than its value. -By analogy, asking a shopkeeper for "5 oranges" is more likely to be correctly -understood than simply asking for "5" without clarification. -This rule requires that authors consider and specify the type of each -`localparam` elaboration-time constant. -Explicit types help readers to understand exactly what effects the constant -might have, thus reducing the effort they need to expend reading how the -parameter is used. - -Without an explicit type, a localparam will take a type compatible with its -constant expression. -Implict types can thereby introduce discrepencies between what the author -intends and how tools interpret the code. -For example, interactions between the default datatype `logic`, constant -functions, and case expressions can result in mismatches between simulation and -synthesis. -A detailed investigation into the semantics of implicit vs explicit types -on SystemVerilog `parameter` and `localparam`s can be found in a tutorial -paper here: - - -See also: -- **localparam_type_twostate** - Useful companion rule. -- **parameter_explicit_type** - Useful companion rule. -- **parameter_type_twostate** - Useful companion rule. +See also: +- **localparam_type_twostate** - Useful companion rule. +- **parameter_explicit_type** - Useful companion rule. +- **parameter_type_twostate** - Useful companion rule. The most relevant clauses of IEEE1800-2017 are: - 6.3 Value set @@ -3111,6 +3515,176 @@ The most relevant clauses of IEEE1800-2017 are: +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `module_ansi_forbidden` + +### Hint + +Declare `module` header in non-ANSI style. + +### Reason + +Only SystemVerilog, not Verilog, allows `localparam` in ANSI module header. + +### Pass Example (1 of 2) +```systemverilog +module M + ( a + , b + ); + input a; // Declaring ports outside the module header declaration + output b; // makes this a non-ANSI module. +endmodule +``` + +### Pass Example (2 of 2) +```systemverilog +module M; // A module with no portlist is ANSI, but allowed. +endmodule +``` + +### Fail Example (1 of 3) +```systemverilog +module M // An ANSI module has ports declared in the module header. + ( input a + , output b + ); +endmodule +``` + +### Fail Example (2 of 3) +```systemverilog +module M // Declaring ports in the header with default direction (inout) + ( a // also specifies an ANSI module where directions are not given + , b // later. + ); +endmodule +``` + +### Fail Example (3 of 3) +```systemverilog +module M + (); // A module with an empty portlist is ANSI. +endmodule +``` + +### Explanation + +There are two ways to declare a module header in SystemVerilog: +1. ANSI style - newer, neater, more succinct, mostly compatible with + IEEE1364-2001 (as long as you don't use `localparam`s for ports). +2. non-ANSI style - additionally compatible with older Verilog (IEEE1364-1995). + +Examples of both styles are given in IEEE1364-2001 (e.g. pages 180 vs 182) and +IEEE1800-2017 (e.g. pages 702 vs 700). + +The non-ANSI style separates the declaration of ports, their direction, and +their datatype. +While requiring more text, and visual noise, to convey the same information, +the non-ANSI style allows non-overridable parameters, i.e. `localparam`, to be +used in port declarations. +If only `parameter` is used instead, as allowed in IEEE1364, an instance may +inadvertently override a parameter, thus causing difficult-to-debug issues. + +This rule requires that module headers are declared using the non-ANSI style. +It is recommended only to use this rule where compatibility with IEEE1364 is +required. +By forbidding the ANSI style, this rule requires that module declarations are +written in a consistent manner, which facilitates easier review and prevents +easily overlooked issues before they become problems. + +See also: +- **module_nonansi_forbidden** - For safer usability where compatibility with + Verilog is not required. + +The most relevant clauses of IEEE1364-2001 are: +- 12.1 Modules +- 12.2 Overriding module parameter values + +The most relevant clauses of IEEE1800-2017 are: +- 23.2 Module definitions + + + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `module_nonansi_forbidden` + +### Hint + +Declare `module` header in ANSI style. + +### Reason + +Non-ANSI module headers are visually noisy and error-prone. + +### Pass Example (1 of 3) +```systemverilog +module M // An ANSI module has ports declared in the module header. + ( input a + , output b + ); +endmodule +``` + +### Pass Example (2 of 3) +```systemverilog +module M; // A module with no ports is also ANSI. +endmodule +``` + +### Pass Example (3 of 3) +```systemverilog +module M // Declaring ports in the header with default direction (inout) + ( a // also specifies an ANSI module. + , b + ); +endmodule +``` + +### Fail Example (1 of 1) +```systemverilog +module M + ( a + , b + ); + input a; // Declaring ports outside the module header declaration + output b; // makes this a non-ANSI module. +endmodule +``` + +### Explanation + +There are two ways to declare a module header in SystemVerilog: +1. ANSI style - newer, neater, more succinct, mostly compatible with + IEEE1364-2001 (as long as you don't use `localparam`s for ports). +2. non-ANSI style - additionally compatible with older Verilog (IEEE1364-1995). + +Examples of both styles are given in IEEE1364-2001 (e.g. pages 180 vs 182) and +IEEE1800-2017 (e.g. pages 702 vs 700). + +The non-ANSI style separates the declaration of ports, their direction, and +their datatype. +In addition to requiring more text, and visual noise, to convey the same +information, the non-ANSI style encourages simple coding mistakes where +essential attributes may be forgotten. +This rule requires that module headers are declared using the ANSI style. + +See also: +- **module_ansi_forbidden** - For consistency in IEEE1364-2001 (compatibility + with non-overridable parameters, i.e. `localparam`, in port declarations, + or compatibility with IEEE1364-1995. + +The most relevant clauses of IEEE1364-2001 are: +- 12.1 Modules +- 12.2 Overriding module parameter values + +The most relevant clauses of IEEE1800-2017 are: +- 23.2 Module definitions + + + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * ## Syntax Rule: `multiline_for_begin` @@ -3214,258 +3788,535 @@ module M; endmodule ``` -### Fail Example (1 of 5) +### Fail Example (1 of 5) +```systemverilog +module M; + always_comb + if (a) + a = 0; // Missing begin/end. +endmodule +``` + +### Fail Example (2 of 5) +```systemverilog +module M; + always_comb + if (a) begin + a = 0; + end else if (a) + a = 0; // Missing begin/end. +endmodule +``` + +### Fail Example (3 of 5) +```systemverilog +module M; + always_comb + if (a) begin + a = 0; + end else if (a) begin + a = 0; + end else + a = 0; // Missing begin/end. +endmodule +``` + +### Fail Example (4 of 5) +```systemverilog +module M; + always_comb begin + if (a) + a = 0; // Missing begin/end. + end +endmodule +``` + +### Fail Example (5 of 5) +```systemverilog +module M; + always_comb begin + if (a) a = 0; // This conditional statement is okay. + else if (a) a = 0; + else a = 0; + + if (a) // Check all if-statements, not only the first. + a = 0; // Missing begin/end. + end +endmodule +``` + +### Explanation + +This rule is to help prevent a common class of coding mistake, where a future +maintainer attempts to add further statements to the conditional block, but +accidentally writes something different. + +See also: +- **multiline_for_begin** - Useful companion rule. +- **style_indent** - Useful companion rule. + +The most relevant clauses of IEEE1800-2017 are: +- 12.4 Conditional if-else statement + + + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `non_blocking_assignment_in_always_comb` + +### Hint + +Remove non-blocking assignment in `always_comb`. + +### Reason + +Scheduling between blocking and non-blocking assignments is non-deterministic. + +### Pass Example (1 of 1) +```systemverilog +module M; + always_comb + x = 0; +endmodule +``` + +### Fail Example (1 of 1) +```systemverilog +module M; + always_comb + x <= 0; +endmodule +``` + +### Explanation + +Simulator event ordering between blocking and non-blocking assignments +is undefined, so observed behavior is simulator-dependent. +This rule forbids the use of non-blocking assigments (using the `<=` operator) +in `always_comb` blocks. +Instead, use the blocking assignment operator `=`. + +An excellent paper detailing the semantics of Verilog blocking and non-blocking +assignments is written by Clifford E Cummings and presented at SNUG-2000, +"Nonblocking Assignments in Verilog Synthesis, Coding Styles that Kill". + +See also: +- **blocking_assignment_in_always_ff** - Useful companion rule. +- **blocking_assignment_in_always_latch** - Useful companion rule. + +The most relevant clauses of IEEE1800-2017 are: +- 4.9.3 Blocking assignment +- 4.9.4 Non-blocking assignment +- 9.2.2.2 Combinational logic `always_comb` procedure +- 9.4.2 Event control +- 10.4.1 Blocking procedural assignments +- 10.4.2 Nonblocking procedural assignments + + + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `non_blocking_assignment_in_always_no_edge` + +### Hint + +Remove non-blocking assignment in combinational `always`. + +### Reason + +Scheduling between blocking and non-blocking assignments is non-deterministic. + +### Pass Example (1 of 2) +```systemverilog +module M; + always @* a = b + c; +endmodule +``` + +### Pass Example (2 of 2) +```systemverilog +module M; + always @(*) a = b + c; +endmodule +``` + +### Fail Example (1 of 2) +```systemverilog +module M; + always @* a <= b + c; +endmodule +``` + +### Fail Example (2 of 2) +```systemverilog +module M; + always @(*) a <= b + c; +endmodule +``` + +### Explanation + +Simulator event ordering between blocking and non-blocking assignments +is undefined, so observed behavior is simulator-dependent. +Value-sensitive (usually combinatorial) processes like, `always @*` +should only contain blocking assignments in order for sampling and variable +evaluation to operate in a defined order, e.g. `a = b;`, not `a <= b;`. + +For SystemVerilog (IEEE1800) code, the keyword `always_comb` should be used +instead of the general purpose `always` to take advantage of extra compile-time +checks. +For code which must be compatible with Verilog (IEEE1364), `always` is the only +option. +Therefore, this rule `reg` assignments to be compatible with Verilog like this +(in conjunction with **blocking_assignment_in_always_at_edge**): +```verilog +always @(posedge clk) q <= d; // Clocked to reg (flip-flop) +always @* a = b + c; // Combinational to reg (logic gates) +assign d = e + f; // Combinational to wire (logic gates) +``` + +See also: +- **non_blocking_assignment_in_always_at_edge** - Useful companion rule. +- **blocking_assignment_in_always_ff** - Similar rule, suggested as alternative + for SystemVerilog code, but not Verilog. +- **blocking_assignment_in_always_latch** - Useful companion rule for + SystemVerilog, but not Verilog. +- **non_blocking_assignment_in_always_comb** - Useful companion rule for + SystemVerilog, but not Verilog. + +The most relevant clauses of IEEE1800-2017 are: +- 4.9.3 Blocking assignment +- 4.9.4 Non-blocking assignment +- 9.4.2 Event control +- 10.4.1 Blocking procedural assignments +- 10.4.2 Nonblocking procedural assignments +- 16.5.1 Sampling + + + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `operator_case_equality` + +### Hint + +Use logical equality instead of case equality. + +### Reason + +Case equality operations are not generally synthesizable. + +### Pass Example (1 of 1) +```systemverilog +module M; + always_latch if (a == b) z = y; + + always_comb z = (a != b) ? y : x; + + always_latch if (a ==? b) z = y; + + always_comb z = (a !=? b) ? y : x; +endmodule +``` + +### Fail Example (1 of 2) +```systemverilog +module M; + always_latch if (a === b) z = y; +endmodule +``` + +### Fail Example (2 of 2) +```systemverilog +module M; + always_comb z = (a !== b) ? y : x; +endmodule +``` + +### Explanation + +Case equality operations (using `===` or `!==` operators) include comparison +against `'z` or `'x`, so they are not generally synthesisable. +Synthesizable code should use logical or wildcard equality operations instead. + +See also: +- **case_default** - Useful companion rule. +- **explicit_case_default** - Useful companion rule. +- **enum_with_type** - Useful companion rule. +- **localparam_type_twostate** - Useful companion rule. +- **parameter_type_twostate** - Useful companion rule. + +The most relevant clauses of IEEE1800-2017 are: +- 11.4.5 Equality operators +- 11.4.6 Wildcard quality operators + + + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `operator_incdec` + +### Hint + +Use `=` with a `+` or `-` instead of an increment or decrement operator. + +### Reason + +Only SystemVerilog, not Verilog, has increment and decrement operators. + +### Pass Example (1 of 6) +```systemverilog +module M; + always @(posedge clk) z = z - 1; +endmodule +``` + +### Pass Example (2 of 6) ```systemverilog module M; - always_comb - if (a) - a = 0; // Missing begin/end. + always @(posedge clk) z = z + 1; endmodule ``` -### Fail Example (2 of 5) +### Pass Example (3 of 6) ```systemverilog module M; - always_comb - if (a) begin - a = 0; - end else if (a) - a = 0; // Missing begin/end. + always @* z = z - 1; endmodule ``` -### Fail Example (3 of 5) +### Pass Example (4 of 6) ```systemverilog module M; - always_comb - if (a) begin - a = 0; - end else if (a) begin - a = 0; - end else - a = 0; // Missing begin/end. + always @* z = z + 1; endmodule ``` -### Fail Example (4 of 5) +### Pass Example (5 of 6) ```systemverilog module M; - always_comb begin - if (a) - a = 0; // Missing begin/end. + genvar i; + for (i = 4; i >= 0; i = i - 1) begin + assign z[i] = y[i] + x[i]; end endmodule ``` -### Fail Example (5 of 5) +### Pass Example (6 of 6) ```systemverilog module M; - always_comb begin - if (a) a = 0; // This conditional statement is okay. - else if (a) a = 0; - else a = 0; - - if (a) // Check all if-statements, not only the first. - a = 0; // Missing begin/end. + genvar i; + for (i = 0; i < 5; i = i + 1) begin + assign z[i] = y[i] + x[i]; end endmodule ``` -### Explanation - -This rule is to help prevent a common class of coding mistake, where a future -maintainer attempts to add further statements to the conditional block, but -accidentally writes something different. - -See also: -- **multiline_for_begin** - Useful companion rule. -- **style_indent** - Useful companion rule. - -The most relevant clauses of IEEE1800-2017 are: -- 12.4 Conditional if-else statement - - - -* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * - -## Syntax Rule: `non_ansi_module` - -### Hint - -Declare `module` header in ANSI style. - -### Reason +### Fail Example (1 of 6) +```systemverilog +module M; + always @(posedge clk) z--; +endmodule +``` -Non-ANSI module headers are visually noisy and error-prone. +### Fail Example (2 of 6) +```systemverilog +module M; + always @(posedge clk) z++; +endmodule +``` -### Pass Example (1 of 3) +### Fail Example (3 of 6) ```systemverilog -module M // An ANSI module has ports declared in the module header. - ( input a - , output b - ); +module M; + always @* z = x + y--; endmodule ``` -### Pass Example (2 of 3) +### Fail Example (4 of 6) ```systemverilog -module M; // A module with no ports is also ANSI. +module M; + always @* z = x + y++; endmodule ``` -### Pass Example (3 of 3) +### Fail Example (5 of 6) ```systemverilog -module M // Declaring ports in the header with default direction (inout) - ( a // also specifies an ANSI module. - , b - ); +module M; + genvar i; + for (i = 4; i >= 0; i--) begin + assign z[i] = y[i] + x[i]; + end endmodule ``` -### Fail Example (1 of 1) +### Fail Example (6 of 6) ```systemverilog -module M - ( a - , b - ); - input a; // Declaring ports outside the module header declaration - output b; // makes this a non-ANSI module. +module M; + genvar i; + for (i = 0; i < 5; i++) begin + assign z[i] = y[i] + x[i]; + end endmodule ``` ### Explanation -There are two ways to declare a module header in SystemVerilog: -1. ANSI style - newer, neater, more succinct, compatible with IEEE1364-2001. -2. non-ANSI style - additionally compatible with older Verilog (IEEE1364-1995). - -Examples of both styles are given in IEEE1364-2001 (e.g. pages 180 vs 182) and -IEEE1800-2017 (e.g. pages 702 vs 700). +Increment and decrement operators (`++` and `--`) are part of SystemVerilog +(IEEE1800), but not Verilog (IEEE1364). -The non-ANSI style separates the declaration of ports, their direction, and -their datatype. -In addition to requiring more text, and visual noise, to convey the same -information, the non-ANSI style encourages simple coding mistakes where -essential attributes may be forgotten. -This rule requires that module headers are declared using the ANSI style. +This rule allows only binary operators with simple assigments (`foo = foo + 1`) +to encourage backwards compatibility with Verilog. See also: -- No related rules. +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **keyword_forbidden_always_comb** - Suggested companion rule. +- **keyword_forbidden_always_ff** - Suggested companion rule. +- **keyword_forbidden_always_latch** - Suggested companion rule. +- **keyword_forbidden_logic** - Suggested companion rule. +- **operator_self_assignment** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 4.1 Operators +- 9.2.1 Blocking procedural assignments +- 12.1.3.2 generate-loop The most relevant clauses of IEEE1800-2017 are: -- 23.2 Module definitions +- 10.4.1 Blocking procedural assignments +- 11.4.2 Increment and decrement operators +- 27.4 Loop generate constructs * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -## Syntax Rule: `non_blocking_assignment_in_always_comb` +## Syntax Rule: `operator_self_assignment` ### Hint -Remove non-blocking assignment in `always_comb`. +Use `=` with a binary operator instead of a self-assignment operator. ### Reason -Scheduling between blocking and non-blocking assignments is non-deterministic. +Only SystemVerilog, not Verilog, allows self-assignment operators. ### Pass Example (1 of 1) ```systemverilog module M; - always_comb - x = 0; + always @* + if (a == b) // Logical-equality operator is not an assignment. + z = y; // Simple assignment operator is allowed. endmodule ``` -### Fail Example (1 of 1) +### Fail Example (1 of 12) ```systemverilog module M; - always_comb - x <= 0; + always @* z += y; // Addition `z = z + y` endmodule ``` -### Explanation - -Simulator event ordering between blocking and non-blocking assignments -is undefined, so observed behavior is simulator-dependent. -This rule forbids the use of non-blocking assigments (using the `<=` operator) -in `always_comb` blocks. -Instead, use the blocking assignment operator `=`. - -An excellent paper detailing the semantics of Verilog blocking and non-blocking -assignments is written by Clifford E Cummings and presented at SNUG-2000, -"Nonblocking Assignments in Verilog Synthesis, Coding Styles that Kill". - -See also: -- **blocking_assignment_in_always_ff** - Useful companion rule. -- **blocking_assignment_in_always_latch** - Useful companion rule. - -The most relevant clauses of IEEE1800-2017 are: -- 4.9.3 Blocking assignment -- 4.9.4 Non-blocking assignment -- 9.2.2.2 Combinational logic `always_comb` procedure -- 9.4.2 Event control -- 10.4.1 Blocking procedural assignments -- 10.4.2 Nonblocking procedural assignments - - - -* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * - -## Syntax Rule: `operator_case_equality` +### Fail Example (2 of 12) +```systemverilog +module M; + always @* z -= y; // Subtraction `z = z - y` +endmodule +``` -### Hint +### Fail Example (3 of 12) +```systemverilog +module M; + always @* z *= y; // Multiplication `z = z * y` +endmodule +``` -Use logical equality instead of case equality. +### Fail Example (4 of 12) +```systemverilog +module M; + always @* z /= y; // Division `z = z / y` +endmodule +``` -### Reason +### Fail Example (5 of 12) +```systemverilog +module M; + always @* z %= y; // Modulo `z = z % y` +endmodule +``` -Case equality operations are not generally synthesizable. +### Fail Example (6 of 12) +```systemverilog +module M; + always @* z &= y; // Bitwise AND `z = z & y` +endmodule +``` -### Pass Example (1 of 1) +### Fail Example (7 of 12) ```systemverilog module M; - always_latch if (a == b) z = y; + always @* z |= y; // Bitwise OR `z = z | y` +endmodule +``` - always_comb z = (a != b) ? y : x; +### Fail Example (8 of 12) +```systemverilog +module M; + always @* z ^= y; // Bitwise XOR `z = z ^ y` +endmodule +``` - always_latch if (a ==? b) z = y; +### Fail Example (9 of 12) +```systemverilog +module M; + always @* z <<= y; // Logical left shift `z = z << y` +endmodule +``` - always_comb z = (a !=? b) ? y : x; +### Fail Example (10 of 12) +```systemverilog +module M; + always @* z >>= y; // Logical right shift `z = z >> y` endmodule ``` -### Fail Example (1 of 2) +### Fail Example (11 of 12) ```systemverilog module M; - always_latch if (a === b) z = y; + always @* z <<<= y; // Arithmetic left shift `z = z <<< y` endmodule ``` -### Fail Example (2 of 2) +### Fail Example (12 of 12) ```systemverilog module M; - always_comb z = (a !== b) ? y : x; + always @* z >>>= y; // Arithmetic right shift `z = z >>> y` endmodule ``` ### Explanation -Case equality operations (using `===` or `!==` operators) include comparison -against `'z` or `'x`, so they are not generally synthesisable. -Synthesizable code should use logical or wildcard equality operations instead. +Self-assignment operators (`+=`, `-=`, `*=`, `/=`, `%=`, `&=`, `|=`, `^=`, +`<<=`, `>>=`, `<<<=`, and `>>>=`) are part of SystemVerilog (IEEE1800), but not +Verilog (IEEE1364). + +This rule allows only simple assigment (using `=`) to encourage backwards +compatibility with Verilog. See also: -- **case_default** - Useful companion rule. -- **explicit_case_default** - Useful companion rule. -- **enum_with_type** - Useful companion rule. -- **localparam_type_twostate** - Useful companion rule. -- **parameter_type_twostate** - Useful companion rule. +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **keyword_forbidden_always_comb** - Suggested companion rule. +- **keyword_forbidden_always_ff** - Suggested companion rule. +- **keyword_forbidden_always_latch** - Suggested companion rule. +- **keyword_forbidden_logic** - Suggested companion rule. +- **operator_incdec** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 4.1 Operators +- 9.2.1 Blocking procedural assignments The most relevant clauses of IEEE1800-2017 are: -- 11.4.5 Equality operators -- 11.4.6 Wildcard quality operators +- 10.4.1 Blocking procedural assignments +- 11.4.1 Assignment operators @@ -4481,7 +5332,7 @@ endmodule ### Fail Example (1 of 3) ```systemverilog module M; - for (genvar i=0; i < 10; i++) // No begin/end delimeters. + for (genvar i=0; i < 10; i++) // No begin/end delimiters. assign a[i] = i; endmodule ``` @@ -4567,7 +5418,7 @@ endmodule ### Fail Example (1 of 8) ```systemverilog module M; - if (x) // No begin/end delimeters. + if (x) // No begin/end delimiters. assign a = 0; // if condition. else if (x) begin: l_def assign a = 1; @@ -4582,7 +5433,7 @@ endmodule module M; if (x) begin: l_abc assign a = 0; - end else if (x) // No begin/end delimeters. + end else if (x) // No begin/end delimiters. assign a = 1; // else-if condition. else begin: l_hij assign a = 2; @@ -4595,7 +5446,7 @@ module M; assign a = 0; end else if (x) begin: l_def assign a = 1; - end else // No begin/end delimeters. + end else // No begin/end delimiters. assign a = 2; // else condition endmodule ``` @@ -5800,7 +6651,7 @@ See also: - **prefix_module** - **uppercamelcase_module** - **lowercamelcase_module** -- **non_ansi_module** +- **module_nonansi_forbidden** @@ -5852,7 +6703,7 @@ See also: - **prefix_module** - **uppercamelcase_module** - **lowercamelcase_module** -- **non_ansi_module** +- **module_nonansi_forbidden** @@ -7027,7 +7878,7 @@ See also: - **prefix_module** - **uppercamelcase_module** - **lowercamelcase_module** -- **non_ansi_module** +- **module_nonansi_forbidden** @@ -7079,7 +7930,7 @@ See also: - **prefix_module** - **uppercamelcase_module** - **lowercamelcase_module** -- **non_ansi_module** +- **module_nonansi_forbidden** @@ -9771,8 +10622,8 @@ If instead the `--config` option was used in wrapper scripts, this could lead to confusion where TOML files exist elsewhere in the hierarchy. It isn't essential for all ruleset scripts to be POSIX compliant, but POSIX -compliance should be encourage because it allows for consistent behavior across -the widest range of systems. +compliance should be encouraged because it allows for consistent behavior +across the widest range of systems. The utilities used in the POSIX wrappers are specified in the current POSIX standard (IEEE1003.1-2017, Volume 3: Shell and Utilities). Some resources related to these components: @@ -10216,7 +11067,7 @@ syntaxrules.function_with_automatic = true syntaxrules.keyword_forbidden_priority = true syntaxrules.keyword_forbidden_unique = true syntaxrules.keyword_forbidden_unique0 = true -syntaxrules.level_sensitive_always = true +syntaxrules.general_always_no_edge = true syntaxrules.operator_case_equality = true # Common to **ruleset-designintent**. @@ -10225,7 +11076,7 @@ syntaxrules.default_nettype_none = true syntaxrules.function_same_as_system_function = true syntaxrules.keyword_forbidden_always = true syntaxrules.keyword_forbidden_wire_reg = true -syntaxrules.non_ansi_module = true +syntaxrules.module_nonansi_forbidden = true ``` Generally, elaboration-time constants (`parameter`, `localparam`) should be @@ -10952,7 +11803,7 @@ syntaxrules.function_with_automatic = true syntaxrules.keyword_forbidden_priority = true syntaxrules.keyword_forbidden_unique = true syntaxrules.keyword_forbidden_unique0 = true -syntaxrules.level_sensitive_always = true # Redundant with keyword_forbidden_always. +#syntaxrules.general_always_no_edge = true # Redundant with keyword_forbidden_always. syntaxrules.operator_case_equality = true ``` @@ -10969,7 +11820,7 @@ syntaxrules.default_nettype_none = true syntaxrules.function_same_as_system_function = true syntaxrules.keyword_forbidden_always = true syntaxrules.keyword_forbidden_wire_reg = true -syntaxrules.non_ansi_module = true +syntaxrules.module_nonansi_forbidden = true ``` When synthesised into a netlist, generate blocks should have labels so that @@ -11058,6 +11909,136 @@ syntaxrules.interface_port_with_modport = true +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Ruleset: *designintentV2001* + +This ruleset has the same aims as **ruleset-designintent** but with the +additional aim of only allowing code which is backwards compatible with +IEEE1364-2001 (Verilog). +Note that IEEE1364-2001 is not the most recent version (IEEE1364-2005), which +was released in the same year as the first version of SystemVerilog +(IEEE1800-2005). + +Firstly, let's forbid some things which are only in SystemVerilog, but not +Verilog. +```toml +syntaxrules.keyword_forbidden_always_comb = true +syntaxrules.keyword_forbidden_always_ff = true +syntaxrules.keyword_forbidden_always_latch = true +syntaxrules.keyword_forbidden_priority = true +syntaxrules.keyword_forbidden_unique = true +syntaxrules.keyword_forbidden_unique0 = true +syntaxrules.keyword_forbidden_logic = true +syntaxrules.operator_incdec = true +syntaxrules.operator_self_assignment = true +``` + +Next, let's use some of the rules in common with **ruleset-simsynth**. +```toml +syntaxrules.enum_with_type = true +syntaxrules.function_with_automatic = true +syntaxrules.operator_case_equality = true +syntaxrules.action_block_with_side_effect = true +syntaxrules.default_nettype_none = true +syntaxrules.function_same_as_system_function = true +``` + +Verilog does allow both ANSI and non-ANSI forms of module declaration, but +there is a crucial difference for the ANSI form: +Only `parameter`s are allowed in the list of parameter ports, not +`localparam`s, meaning that derived parameters are overridable. +In the following example, there is no way of preventing `PTR_W` from being +overridden to something incorrect, risking some frustration and wasted time +when non-obvious effects cause issues later. +```verilog +module M + #(parameter integer WIDTH = 123 + , parameter integer PTR_W = clogb2(WIDTH) + ) + ( input wire [WIDTH-1:0] i_data + , output wire [PTR_W-1:0] o_pointer + ); +``` +However, using the non-ANSI form allows `PTR_W` to be specified as +`localparam`, thus preventing overrides and the resulting confusion, i.e: +```verilog +module M + ( i_data + , o_pointer + ); + + parameter integer WIDTH = 123; + localparam integer PTR_W = clogb2(WIDTH); + + input wire [WIDTH-1:0] i_data; + output wire [PTR_W-1:0] o_pointer; +``` +While this only affects modules which use derived parameters in the port +declarations, a consistent style is generally easier to work with. +For these reasons, the non-ANSI form is required. +```toml +syntaxrules.module_ansi_forbidden = true +``` + +SystemVerilog introduced several keywords which greatly help to clarify intent, +but these are unavailable. +Instead of `always_ff @(posedge clk)` and `always_comb`, we can use +`always @(posedge clk)` and `always @*`. +That means only the form like `always @(a or b)`, i.e. no edge sensitivities, +can be forbidden. +```toml +syntaxrules.general_always_level_sensitive = true +``` +On the same theme, guidelines around blocking vs non-blocking assignments also +need to be altered, but keeping the same general intention. +Clocked `always` processes should only use non-blocking assignment `<=`, and +combinatorial `always` processes should only use blocking assignment `=`. +```toml +syntaxrules.blocking_assignment_in_always_at_edge = true +syntaxrules.non_blocking_assignment_in_always_no_edge = true +``` + +Verilog doesn't have the same distinction between 2-state and 4-state types as +SystemVerilog, e.g. `int` and `integer`, but requiring some type is still a +good idea. +```toml +syntaxrules.localparam_explicit_type = true +syntaxrules.parameter_explicit_type = true +syntaxrules.parameter_default_value = true +syntaxrules.parameter_in_generate = true +``` + +In IEEE1364-2001, the use of `generate` and `endgenerate` is mandatory, but +optional in IEEE1364-2005. +For more compatibility, these keywords are required by this ruleset, as are +`genvar` declarations outside their generate `for` statements. +The enablements of these rules are swapped in **ruleset-designintent** to +reduce visual noise in SystemVerilog. +```toml +syntaxrules.genvar_declaration_in_loop = false +syntaxrules.genvar_declaration_out_loop = true +syntaxrules.keyword_forbidden_generate = false +syntaxrules.keyword_required_generate = true +``` + +Unlike the in the richer language of SystemVerilog, forbidding sequential +blocks (between `begin` and `end`) and sequential loops (`for` under `always`) +is probably too restrictive for Verilog. +Indeed, there is little point in using `always @*` instead of `assign` if +`begin` and `end` are forbidden - in SystemVerilog, `always_comb` provides +extra compile-time checks that `assign` does not. +```toml +#syntaxrules.loop_statement_in_always = true # Not implemented. +#syntaxrules.sequential_block_in_always = true # Not implemented. +syntaxrules.case_default = true # Applies in functions. +syntaxrules.explicit_case_default = true # Applies under `always`. +syntaxrules.explicit_if_else = true +syntaxrules.multiline_for_begin = true +syntaxrules.multiline_if_begin = true +``` + + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * ## Ruleset: *parseonly* @@ -11105,7 +12086,7 @@ syntaxrules.function_with_automatic = true syntaxrules.keyword_forbidden_priority = true syntaxrules.keyword_forbidden_unique = true syntaxrules.keyword_forbidden_unique0 = true -syntaxrules.level_sensitive_always = true +syntaxrules.general_always_no_edge = true syntaxrules.operator_case_equality = true ``` @@ -11446,7 +12427,7 @@ syntaxrules.action_block_with_side_effect = true syntaxrules.default_nettype_none = true syntaxrules.function_same_as_system_function = true syntaxrules.keyword_forbidden_wire_reg = true -syntaxrules.non_ansi_module = true +syntaxrules.module_nonansi_forbidden = true ``` Generally, elaboration-time constant (`parameter`, `localparam`) should be diff --git a/build.rs b/build.rs index e4e98f46..5761149f 100644 --- a/build.rs +++ b/build.rs @@ -54,6 +54,16 @@ const RENAMED_SYNTAXRULES: &[(&str, &str, &str)] = &[ "keyword_required_generate", "KeywordRequiredGenerate", ), + ( + "non_ansi_module", + "module_nonansi_forbidden", + "ModuleNonansiForbidden", + ), + ( + "level_sensitive_always", + "general_always_no_edge", + "GeneralAlwaysNoEdge", + ), ]; fn write_rules_rs( diff --git a/md/manual-introduction.md b/md/manual-introduction.md index 80e3e48c..9b7da746 100644 --- a/md/manual-introduction.md +++ b/md/manual-introduction.md @@ -155,7 +155,7 @@ prefix_label = "lab_" style_indent = true [syntaxrules] -non_ansi_module = true +module_nonansi_forbidden = true keyword_forbidden_wire_reg = true ``` diff --git a/md/manual-rulesets.md b/md/manual-rulesets.md index 90292522..3adaf11f 100644 --- a/md/manual-rulesets.md +++ b/md/manual-rulesets.md @@ -137,8 +137,8 @@ If instead the `--config` option was used in wrapper scripts, this could lead to confusion where TOML files exist elsewhere in the hierarchy. It isn't essential for all ruleset scripts to be POSIX compliant, but POSIX -compliance should be encourage because it allows for consistent behavior across -the widest range of systems. +compliance should be encouraged because it allows for consistent behavior +across the widest range of systems. The utilities used in the POSIX wrappers are specified in the current POSIX standard (IEEE1003.1-2017, Volume 3: Shell and Utilities). Some resources related to these components: diff --git a/md/ruleset-DaveMcEwan-design.md b/md/ruleset-DaveMcEwan-design.md index c98c2a55..2343de11 100644 --- a/md/ruleset-DaveMcEwan-design.md +++ b/md/ruleset-DaveMcEwan-design.md @@ -366,7 +366,7 @@ syntaxrules.function_with_automatic = true syntaxrules.keyword_forbidden_priority = true syntaxrules.keyword_forbidden_unique = true syntaxrules.keyword_forbidden_unique0 = true -syntaxrules.level_sensitive_always = true +syntaxrules.general_always_no_edge = true syntaxrules.operator_case_equality = true # Common to **ruleset-designintent**. @@ -375,7 +375,7 @@ syntaxrules.default_nettype_none = true syntaxrules.function_same_as_system_function = true syntaxrules.keyword_forbidden_always = true syntaxrules.keyword_forbidden_wire_reg = true -syntaxrules.non_ansi_module = true +syntaxrules.module_nonansi_forbidden = true ``` Generally, elaboration-time constants (`parameter`, `localparam`) should be diff --git a/md/ruleset-designintent.md b/md/ruleset-designintent.md index 26391b9a..0ae1b7ad 100644 --- a/md/ruleset-designintent.md +++ b/md/ruleset-designintent.md @@ -15,7 +15,7 @@ syntaxrules.function_with_automatic = true syntaxrules.keyword_forbidden_priority = true syntaxrules.keyword_forbidden_unique = true syntaxrules.keyword_forbidden_unique0 = true -syntaxrules.level_sensitive_always = true # Redundant with keyword_forbidden_always. +#syntaxrules.general_always_no_edge = true # Redundant with keyword_forbidden_always. syntaxrules.operator_case_equality = true ``` @@ -32,7 +32,7 @@ syntaxrules.default_nettype_none = true syntaxrules.function_same_as_system_function = true syntaxrules.keyword_forbidden_always = true syntaxrules.keyword_forbidden_wire_reg = true -syntaxrules.non_ansi_module = true +syntaxrules.module_nonansi_forbidden = true ``` When synthesised into a netlist, generate blocks should have labels so that diff --git a/md/ruleset-designintentV2001.md b/md/ruleset-designintentV2001.md new file mode 100644 index 00000000..2da58a15 --- /dev/null +++ b/md/ruleset-designintentV2001.md @@ -0,0 +1,125 @@ + +This ruleset has the same aims as **ruleset-designintent** but with the +additional aim of only allowing code which is backwards compatible with +IEEE1364-2001 (Verilog). +Note that IEEE1364-2001 is not the most recent version (IEEE1364-2005), which +was released in the same year as the first version of SystemVerilog +(IEEE1800-2005). + +Firstly, let's forbid some things which are only in SystemVerilog, but not +Verilog. +```toml +syntaxrules.keyword_forbidden_always_comb = true +syntaxrules.keyword_forbidden_always_ff = true +syntaxrules.keyword_forbidden_always_latch = true +syntaxrules.keyword_forbidden_priority = true +syntaxrules.keyword_forbidden_unique = true +syntaxrules.keyword_forbidden_unique0 = true +syntaxrules.keyword_forbidden_logic = true +syntaxrules.operator_incdec = true +syntaxrules.operator_self_assignment = true +``` + +Next, let's use some of the rules in common with **ruleset-simsynth**. +```toml +syntaxrules.enum_with_type = true +syntaxrules.function_with_automatic = true +syntaxrules.operator_case_equality = true +syntaxrules.action_block_with_side_effect = true +syntaxrules.default_nettype_none = true +syntaxrules.function_same_as_system_function = true +``` + +Verilog does allow both ANSI and non-ANSI forms of module declaration, but +there is a crucial difference for the ANSI form: +Only `parameter`s are allowed in the list of parameter ports, not +`localparam`s, meaning that derived parameters are overridable. +In the following example, there is no way of preventing `PTR_W` from being +overridden to something incorrect, risking some frustration and wasted time +when non-obvious effects cause issues later. +```verilog +module M + #(parameter integer WIDTH = 123 + , parameter integer PTR_W = clogb2(WIDTH) + ) + ( input wire [WIDTH-1:0] i_data + , output wire [PTR_W-1:0] o_pointer + ); +``` +However, using the non-ANSI form allows `PTR_W` to be specified as +`localparam`, thus preventing overrides and the resulting confusion, i.e: +```verilog +module M + ( i_data + , o_pointer + ); + + parameter integer WIDTH = 123; + localparam integer PTR_W = clogb2(WIDTH); + + input wire [WIDTH-1:0] i_data; + output wire [PTR_W-1:0] o_pointer; +``` +While this only affects modules which use derived parameters in the port +declarations, a consistent style is generally easier to work with. +For these reasons, the non-ANSI form is required. +```toml +syntaxrules.module_ansi_forbidden = true +``` + +SystemVerilog introduced several keywords which greatly help to clarify intent, +but these are unavailable. +Instead of `always_ff @(posedge clk)` and `always_comb`, we can use +`always @(posedge clk)` and `always @*`. +That means only the form like `always @(a or b)`, i.e. no edge sensitivities, +can be forbidden. +```toml +syntaxrules.general_always_level_sensitive = true +``` +On the same theme, guidelines around blocking vs non-blocking assignments also +need to be altered, but keeping the same general intention. +Clocked `always` processes should only use non-blocking assignment `<=`, and +combinatorial `always` processes should only use blocking assignment `=`. +```toml +syntaxrules.blocking_assignment_in_always_at_edge = true +syntaxrules.non_blocking_assignment_in_always_no_edge = true +``` + +Verilog doesn't have the same distinction between 2-state and 4-state types as +SystemVerilog, e.g. `int` and `integer`, but requiring some type is still a +good idea. +```toml +syntaxrules.localparam_explicit_type = true +syntaxrules.parameter_explicit_type = true +syntaxrules.parameter_default_value = true +syntaxrules.parameter_in_generate = true +``` + +In IEEE1364-2001, the use of `generate` and `endgenerate` is mandatory, but +optional in IEEE1364-2005. +For more compatibility, these keywords are required by this ruleset, as are +`genvar` declarations outside their generate `for` statements. +The enablements of these rules are swapped in **ruleset-designintent** to +reduce visual noise in SystemVerilog. +```toml +syntaxrules.genvar_declaration_in_loop = false +syntaxrules.genvar_declaration_out_loop = true +syntaxrules.keyword_forbidden_generate = false +syntaxrules.keyword_required_generate = true +``` + +Unlike the in the richer language of SystemVerilog, forbidding sequential +blocks (between `begin` and `end`) and sequential loops (`for` under `always`) +is probably too restrictive for Verilog. +Indeed, there is little point in using `always @*` instead of `assign` if +`begin` and `end` are forbidden - in SystemVerilog, `always_comb` provides +extra compile-time checks that `assign` does not. +```toml +#syntaxrules.loop_statement_in_always = true # Not implemented. +#syntaxrules.sequential_block_in_always = true # Not implemented. +syntaxrules.case_default = true # Applies in functions. +syntaxrules.explicit_case_default = true # Applies under `always`. +syntaxrules.explicit_if_else = true +syntaxrules.multiline_for_begin = true +syntaxrules.multiline_if_begin = true +``` diff --git a/md/ruleset-simsynth.md b/md/ruleset-simsynth.md index 7bdd9a29..ed76646b 100644 --- a/md/ruleset-simsynth.md +++ b/md/ruleset-simsynth.md @@ -16,7 +16,7 @@ syntaxrules.function_with_automatic = true syntaxrules.keyword_forbidden_priority = true syntaxrules.keyword_forbidden_unique = true syntaxrules.keyword_forbidden_unique0 = true -syntaxrules.level_sensitive_always = true +syntaxrules.general_always_no_edge = true syntaxrules.operator_case_equality = true ``` diff --git a/md/ruleset-verifintent.md b/md/ruleset-verifintent.md index 58e55b20..1a0b69c8 100644 --- a/md/ruleset-verifintent.md +++ b/md/ruleset-verifintent.md @@ -30,7 +30,7 @@ syntaxrules.action_block_with_side_effect = true syntaxrules.default_nettype_none = true syntaxrules.function_same_as_system_function = true syntaxrules.keyword_forbidden_wire_reg = true -syntaxrules.non_ansi_module = true +syntaxrules.module_nonansi_forbidden = true ``` Generally, elaboration-time constant (`parameter`, `localparam`) should be diff --git a/md/syntaxrules-explanation-blocking_assignment_in_always_at_edge.md b/md/syntaxrules-explanation-blocking_assignment_in_always_at_edge.md new file mode 100644 index 00000000..e66130a0 --- /dev/null +++ b/md/syntaxrules-explanation-blocking_assignment_in_always_at_edge.md @@ -0,0 +1,35 @@ +Simulator event ordering between blocking and non-blocking assignments +is undefined, so observed behavior is simulator-dependent. +Edge-sensitive (usually clocked) processes like, `always @(posedge clk)` should +only contain non-blocking assignments in order for sampling and variable +evaluation to operate in a defined order, e.g. `q <= d;`, not `q = d;`. + +For SystemVerilog (IEEE1800) code, the keyword `always_ff` (or `always_latch`) +should be used instead of the general purpose `always` to take advantage of +extra compile-time checks. +For code which must be compatible with Verilog (IEEE1364), `always` is the only +option. +Therefore, this rule `reg` assignments to be compatible with Verilog like this +(in conjunction with **non_blocking_assignment_in_always_no_edge**): +```verilog +always @(posedge clk) q <= d; // Clocked to reg (flip-flop) +always @* a = b + c; // Combinational to reg (logic gates) +assign d = e + f; // Combinational to wire (logic gates) +``` + +See also: +- **non_blocking_assignment_in_always_no_edge** - Useful companion rule. +- **blocking_assignment_in_always_ff** - Similar rule, suggested as alternative + for SystemVerilog code, but not Verilog. +- **blocking_assignment_in_always_latch** - Useful companion rule for + SystemVerilog, but not Verilog. +- **non_blocking_assignment_in_always_comb** - Useful companion rule for + SystemVerilog, but not Verilog. + +The most relevant clauses of IEEE1800-2017 are: +- 4.9.3 Blocking assignment +- 4.9.4 Non-blocking assignment +- 9.4.2 Event control +- 10.4.1 Blocking procedural assignments +- 10.4.2 Nonblocking procedural assignments +- 16.5.1 Sampling diff --git a/md/syntaxrules-explanation-case_default.md b/md/syntaxrules-explanation-case_default.md index e0f5d267..f0aa688b 100644 --- a/md/syntaxrules-explanation-case_default.md +++ b/md/syntaxrules-explanation-case_default.md @@ -19,7 +19,7 @@ When `foo` is non-zero, this example may be interpreted in at least two ways: See also: - **explicit_case_default** - Useful companion rule. - **explicit_if_else** - Useful companion rule. -- **legacy_always** - Useful companion rule. +- **keyword_forbidden_always** - Useful companion rule. - **sequential_block_in_always_comb** - Useful companion rule. The most relevant clauses of IEEE1800-2017 are: diff --git a/md/syntaxrules-explanation-eventlist_comma_always_ff.md b/md/syntaxrules-explanation-eventlist_comma_always_ff.md index 83320055..6d3762ef 100644 --- a/md/syntaxrules-explanation-eventlist_comma_always_ff.md +++ b/md/syntaxrules-explanation-eventlist_comma_always_ff.md @@ -33,7 +33,7 @@ This rule only applies to event expressions in `always_ff` processes. See also: - **eventlist_or** - Mutually exclusive rule. - **blocking_assignment_in_always_ff** - Useful companion rule. -- **level_sensitive_always** - Useful companion rule. +- **general_always_no_edge** - Useful companion rule. - **style_keyword_1space** - Useful companion rule. The most relevant clauses of IEEE1800-2017 are: diff --git a/md/syntaxrules-explanation-eventlist_or.md b/md/syntaxrules-explanation-eventlist_or.md index 7da148b0..4c54d2c5 100644 --- a/md/syntaxrules-explanation-eventlist_or.md +++ b/md/syntaxrules-explanation-eventlist_or.md @@ -31,7 +31,7 @@ processes. See also: - **eventlist_comma_always_ff** - Mutually exclusive rule. - **blocking_assignment_in_always_ff** - Useful companion rule. -- **level_sensitive_always** - Useful companion rule. +- **general_always_no_edge** - Useful companion rule. - **style_keyword_commaleading** - Useful companion rule. The most relevant clauses of IEEE1800-2017 are: diff --git a/md/syntaxrules-explanation-explicit_case_default.md b/md/syntaxrules-explanation-explicit_case_default.md index 0e6001b6..48653b90 100644 --- a/md/syntaxrules-explanation-explicit_case_default.md +++ b/md/syntaxrules-explanation-explicit_case_default.md @@ -1,4 +1,4 @@ -The reasoning behind this rule are different between combinatial constructs +The reasoning behind this is are different between combinatial constructs (`always_comb`, `always @*`) vs sequential constructs (`always_ff`, `always_latch`). The reasoning behind this rule is equivalent to that of **explicit_if_else**. @@ -19,12 +19,12 @@ and clear through some useful redundancy. NOTE: The legacy keyword `always` can infer both combinational and sequential constructs in the same block, which can be confusing and should be avoided. -Use of the legacy keyword can be detected with the rule **legacy_always**. +Use of the legacy keyword can be detected with the rule **keyword_forbidden_always**. See also: - **case_default** - Useful companion rule. - **explicit_if_else** - Useful companion rule. -- **legacy_always** - Useful companion rule. +- **keyword_forbidden_always** - Useful companion rule. - **sequential_block_in_always_comb** - Useful companion rule. - **sequential_block_in_always_ff** - Useful companion rule. - **sequential_block_in_always_latch** - Useful companion rule. diff --git a/md/syntaxrules-explanation-explicit_if_else.md b/md/syntaxrules-explanation-explicit_if_else.md index bd91059f..9a2f9bf3 100644 --- a/md/syntaxrules-explanation-explicit_if_else.md +++ b/md/syntaxrules-explanation-explicit_if_else.md @@ -17,11 +17,11 @@ and clear through some useful redundancy. NOTE: The legacy keyword `always` can infer both combinational and sequential constructs in the same block, which can be confusing and should be avoided. -Use of the legacy keyword can be detected with the rule **legacy_always**. +Use of the legacy keyword can be detected with the rule **keyword_forbidden_always**. See also: - **explicit_case_default** - Useful companion rule. -- **legacy_always** - Useful companion rule. +- **keyword_forbidden_always** - Useful companion rule. - **sequential_block_in_always_comb** - Useful companion rule. - **sequential_block_in_always_ff** - Useful companion rule. - **sequential_block_in_always_latch** - Useful companion rule. diff --git a/md/syntaxrules-explanation-general_always_level_sensitive.md b/md/syntaxrules-explanation-general_always_level_sensitive.md new file mode 100644 index 00000000..269e9826 --- /dev/null +++ b/md/syntaxrules-explanation-general_always_level_sensitive.md @@ -0,0 +1,39 @@ +This rule is specific to code which must be compatible with Verilog, not +only SystemVerilog. + +In Verilog (IEEE1364), there are two language constructs which can be used to +model combinatorial logic: +1. Continuous assignment to `wire` signals is specified with the `assign` + keyword. +2. `reg` signals are assigned to with an `always` block, which is evaluated + whenever anything in the sensitivity list changes value. + +To ensure that the process correctly sensitive to changes on all driving +signals, `always @*` should be used instead of providing an explicit +sensitivity list like `always @(a or b or c)`. +The `always` keyword can also be used for modelling sequential logic by +including the edge of a signal in the sensitivity list. +Providing an explicit sensitivity list is prone to two mistakes: +1. Forgetting to add a driver to the list, e.g. `always @(b) a = b + c;` + instead of `always @(b or c) a = b + c;`. +2. Forgetting to add and edge specifier, e.g. `always @(clk) q <= d;` instead + of `always @(posedge clk) q <= d;`. + That makes the process level-sensitive, instead of the edge-sensitive. + +This rule requires that general-purpose `always` blocks with an explicit +sensitivity list which include at least one edge. +Combinational logic should use the Kleen-star notation, +e.g. `always @* a = b + c;` + +See also: +- **keyword_forbidden_always** - Related rule forbidding general-purpose + `always`, only applicable for SystemVerilog code. +- **general_always_no_edge** - Related rule forbidding purely combinational + logic in `always` processes. + While this is straightforward to use with SystemVerilog, this might be overly + restrictive for Verilog because all combinational variables must be driven + with `assign`. + +The most relevant clauses of IEEE1800-2017 are: +- 9.2.2 Always procedures +- 9.5 Process execution threads diff --git a/md/syntaxrules-explanation-level_sensitive_always.md b/md/syntaxrules-explanation-general_always_no_edge.md similarity index 93% rename from md/syntaxrules-explanation-level_sensitive_always.md rename to md/syntaxrules-explanation-general_always_no_edge.md index f03f0ab2..5c4e6d48 100644 --- a/md/syntaxrules-explanation-level_sensitive_always.md +++ b/md/syntaxrules-explanation-general_always_no_edge.md @@ -29,10 +29,11 @@ meet that requriment. The alternative rule **keyword_forbidden_always** has similar reasoning but is more strict, completely forbidding the use of general-purpose `always` blocks. It is appropriate to use **keyword_forbidden_always** on synthesizable design -code, but on verification code use **level_sensitive_always** instead. +code, but on verification code use **general_always_no_edge** instead. See also: - **keyword_forbidden_always** - Alternative rule. +- **general_always_no_edge** - Similar rule that allows `always @*`. The most relevant clauses of IEEE1800-2017 are: - 9.2.2 Always procedures diff --git a/md/syntaxrules-explanation-interface_port_with_modport.md b/md/syntaxrules-explanation-interface_port_with_modport.md index 018f56b3..7dfdb13a 100644 --- a/md/syntaxrules-explanation-interface_port_with_modport.md +++ b/md/syntaxrules-explanation-interface_port_with_modport.md @@ -20,7 +20,7 @@ requires that each interface port includes a modport identifier. See also: - **inout_with_tri** - Useful companion rule. - **input_with_var** - Useful companion rule. -- **non_ansi_module** - Useful companion rule. +- **module_nonansi_forbidden** - Useful companion rule. - **output_with_var** - Useful companion rule. The most relevant clauses of IEEE1800-2017 are: diff --git a/md/syntaxrules-explanation-keyword_forbidden_always.md b/md/syntaxrules-explanation-keyword_forbidden_always.md index 78c4167f..5583c572 100644 --- a/md/syntaxrules-explanation-keyword_forbidden_always.md +++ b/md/syntaxrules-explanation-keyword_forbidden_always.md @@ -28,17 +28,18 @@ process is a valid and useful way of scheduling events. Therefore, this rule is intended only for synthesizable design code, not for testbench code. -The alternative rule **level_sensitive_always** has similar reasoning but is +The alternative rule **general_always_no_edge** has similar reasoning but is slightly relaxed, requiring that `always` blocks have an explicit sensitivity list including an edge. It is possible to construct a full-featured testbench where all `always` blocks meet that requriment. Therefore, it is appropriate to use **keyword_forbidden_always** on synthesizable design code, but on verification code use -**level_sensitive_always** instead. +**general_always_no_edge** instead. See also: -- **level_sensitive_always** - Alternative rule. +- **general_always_no_edge** - Alternative rule. +- **general_always_level_sensitive** - Alternative rule. - **sequential_block_in_always_comb** - Useful companion rule. - **sequential_block_in_always_if** - Useful companion rule. - **sequential_block_in_always_latch** - Useful companion rule. diff --git a/md/syntaxrules-explanation-keyword_forbidden_always_comb.md b/md/syntaxrules-explanation-keyword_forbidden_always_comb.md new file mode 100644 index 00000000..dc3f526d --- /dev/null +++ b/md/syntaxrules-explanation-keyword_forbidden_always_comb.md @@ -0,0 +1,20 @@ +The keywords `always_comb`, `always_ff`, and `always_latch` were added to +SystemVerilog (IEEE1800) to require extra safety checks at compile time. +Verilog (IEEE1364) only has `always`, which can describe equivalent behavior +but without the compile-time checks. +This rule requires `always @*` to be used instead of `always_comb` for +backwards compatibility with Verilog. + +See also: +- **keyword_forbidden_always_ff** - Suggested companion rule. +- **keyword_forbidden_always_latch** - Suggested companion rule. +- **keyword_forbidden_logic** - Suggested companion rule. +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **operator_incdec** - Suggested companion rule. +- **operator_self_assignment** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 9.9 Structured procedures + +The most relevant clauses of IEEE1800-2017 are: +- 9.2 Structured procedures diff --git a/md/syntaxrules-explanation-keyword_forbidden_always_ff.md b/md/syntaxrules-explanation-keyword_forbidden_always_ff.md new file mode 100644 index 00000000..a729619a --- /dev/null +++ b/md/syntaxrules-explanation-keyword_forbidden_always_ff.md @@ -0,0 +1,20 @@ +The keywords `always_comb`, `always_ff`, and `always_latch` were added to +SystemVerilog (IEEE1800) to require extra safety checks at compile time. +Verilog (IEEE1364) only has `always`, which can describe equivalent behavior +but without the compile-time checks. +This rule requires something like `always @(posedge clk)` to be used instead of +`always_ff @(posedge clk)` for backwards compatibility with Verilog. + +See also: +- **keyword_forbidden_always_comb** - Suggested companion rule. +- **keyword_forbidden_always_latch** - Suggested companion rule. +- **keyword_forbidden_logic** - Suggested companion rule. +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **operator_incdec** - Suggested companion rule. +- **operator_self_assignment** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 9.9 Structured procedures + +The most relevant clauses of IEEE1800-2017 are: +- 9.2 Structured procedures diff --git a/md/syntaxrules-explanation-keyword_forbidden_always_latch.md b/md/syntaxrules-explanation-keyword_forbidden_always_latch.md new file mode 100644 index 00000000..2ec9d25b --- /dev/null +++ b/md/syntaxrules-explanation-keyword_forbidden_always_latch.md @@ -0,0 +1,20 @@ +The keywords `always_comb`, `always_ff`, and `always_latch` were added to +SystemVerilog (IEEE1800) to require extra safety checks at compile time. +Verilog (IEEE1364) only has `always`, which can describe equivalent behavior +but without the compile-time checks. +This rule requires `always @*` or something like `always @(en)` to be used +instead of `always_latch` for backwards compatibility with Verilog. + +See also: +- **keyword_forbidden_always_comb** - Suggested companion rule. +- **keyword_forbidden_always_ff** - Suggested companion rule. +- **keyword_forbidden_logic** - Suggested companion rule. +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **operator_incdec** - Suggested companion rule. +- **operator_self_assignment** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 9.9 Structured procedures + +The most relevant clauses of IEEE1800-2017 are: +- 9.2 Structured procedures diff --git a/md/syntaxrules-explanation-keyword_forbidden_logic.md b/md/syntaxrules-explanation-keyword_forbidden_logic.md new file mode 100644 index 00000000..248b0ac8 --- /dev/null +++ b/md/syntaxrules-explanation-keyword_forbidden_logic.md @@ -0,0 +1,24 @@ +The datatype `logic` was added to SystemVerilog (IEEE1800) to clarify +designer's intent, mostly replacing `wire` and fully replacing `reg`. +Verilog (IEEE1364) only has the `reg` bit-vector variable (and the various type +of nets). +This rule forbids `logic` for backwards compatibility with Verilog. + +See also: +- **keyword_forbidden_always_comb** - Suggested companion rule. +- **keyword_forbidden_always_ff** - Suggested companion rule. +- **keyword_forbidden_always_latch** - Suggested companion rule. +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **operator_incdec** - Suggested companion rule. +- **operator_self_assignment** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 3.2 Nets and variables +- 3.3 Vectors +- 3.7 Nets types +- 3.8 regs + +The most relevant clauses of IEEE1800-2017 are: +- 6.5 Nets and variables +- 6.5 Vector declarations +- 6.11 Integer data types diff --git a/md/syntaxrules-explanation-module_ansi_forbidden.md b/md/syntaxrules-explanation-module_ansi_forbidden.md new file mode 100644 index 00000000..78f94a9f --- /dev/null +++ b/md/syntaxrules-explanation-module_ansi_forbidden.md @@ -0,0 +1,33 @@ +There are two ways to declare a module header in SystemVerilog: +1. ANSI style - newer, neater, more succinct, mostly compatible with + IEEE1364-2001 (as long as you don't use `localparam`s for ports). +2. non-ANSI style - additionally compatible with older Verilog (IEEE1364-1995). + +Examples of both styles are given in IEEE1364-2001 (e.g. pages 180 vs 182) and +IEEE1800-2017 (e.g. pages 702 vs 700). + +The non-ANSI style separates the declaration of ports, their direction, and +their datatype. +While requiring more text, and visual noise, to convey the same information, +the non-ANSI style allows non-overridable parameters, i.e. `localparam`, to be +used in port declarations. +If only `parameter` is used instead, as allowed in IEEE1364, an instance may +inadvertently override a parameter, thus causing difficult-to-debug issues. + +This rule requires that module headers are declared using the non-ANSI style. +It is recommended only to use this rule where compatibility with IEEE1364 is +required. +By forbidding the ANSI style, this rule requires that module declarations are +written in a consistent manner, which facilitates easier review and prevents +easily overlooked issues before they become problems. + +See also: +- **module_nonansi_forbidden** - For safer usability where compatibility with + Verilog is not required. + +The most relevant clauses of IEEE1364-2001 are: +- 12.1 Modules +- 12.2 Overriding module parameter values + +The most relevant clauses of IEEE1800-2017 are: +- 23.2 Module definitions diff --git a/md/syntaxrules-explanation-non_ansi_module.md b/md/syntaxrules-explanation-module_nonansi_forbidden.md similarity index 61% rename from md/syntaxrules-explanation-non_ansi_module.md rename to md/syntaxrules-explanation-module_nonansi_forbidden.md index cfc0a9b4..555b33b5 100644 --- a/md/syntaxrules-explanation-non_ansi_module.md +++ b/md/syntaxrules-explanation-module_nonansi_forbidden.md @@ -1,5 +1,6 @@ There are two ways to declare a module header in SystemVerilog: -1. ANSI style - newer, neater, more succinct, compatible with IEEE1364-2001. +1. ANSI style - newer, neater, more succinct, mostly compatible with + IEEE1364-2001 (as long as you don't use `localparam`s for ports). 2. non-ANSI style - additionally compatible with older Verilog (IEEE1364-1995). Examples of both styles are given in IEEE1364-2001 (e.g. pages 180 vs 182) and @@ -13,7 +14,13 @@ essential attributes may be forgotten. This rule requires that module headers are declared using the ANSI style. See also: -- No related rules. +- **module_ansi_forbidden** - For consistency in IEEE1364-2001 (compatibility + with non-overridable parameters, i.e. `localparam`, in port declarations, + or compatibility with IEEE1364-1995. + +The most relevant clauses of IEEE1364-2001 are: +- 12.1 Modules +- 12.2 Overriding module parameter values The most relevant clauses of IEEE1800-2017 are: - 23.2 Module definitions diff --git a/md/syntaxrules-explanation-non_blocking_assignment_in_always_no_edge.md b/md/syntaxrules-explanation-non_blocking_assignment_in_always_no_edge.md new file mode 100644 index 00000000..012a8ee7 --- /dev/null +++ b/md/syntaxrules-explanation-non_blocking_assignment_in_always_no_edge.md @@ -0,0 +1,35 @@ +Simulator event ordering between blocking and non-blocking assignments +is undefined, so observed behavior is simulator-dependent. +Value-sensitive (usually combinatorial) processes like, `always @*` +should only contain blocking assignments in order for sampling and variable +evaluation to operate in a defined order, e.g. `a = b;`, not `a <= b;`. + +For SystemVerilog (IEEE1800) code, the keyword `always_comb` should be used +instead of the general purpose `always` to take advantage of extra compile-time +checks. +For code which must be compatible with Verilog (IEEE1364), `always` is the only +option. +Therefore, this rule `reg` assignments to be compatible with Verilog like this +(in conjunction with **blocking_assignment_in_always_at_edge**): +```verilog +always @(posedge clk) q <= d; // Clocked to reg (flip-flop) +always @* a = b + c; // Combinational to reg (logic gates) +assign d = e + f; // Combinational to wire (logic gates) +``` + +See also: +- **non_blocking_assignment_in_always_at_edge** - Useful companion rule. +- **blocking_assignment_in_always_ff** - Similar rule, suggested as alternative + for SystemVerilog code, but not Verilog. +- **blocking_assignment_in_always_latch** - Useful companion rule for + SystemVerilog, but not Verilog. +- **non_blocking_assignment_in_always_comb** - Useful companion rule for + SystemVerilog, but not Verilog. + +The most relevant clauses of IEEE1800-2017 are: +- 4.9.3 Blocking assignment +- 4.9.4 Non-blocking assignment +- 9.4.2 Event control +- 10.4.1 Blocking procedural assignments +- 10.4.2 Nonblocking procedural assignments +- 16.5.1 Sampling diff --git a/md/syntaxrules-explanation-operator_incdec.md b/md/syntaxrules-explanation-operator_incdec.md new file mode 100644 index 00000000..9a7a63b3 --- /dev/null +++ b/md/syntaxrules-explanation-operator_incdec.md @@ -0,0 +1,23 @@ +Increment and decrement operators (`++` and `--`) are part of SystemVerilog +(IEEE1800), but not Verilog (IEEE1364). + +This rule allows only binary operators with simple assigments (`foo = foo + 1`) +to encourage backwards compatibility with Verilog. + +See also: +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **keyword_forbidden_always_comb** - Suggested companion rule. +- **keyword_forbidden_always_ff** - Suggested companion rule. +- **keyword_forbidden_always_latch** - Suggested companion rule. +- **keyword_forbidden_logic** - Suggested companion rule. +- **operator_self_assignment** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 4.1 Operators +- 9.2.1 Blocking procedural assignments +- 12.1.3.2 generate-loop + +The most relevant clauses of IEEE1800-2017 are: +- 10.4.1 Blocking procedural assignments +- 11.4.2 Increment and decrement operators +- 27.4 Loop generate constructs diff --git a/md/syntaxrules-explanation-operator_self_assignment.md b/md/syntaxrules-explanation-operator_self_assignment.md new file mode 100644 index 00000000..6ec0e6d1 --- /dev/null +++ b/md/syntaxrules-explanation-operator_self_assignment.md @@ -0,0 +1,22 @@ +Self-assignment operators (`+=`, `-=`, `*=`, `/=`, `%=`, `&=`, `|=`, `^=`, +`<<=`, `>>=`, `<<<=`, and `>>>=`) are part of SystemVerilog (IEEE1800), but not +Verilog (IEEE1364). + +This rule allows only simple assigment (using `=`) to encourage backwards +compatibility with Verilog. + +See also: +- **module_ansi_forbidden** - Useful companion rule for Verilog compatibility. +- **keyword_forbidden_always_comb** - Suggested companion rule. +- **keyword_forbidden_always_ff** - Suggested companion rule. +- **keyword_forbidden_always_latch** - Suggested companion rule. +- **keyword_forbidden_logic** - Suggested companion rule. +- **operator_incdec** - Suggested companion rule. + +The most relevant clauses of IEEE1364-2001 are: +- 4.1 Operators +- 9.2.1 Blocking procedural assignments + +The most relevant clauses of IEEE1800-2017 are: +- 10.4.1 Blocking procedural assignments +- 11.4.1 Assignment operators diff --git a/md/syntaxrules-explanation-re_forbidden_module_ansi.md b/md/syntaxrules-explanation-re_forbidden_module_ansi.md index d7e805a7..eb0e8af0 100644 --- a/md/syntaxrules-explanation-re_forbidden_module_ansi.md +++ b/md/syntaxrules-explanation-re_forbidden_module_ansi.md @@ -12,4 +12,4 @@ See also: - **prefix_module** - **uppercamelcase_module** - **lowercamelcase_module** -- **non_ansi_module** +- **module_nonansi_forbidden** diff --git a/md/syntaxrules-explanation-re_forbidden_module_nonansi.md b/md/syntaxrules-explanation-re_forbidden_module_nonansi.md index b7eeaeee..05ccb3f8 100644 --- a/md/syntaxrules-explanation-re_forbidden_module_nonansi.md +++ b/md/syntaxrules-explanation-re_forbidden_module_nonansi.md @@ -14,4 +14,4 @@ See also: - **prefix_module** - **uppercamelcase_module** - **lowercamelcase_module** -- **non_ansi_module** +- **module_nonansi_forbidden** diff --git a/md/syntaxrules-explanation-re_required_module_ansi.md b/md/syntaxrules-explanation-re_required_module_ansi.md index 0162a9c8..012d73fd 100644 --- a/md/syntaxrules-explanation-re_required_module_ansi.md +++ b/md/syntaxrules-explanation-re_required_module_ansi.md @@ -12,4 +12,4 @@ See also: - **prefix_module** - **uppercamelcase_module** - **lowercamelcase_module** -- **non_ansi_module** +- **module_nonansi_forbidden** diff --git a/md/syntaxrules-explanation-re_required_module_nonansi.md b/md/syntaxrules-explanation-re_required_module_nonansi.md index f8326c39..db52cfef 100644 --- a/md/syntaxrules-explanation-re_required_module_nonansi.md +++ b/md/syntaxrules-explanation-re_required_module_nonansi.md @@ -14,4 +14,4 @@ See also: - **prefix_module** - **uppercamelcase_module** - **lowercamelcase_module** -- **non_ansi_module** +- **module_nonansi_forbidden** diff --git a/rulesets/DaveMcEwan-design.toml b/rulesets/DaveMcEwan-design.toml index 2491cfe8..ac2ee404 100644 --- a/rulesets/DaveMcEwan-design.toml +++ b/rulesets/DaveMcEwan-design.toml @@ -35,7 +35,7 @@ syntaxrules.function_with_automatic = true syntaxrules.keyword_forbidden_priority = true syntaxrules.keyword_forbidden_unique = true syntaxrules.keyword_forbidden_unique0 = true -syntaxrules.level_sensitive_always = true +syntaxrules.general_always_no_edge = true syntaxrules.operator_case_equality = true # Common to **ruleset-designintent**. @@ -44,7 +44,7 @@ syntaxrules.default_nettype_none = true syntaxrules.function_same_as_system_function = true syntaxrules.keyword_forbidden_always = true syntaxrules.keyword_forbidden_wire_reg = true -syntaxrules.non_ansi_module = true +syntaxrules.module_nonansi_forbidden = true syntaxrules.localparam_type_twostate = true syntaxrules.parameter_type_twostate = true syntaxrules.localparam_explicit_type = true diff --git a/rulesets/designintent.toml b/rulesets/designintent.toml index 8c673059..ba04a851 100644 --- a/rulesets/designintent.toml +++ b/rulesets/designintent.toml @@ -7,14 +7,14 @@ syntaxrules.function_with_automatic = true syntaxrules.keyword_forbidden_priority = true syntaxrules.keyword_forbidden_unique = true syntaxrules.keyword_forbidden_unique0 = true -syntaxrules.level_sensitive_always = true # Redundant with keyword_forbidden_always. +#syntaxrules.general_always_no_edge = true # Redundant with keyword_forbidden_always. syntaxrules.operator_case_equality = true syntaxrules.action_block_with_side_effect = true syntaxrules.default_nettype_none = true syntaxrules.function_same_as_system_function = true syntaxrules.keyword_forbidden_always = true syntaxrules.keyword_forbidden_wire_reg = true -syntaxrules.non_ansi_module = true +syntaxrules.module_nonansi_forbidden = true syntaxrules.generate_case_with_label = true syntaxrules.generate_for_with_label = true syntaxrules.generate_if_with_label = true diff --git a/rulesets/designintentV2001.toml b/rulesets/designintentV2001.toml new file mode 100644 index 00000000..b675d5f7 --- /dev/null +++ b/rulesets/designintentV2001.toml @@ -0,0 +1,34 @@ +syntaxrules.keyword_forbidden_always_comb = true +syntaxrules.keyword_forbidden_always_ff = true +syntaxrules.keyword_forbidden_always_latch = true +syntaxrules.keyword_forbidden_priority = true +syntaxrules.keyword_forbidden_unique = true +syntaxrules.keyword_forbidden_unique0 = true +syntaxrules.keyword_forbidden_logic = true +syntaxrules.operator_incdec = true +syntaxrules.operator_self_assignment = true +syntaxrules.enum_with_type = true +syntaxrules.function_with_automatic = true +syntaxrules.operator_case_equality = true +syntaxrules.action_block_with_side_effect = true +syntaxrules.default_nettype_none = true +syntaxrules.function_same_as_system_function = true +syntaxrules.module_ansi_forbidden = true +syntaxrules.general_always_level_sensitive = true +syntaxrules.blocking_assignment_in_always_at_edge = true +syntaxrules.non_blocking_assignment_in_always_no_edge = true +syntaxrules.localparam_explicit_type = true +syntaxrules.parameter_explicit_type = true +syntaxrules.parameter_default_value = true +syntaxrules.parameter_in_generate = true +syntaxrules.genvar_declaration_in_loop = false +syntaxrules.genvar_declaration_out_loop = true +syntaxrules.keyword_forbidden_generate = false +syntaxrules.keyword_required_generate = true +#syntaxrules.loop_statement_in_always = true # Not implemented. +#syntaxrules.sequential_block_in_always = true # Not implemented. +syntaxrules.case_default = true # Applies in functions. +syntaxrules.explicit_case_default = true # Applies under `always`. +syntaxrules.explicit_if_else = true +syntaxrules.multiline_for_begin = true +syntaxrules.multiline_if_begin = true diff --git a/rulesets/simsynth.toml b/rulesets/simsynth.toml index 6ac0b087..cbd65301 100644 --- a/rulesets/simsynth.toml +++ b/rulesets/simsynth.toml @@ -7,5 +7,5 @@ syntaxrules.function_with_automatic = true syntaxrules.keyword_forbidden_priority = true syntaxrules.keyword_forbidden_unique = true syntaxrules.keyword_forbidden_unique0 = true -syntaxrules.level_sensitive_always = true +syntaxrules.general_always_no_edge = true syntaxrules.operator_case_equality = true diff --git a/rulesets/svlint-designintentV2001 b/rulesets/svlint-designintentV2001 new file mode 100755 index 00000000..b1dc5ad1 --- /dev/null +++ b/rulesets/svlint-designintentV2001 @@ -0,0 +1,32 @@ +#!/usr/bin/env sh +set -e + +# If flag/options are given that don't use the ruleset config, simply run +# svlint with the given arguments. +NONRULESET="-h|--help|-V|--version" +NONRULESET="${NONRULESET}|--dump-filelist|--shell-completion" +NONRULESET="${NONRULESET}|-E|--preprocess-only" +NONRULESET="${NONRULESET}|--config-example|--config-update|--example|--update" +if printf "%b\n" " $*" | grep -Eq " (${NONRULESET})"; +then + svlint $* + exit $? +fi + +SVLINT_CONFIG="$(dirname $(command -v svlint-designintentV2001))/designintentV2001.toml" + +# Delete ANSI control sequences that begin with ESC and (usually) end with m. +# Delete ASCII control characters except line feed ('\n' = 0o12 = 10 = 0x0A). +SANS_CONTROL="| sed -e 's/\\o33\\[[0-9;]*[mGKHF]//g'" +SANS_CONTROL="${SANS_CONTROL} | tr -d '[\\000-\\011\\013-\\037\\177]'" + +# Combine the above output sanitization fragments into variables which can be +# evaluated and processed with xargs, e.g: +# eval "${SVFILES}" | xargs -I {} sh -c "grep foo {};" +# NOTE: Creating a variable with the result (instead of the command) would lead +# to undefined behavior where the list of file paths exceeds 2MiB. +SVFILES="svlint --dump-filelist=files $* ${SANS_CONTROL}" +SVINCDIRS="svlint --dump-filelist=incdirs $* ${SANS_CONTROL}" + + +env SVLINT_CONFIG="${SVLINT_CONFIG}" svlint $* diff --git a/rulesets/svlint-designintentV2001.cmd b/rulesets/svlint-designintentV2001.cmd new file mode 100644 index 00000000..9319f52c --- /dev/null +++ b/rulesets/svlint-designintentV2001.cmd @@ -0,0 +1,7 @@ + +@echo off +for /f %%E in ('where.exe svlint-designintentV2001') do ( + set "SVLINT_CONFIG=%%~dpEdesignintentV2001.toml" +) +svlint %* + diff --git a/rulesets/svls-designintentV2001 b/rulesets/svls-designintentV2001 new file mode 100755 index 00000000..9d8c23aa --- /dev/null +++ b/rulesets/svls-designintentV2001 @@ -0,0 +1,15 @@ +#!/usr/bin/env sh +set -e + +# If flag/options are given that don't use the ruleset config, simply run +# svls with the given arguments. +NONRULESET="-h|--help|-V|--version" +if printf "%b\n" " $*" | grep -Eq " (${NONRULESET})"; +then + svls $* + exit $? +fi + +SVLINT_CONFIG="$(dirname $(command -v svls-designintentV2001))/designintentV2001.toml" + +env SVLINT_CONFIG="${SVLINT_CONFIG}" svls $* diff --git a/rulesets/svls-designintentV2001.cmd b/rulesets/svls-designintentV2001.cmd new file mode 100644 index 00000000..fb2ae2c8 --- /dev/null +++ b/rulesets/svls-designintentV2001.cmd @@ -0,0 +1,7 @@ + +@echo off +for /f %%E in ('where.exe svls-designintentV2001') do ( + set "SVLINT_CONFIG=%%~dpEdesignintentV2001.toml" +) +svls %* + diff --git a/rulesets/verifintent.toml b/rulesets/verifintent.toml index 73e97c79..03d980d5 100644 --- a/rulesets/verifintent.toml +++ b/rulesets/verifintent.toml @@ -8,7 +8,7 @@ syntaxrules.action_block_with_side_effect = true syntaxrules.default_nettype_none = true syntaxrules.function_same_as_system_function = true syntaxrules.keyword_forbidden_wire_reg = true -syntaxrules.non_ansi_module = true +syntaxrules.module_nonansi_forbidden = true syntaxrules.localparam_type_twostate = true syntaxrules.parameter_type_twostate = true syntaxrules.localparam_explicit_type = true diff --git a/src/syntaxrules/blocking_assignment_in_always_at_edge.rs b/src/syntaxrules/blocking_assignment_in_always_at_edge.rs new file mode 100644 index 00000000..99d49faa --- /dev/null +++ b/src/syntaxrules/blocking_assignment_in_always_at_edge.rs @@ -0,0 +1,54 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{unwrap_node, AlwaysKeyword, NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct BlockingAssignmentInAlwaysAtEdge; + +impl SyntaxRule for BlockingAssignmentInAlwaysAtEdge { + fn check( + &mut self, + _syntax_tree: &SyntaxTree, + event: &NodeEvent, + _option: &ConfigOption, + ) -> SyntaxRuleResult { + let node = match event { + NodeEvent::Enter(x) => x, + NodeEvent::Leave(_) => { + return SyntaxRuleResult::Pass; + } + }; + + match node { + RefNode::AlwaysConstruct(x) => { + let (t, x) = &x.nodes; + match t { + AlwaysKeyword::Always(_) => { + let edge = unwrap_node!(x, EdgeIdentifier); + let blocking_assignment = unwrap_node!(x, BlockingAssignment); + let variable_assignment = unwrap_node!(x, VariableDeclAssignment); + if edge.is_some() && (blocking_assignment.is_some() || variable_assignment.is_some()) { + SyntaxRuleResult::Fail + } else { + SyntaxRuleResult::Pass + } + } + _ => SyntaxRuleResult::Pass, + } + } + _ => SyntaxRuleResult::Pass, + } + } + + fn name(&self) -> String { + String::from("blocking_assignment_in_always_at_edge") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Do not use blocking assignments within edge-sensitive `always`.") + } + + fn reason(&self) -> String { + String::from("Blocking assignment in `always_ff` may cause undefined event ordering.") + } +} diff --git a/src/syntaxrules/general_always_level_sensitive.rs b/src/syntaxrules/general_always_level_sensitive.rs new file mode 100644 index 00000000..296f08cd --- /dev/null +++ b/src/syntaxrules/general_always_level_sensitive.rs @@ -0,0 +1,55 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{unwrap_node, AlwaysKeyword, NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct GeneralAlwaysLevelSensitive; + +impl SyntaxRule for GeneralAlwaysLevelSensitive { + fn check( + &mut self, + _syntax_tree: &SyntaxTree, + event: &NodeEvent, + _option: &ConfigOption, + ) -> SyntaxRuleResult { + let node = match event { + NodeEvent::Enter(x) => x, + NodeEvent::Leave(_) => { + return SyntaxRuleResult::Pass; + } + }; + match node { + RefNode::AlwaysConstruct(x) => { + let (t, x) = &x.nodes; + match t { + AlwaysKeyword::Always(_) => { + let c = unwrap_node!(x, EventControlEventExpression); + if let Some(x) = c { + if let Some(_) = unwrap_node!(x, EdgeIdentifier) { + SyntaxRuleResult::Pass + } else { + SyntaxRuleResult::Fail + } + } else { + SyntaxRuleResult::Pass + } + } + _ => SyntaxRuleResult::Pass, + } + } + _ => SyntaxRuleResult::Pass, + } + } + + fn name(&self) -> String { + String::from("general_always_level_sensitive") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Replace general-purpose `always @(...no edge...)` with `always @*`.") + } + + fn reason(&self) -> String { + String::from("General-purpose `always` cannot detect combinatorial/stateful mistakes.") + } +} diff --git a/src/syntaxrules/level_sensitive_always.rs b/src/syntaxrules/general_always_no_edge.rs similarity index 82% rename from src/syntaxrules/level_sensitive_always.rs rename to src/syntaxrules/general_always_no_edge.rs index e62b2d8f..a68d8f93 100644 --- a/src/syntaxrules/level_sensitive_always.rs +++ b/src/syntaxrules/general_always_no_edge.rs @@ -3,9 +3,9 @@ use crate::linter::{SyntaxRule, SyntaxRuleResult}; use sv_parser::{unwrap_node, AlwaysKeyword, NodeEvent, RefNode, SyntaxTree}; #[derive(Default)] -pub struct LevelSensitiveAlways; +pub struct GeneralAlwaysNoEdge; -impl SyntaxRule for LevelSensitiveAlways { +impl SyntaxRule for GeneralAlwaysNoEdge { fn check( &mut self, _syntax_tree: &SyntaxTree, @@ -39,14 +39,14 @@ impl SyntaxRule for LevelSensitiveAlways { } fn name(&self) -> String { - String::from("level_sensitive_always") + String::from("general_always_no_edge") } fn hint(&self, _option: &ConfigOption) -> String { - String::from("Replace level-sensitive `always` with `always_comb`.") + String::from("Replace general-purpose `always` with `always_comb`.") } fn reason(&self) -> String { - String::from("Level-sensitive `always` cannot detect combinatorial/stateful (non-)blocking mistakes.") + String::from("General-purpose `always` cannot detect combinatorial/stateful mistakes.") } } diff --git a/src/syntaxrules/keyword_forbidden_always_comb.rs b/src/syntaxrules/keyword_forbidden_always_comb.rs new file mode 100644 index 00000000..2673cd28 --- /dev/null +++ b/src/syntaxrules/keyword_forbidden_always_comb.rs @@ -0,0 +1,38 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{AlwaysKeyword, NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct KeywordForbiddenAlwaysComb; + +impl SyntaxRule for KeywordForbiddenAlwaysComb { + fn check( + &mut self, + _syntax_tree: &SyntaxTree, + event: &NodeEvent, + _option: &ConfigOption, + ) -> SyntaxRuleResult { + let node = match event { + NodeEvent::Enter(x) => x, + NodeEvent::Leave(_) => { + return SyntaxRuleResult::Pass; + } + }; + match node { + RefNode::AlwaysKeyword(AlwaysKeyword::AlwaysComb(_)) => SyntaxRuleResult::Fail, + _ => SyntaxRuleResult::Pass, + } + } + + fn name(&self) -> String { + String::from("keyword_forbidden_always_comb") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Use `always @*` instead of `always_comb`.") + } + + fn reason(&self) -> String { + String::from("Only SystemVerilog, not Verilog, has `always_comb`.") + } +} diff --git a/src/syntaxrules/keyword_forbidden_always_ff.rs b/src/syntaxrules/keyword_forbidden_always_ff.rs new file mode 100644 index 00000000..b586ea4d --- /dev/null +++ b/src/syntaxrules/keyword_forbidden_always_ff.rs @@ -0,0 +1,38 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{AlwaysKeyword, NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct KeywordForbiddenAlwaysFf; + +impl SyntaxRule for KeywordForbiddenAlwaysFf { + fn check( + &mut self, + _syntax_tree: &SyntaxTree, + event: &NodeEvent, + _option: &ConfigOption, + ) -> SyntaxRuleResult { + let node = match event { + NodeEvent::Enter(x) => x, + NodeEvent::Leave(_) => { + return SyntaxRuleResult::Pass; + } + }; + match node { + RefNode::AlwaysKeyword(AlwaysKeyword::AlwaysFf(_)) => SyntaxRuleResult::Fail, + _ => SyntaxRuleResult::Pass, + } + } + + fn name(&self) -> String { + String::from("keyword_forbidden_always_ff") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Use `always @(posedge clk)` instead of `always_ff @(posedge clk)`.") + } + + fn reason(&self) -> String { + String::from("Only SystemVerilog, not Verilog, has `always_ff`.") + } +} diff --git a/src/syntaxrules/keyword_forbidden_always_latch.rs b/src/syntaxrules/keyword_forbidden_always_latch.rs new file mode 100644 index 00000000..5df55a01 --- /dev/null +++ b/src/syntaxrules/keyword_forbidden_always_latch.rs @@ -0,0 +1,38 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{AlwaysKeyword, NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct KeywordForbiddenAlwaysLatch; + +impl SyntaxRule for KeywordForbiddenAlwaysLatch { + fn check( + &mut self, + _syntax_tree: &SyntaxTree, + event: &NodeEvent, + _option: &ConfigOption, + ) -> SyntaxRuleResult { + let node = match event { + NodeEvent::Enter(x) => x, + NodeEvent::Leave(_) => { + return SyntaxRuleResult::Pass; + } + }; + match node { + RefNode::AlwaysKeyword(AlwaysKeyword::AlwaysLatch(_)) => SyntaxRuleResult::Fail, + _ => SyntaxRuleResult::Pass, + } + } + + fn name(&self) -> String { + String::from("keyword_forbidden_always_latch") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Use `always @*` or `always @(en)` instead of `always_latch`.") + } + + fn reason(&self) -> String { + String::from("Only SystemVerilog, not Verilog, has `always_latch`.") + } +} diff --git a/src/syntaxrules/keyword_forbidden_logic.rs b/src/syntaxrules/keyword_forbidden_logic.rs new file mode 100644 index 00000000..a6190834 --- /dev/null +++ b/src/syntaxrules/keyword_forbidden_logic.rs @@ -0,0 +1,38 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{IntegerVectorType, NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct KeywordForbiddenLogic; + +impl SyntaxRule for KeywordForbiddenLogic { + fn check( + &mut self, + _syntax_tree: &SyntaxTree, + event: &NodeEvent, + _option: &ConfigOption, + ) -> SyntaxRuleResult { + let node = match event { + NodeEvent::Enter(x) => x, + NodeEvent::Leave(_) => { + return SyntaxRuleResult::Pass; + } + }; + match node { + RefNode::IntegerVectorType(IntegerVectorType::Logic(_)) => SyntaxRuleResult::Fail, + _ => SyntaxRuleResult::Pass, + } + } + + fn name(&self) -> String { + String::from("keyword_forbidden_logic") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Replace `logic` keywords with `wire` or `reg`.") + } + + fn reason(&self) -> String { + String::from("Only SystemVerilog, not Verilog, has `logic`.") + } +} diff --git a/src/syntaxrules/module_ansi_forbidden.rs b/src/syntaxrules/module_ansi_forbidden.rs new file mode 100644 index 00000000..96f62617 --- /dev/null +++ b/src/syntaxrules/module_ansi_forbidden.rs @@ -0,0 +1,45 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct ModuleAnsiForbidden; + +impl SyntaxRule for ModuleAnsiForbidden { + fn check( + &mut self, + _syntax_tree: &SyntaxTree, + event: &NodeEvent, + _option: &ConfigOption, + ) -> SyntaxRuleResult { + let node = match event { + NodeEvent::Enter(x) => x, + NodeEvent::Leave(_) => { + return SyntaxRuleResult::Pass; + } + }; + match node { + RefNode::ModuleAnsiHeader(x) => { + let (_, _, _, _, _, _, ports, _) = &x.nodes; + if let Some(_) = ports { + SyntaxRuleResult::Fail + } else { + SyntaxRuleResult::Pass + } + } + _ => SyntaxRuleResult::Pass, + } + } + + fn name(&self) -> String { + String::from("module_ansi_forbidden") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Declare `module` header in non-ANSI style.") + } + + fn reason(&self) -> String { + String::from("Only SystemVerilog, not Verilog, allows `localparam` in ANSI module header.") + } +} diff --git a/src/syntaxrules/non_ansi_module.rs b/src/syntaxrules/module_nonansi_forbidden.rs similarity index 87% rename from src/syntaxrules/non_ansi_module.rs rename to src/syntaxrules/module_nonansi_forbidden.rs index 817e1786..94d16edd 100644 --- a/src/syntaxrules/non_ansi_module.rs +++ b/src/syntaxrules/module_nonansi_forbidden.rs @@ -3,9 +3,9 @@ use crate::linter::{SyntaxRule, SyntaxRuleResult}; use sv_parser::{NodeEvent, RefNode, SyntaxTree}; #[derive(Default)] -pub struct NonAnsiModule; +pub struct ModuleNonansiForbidden; -impl SyntaxRule for NonAnsiModule { +impl SyntaxRule for ModuleNonansiForbidden { fn check( &mut self, _syntax_tree: &SyntaxTree, @@ -25,7 +25,7 @@ impl SyntaxRule for NonAnsiModule { } fn name(&self) -> String { - String::from("non_ansi_module") + String::from("module_nonansi_forbidden") } fn hint(&self, _option: &ConfigOption) -> String { diff --git a/src/syntaxrules/non_blocking_assignment_in_always_no_edge.rs b/src/syntaxrules/non_blocking_assignment_in_always_no_edge.rs new file mode 100644 index 00000000..9290380d --- /dev/null +++ b/src/syntaxrules/non_blocking_assignment_in_always_no_edge.rs @@ -0,0 +1,53 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{unwrap_node, AlwaysKeyword, NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct NonBlockingAssignmentInAlwaysNoEdge; + +impl SyntaxRule for NonBlockingAssignmentInAlwaysNoEdge { + fn check( + &mut self, + _syntax_tree: &SyntaxTree, + event: &NodeEvent, + _option: &ConfigOption, + ) -> SyntaxRuleResult { + let node = match event { + NodeEvent::Enter(x) => x, + NodeEvent::Leave(_) => { + return SyntaxRuleResult::Pass; + } + }; + + match node { + RefNode::AlwaysConstruct(x) => { + let (t, x) = &x.nodes; + match t { + AlwaysKeyword::Always(_) => { + let edge = unwrap_node!(x, EdgeIdentifier); + let nonblocking_assignment = unwrap_node!(x, NonblockingAssignment); + if edge.is_none() && nonblocking_assignment.is_some() { + SyntaxRuleResult::Fail + } else { + SyntaxRuleResult::Pass + } + } + _ => SyntaxRuleResult::Pass, + } + } + _ => SyntaxRuleResult::Pass, + } + } + + fn name(&self) -> String { + String::from("non_blocking_assignment_in_always_no_edge") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Remove non-blocking assignment in combinational `always`.") + } + + fn reason(&self) -> String { + String::from("Scheduling between blocking and non-blocking assignments is non-deterministic.") + } +} diff --git a/src/syntaxrules/operator_incdec.rs b/src/syntaxrules/operator_incdec.rs new file mode 100644 index 00000000..714b5c33 --- /dev/null +++ b/src/syntaxrules/operator_incdec.rs @@ -0,0 +1,39 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct OperatorIncdec; + +impl SyntaxRule for OperatorIncdec { + fn check( + &mut self, + _syntax_tree: &SyntaxTree, + event: &NodeEvent, + _option: &ConfigOption, + ) -> SyntaxRuleResult { + let node = match event { + NodeEvent::Enter(x) => x, + NodeEvent::Leave(_) => { + return SyntaxRuleResult::Pass; + } + }; + + match node { + RefNode::IncOrDecOperator(_) => SyntaxRuleResult::Fail, + _ => SyntaxRuleResult::Pass, + } + } + + fn name(&self) -> String { + String::from("operator_incdec") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Use `=` with a `+` or `-` instead of an increment or decrement operator.") + } + + fn reason(&self) -> String { + String::from("Only SystemVerilog, not Verilog, has increment and decrement operators.") + } +} diff --git a/src/syntaxrules/operator_self_assignment.rs b/src/syntaxrules/operator_self_assignment.rs new file mode 100644 index 00000000..1682ddc1 --- /dev/null +++ b/src/syntaxrules/operator_self_assignment.rs @@ -0,0 +1,61 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{unwrap_node, unwrap_locate, Locate, NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct OperatorSelfAssignment; + +impl SyntaxRule for OperatorSelfAssignment { + fn check( + &mut self, + syntax_tree: &SyntaxTree, + event: &NodeEvent, + _option: &ConfigOption, + ) -> SyntaxRuleResult { + let node = match event { + NodeEvent::Enter(x) => x, + NodeEvent::Leave(_) => { + return SyntaxRuleResult::Pass; + } + }; + + match node { + RefNode::AssignmentOperator(x) => { + let loc: Option<&Locate> = match unwrap_node!(*x, Symbol) { + Some(RefNode::Symbol(symbol_)) => { + unwrap_locate!(symbol_) + } + _ => None, + }; + + if let Some(loc) = loc { + let s = syntax_tree.get_str(loc).unwrap(); + + // Only Verilog-compatible assignment `=` is a single ASCII character. + // assignment_operator ::= + // = | += | -= | *= | /= | %= | &= | |= | ^= | <<= | >>= | <<<= | >>>= + if 1 < s.len() { + SyntaxRuleResult::Fail + } else { + SyntaxRuleResult::Pass + } + } else { + SyntaxRuleResult::Pass + } + } + _ => SyntaxRuleResult::Pass, + } + } + + fn name(&self) -> String { + String::from("operator_self_assignment") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Use `=` with a binary operator instead of a self-assignment operator.") + } + + fn reason(&self) -> String { + String::from("Only SystemVerilog, not Verilog, allows self-assignment operators.") + } +} diff --git a/testcases/syntaxrules/fail/blocking_assignment_in_always_at_edge.sv b/testcases/syntaxrules/fail/blocking_assignment_in_always_at_edge.sv new file mode 100644 index 00000000..67d46454 --- /dev/null +++ b/testcases/syntaxrules/fail/blocking_assignment_in_always_at_edge.sv @@ -0,0 +1,11 @@ +module M; + always @(posedge clk) q = d; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(negedge clk) q = d; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(edge clk) q = d; +endmodule diff --git a/testcases/syntaxrules/fail/blocking_assignment_in_always_ff.sv b/testcases/syntaxrules/fail/blocking_assignment_in_always_ff.sv index 0998910c..c22b5344 100644 --- a/testcases/syntaxrules/fail/blocking_assignment_in_always_ff.sv +++ b/testcases/syntaxrules/fail/blocking_assignment_in_always_ff.sv @@ -1,7 +1,3 @@ module M; -/* svlint off blocking_assignment_in_always_ff */ -always_ff @(posedge clk) q1 = d; // Control comments avoid failure. -/* svlint on blocking_assignment_in_always_ff */ - -always_ff @(posedge clk) q2 = d; // Failure. + always_ff @(posedge clk) q = d; // Failure. endmodule diff --git a/testcases/syntaxrules/fail/general_always_level_sensitive.sv b/testcases/syntaxrules/fail/general_always_level_sensitive.sv new file mode 100644 index 00000000..183ceb91 --- /dev/null +++ b/testcases/syntaxrules/fail/general_always_level_sensitive.sv @@ -0,0 +1,9 @@ +module M; + always @(b) // Missing sensitivity to `c`. + a = b + c; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(a or b) // Correct sensitivity list, but error prone. + a = b + c; +endmodule diff --git a/testcases/syntaxrules/fail/level_sensitive_always.sv b/testcases/syntaxrules/fail/general_always_no_edge.sv similarity index 70% rename from testcases/syntaxrules/fail/level_sensitive_always.sv rename to testcases/syntaxrules/fail/general_always_no_edge.sv index ad7f77f9..39161f49 100644 --- a/testcases/syntaxrules/fail/level_sensitive_always.sv +++ b/testcases/syntaxrules/fail/general_always_no_edge.sv @@ -4,6 +4,6 @@ module M; endmodule //////////////////////////////////////////////////////////////////////////////// module M; - always @ (a or b) begin // No sensitivity to posedge, negedge, or edge. + always @(a or b) begin // No sensitivity to posedge, negedge, or edge. end endmodule diff --git a/testcases/syntaxrules/fail/generate_for_with_label.sv b/testcases/syntaxrules/fail/generate_for_with_label.sv index d2021926..5d20a6b7 100644 --- a/testcases/syntaxrules/fail/generate_for_with_label.sv +++ b/testcases/syntaxrules/fail/generate_for_with_label.sv @@ -1,5 +1,5 @@ module M; - for (genvar i=0; i < 10; i++) // No begin/end delimeters. + for (genvar i=0; i < 10; i++) // No begin/end delimiters. assign a[i] = i; endmodule //////////////////////////////////////////////////////////////////////////////// diff --git a/testcases/syntaxrules/fail/generate_if_with_label.sv b/testcases/syntaxrules/fail/generate_if_with_label.sv index adf001a0..53576cd7 100644 --- a/testcases/syntaxrules/fail/generate_if_with_label.sv +++ b/testcases/syntaxrules/fail/generate_if_with_label.sv @@ -1,5 +1,5 @@ module M; - if (x) // No begin/end delimeters. + if (x) // No begin/end delimiters. assign a = 0; // if condition. else if (x) begin: l_def assign a = 1; @@ -11,7 +11,7 @@ endmodule module M; if (x) begin: l_abc assign a = 0; - end else if (x) // No begin/end delimeters. + end else if (x) // No begin/end delimiters. assign a = 1; // else-if condition. else begin: l_hij assign a = 2; @@ -24,7 +24,7 @@ module M; assign a = 0; end else if (x) begin: l_def assign a = 1; - end else // No begin/end delimeters. + end else // No begin/end delimiters. assign a = 2; // else condition endmodule //////////////////////////////////////////////////////////////////////////////// diff --git a/testcases/syntaxrules/fail/keyword_forbidden_always_comb.sv b/testcases/syntaxrules/fail/keyword_forbidden_always_comb.sv new file mode 100644 index 00000000..9ed037ba --- /dev/null +++ b/testcases/syntaxrules/fail/keyword_forbidden_always_comb.sv @@ -0,0 +1,3 @@ +module M; + always_comb z = x + y; +endmodule diff --git a/testcases/syntaxrules/fail/keyword_forbidden_always_ff.sv b/testcases/syntaxrules/fail/keyword_forbidden_always_ff.sv new file mode 100644 index 00000000..35bd9f29 --- /dev/null +++ b/testcases/syntaxrules/fail/keyword_forbidden_always_ff.sv @@ -0,0 +1,4 @@ +module M; + always_ff @(posedge clk) + d <= q; +endmodule diff --git a/testcases/syntaxrules/fail/keyword_forbidden_always_latch.sv b/testcases/syntaxrules/fail/keyword_forbidden_always_latch.sv new file mode 100644 index 00000000..0989992b --- /dev/null +++ b/testcases/syntaxrules/fail/keyword_forbidden_always_latch.sv @@ -0,0 +1,5 @@ +module M; + always_latch + if (en) + d <= q; +endmodule diff --git a/testcases/syntaxrules/fail/keyword_forbidden_logic.sv b/testcases/syntaxrules/fail/keyword_forbidden_logic.sv new file mode 100644 index 00000000..31e04a7c --- /dev/null +++ b/testcases/syntaxrules/fail/keyword_forbidden_logic.sv @@ -0,0 +1,3 @@ +module M; + logic a; +endmodule diff --git a/testcases/syntaxrules/fail/module_ansi_forbidden.sv b/testcases/syntaxrules/fail/module_ansi_forbidden.sv new file mode 100644 index 00000000..c9503051 --- /dev/null +++ b/testcases/syntaxrules/fail/module_ansi_forbidden.sv @@ -0,0 +1,15 @@ +module M // An ANSI module has ports declared in the module header. + ( input a + , output b + ); +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M // Declaring ports in the header with default direction (inout) + ( a // also specifies an ANSI module where directions are not given + , b // later. + ); +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M + (); // A module with an empty portlist is ANSI. +endmodule diff --git a/testcases/syntaxrules/fail/non_ansi_module.sv b/testcases/syntaxrules/fail/module_nonansi_forbidden.sv similarity index 100% rename from testcases/syntaxrules/fail/non_ansi_module.sv rename to testcases/syntaxrules/fail/module_nonansi_forbidden.sv diff --git a/testcases/syntaxrules/fail/non_blocking_assignment_in_always_no_edge.sv b/testcases/syntaxrules/fail/non_blocking_assignment_in_always_no_edge.sv new file mode 100644 index 00000000..10ce89d8 --- /dev/null +++ b/testcases/syntaxrules/fail/non_blocking_assignment_in_always_no_edge.sv @@ -0,0 +1,7 @@ +module M; + always @* a <= b + c; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(*) a <= b + c; +endmodule diff --git a/testcases/syntaxrules/fail/operator_incdec.sv b/testcases/syntaxrules/fail/operator_incdec.sv new file mode 100644 index 00000000..809d6bd5 --- /dev/null +++ b/testcases/syntaxrules/fail/operator_incdec.sv @@ -0,0 +1,29 @@ +module M; + always @(posedge clk) z--; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(posedge clk) z++; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z = x + y--; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z = x + y++; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + genvar i; + for (i = 4; i >= 0; i--) begin + assign z[i] = y[i] + x[i]; + end +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + genvar i; + for (i = 0; i < 5; i++) begin + assign z[i] = y[i] + x[i]; + end +endmodule diff --git a/testcases/syntaxrules/fail/operator_self_assignment.sv b/testcases/syntaxrules/fail/operator_self_assignment.sv new file mode 100644 index 00000000..76a86eac --- /dev/null +++ b/testcases/syntaxrules/fail/operator_self_assignment.sv @@ -0,0 +1,47 @@ +module M; + always @* z += y; // Addition `z = z + y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z -= y; // Subtraction `z = z - y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z *= y; // Multiplication `z = z * y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z /= y; // Division `z = z / y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z %= y; // Modulo `z = z % y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z &= y; // Bitwise AND `z = z & y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z |= y; // Bitwise OR `z = z | y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z ^= y; // Bitwise XOR `z = z ^ y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z <<= y; // Logical left shift `z = z << y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z >>= y; // Logical right shift `z = z >> y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z <<<= y; // Arithmetic left shift `z = z <<< y` +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z >>>= y; // Arithmetic right shift `z = z >>> y` +endmodule diff --git a/testcases/syntaxrules/pass/blocking_assignment_in_always_at_edge.sv b/testcases/syntaxrules/pass/blocking_assignment_in_always_at_edge.sv new file mode 100644 index 00000000..c88c3480 --- /dev/null +++ b/testcases/syntaxrules/pass/blocking_assignment_in_always_at_edge.sv @@ -0,0 +1,11 @@ +module M; + always @(posedge clk) q <= d; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(negedge clk) q <= d; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(edge clk) q <= d; +endmodule diff --git a/testcases/syntaxrules/pass/general_always_level_sensitive.sv b/testcases/syntaxrules/pass/general_always_level_sensitive.sv new file mode 100644 index 00000000..9ed28422 --- /dev/null +++ b/testcases/syntaxrules/pass/general_always_level_sensitive.sv @@ -0,0 +1,9 @@ +module M; + always @* // Sensitive to `b` and `c`. + a = b + c; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(posedge clk) // Sensitive to edges of `clk`. + q <= d; +endmodule diff --git a/testcases/syntaxrules/pass/level_sensitive_always.sv b/testcases/syntaxrules/pass/general_always_no_edge.sv similarity index 100% rename from testcases/syntaxrules/pass/level_sensitive_always.sv rename to testcases/syntaxrules/pass/general_always_no_edge.sv diff --git a/testcases/syntaxrules/pass/keyword_forbidden_always_comb.sv b/testcases/syntaxrules/pass/keyword_forbidden_always_comb.sv new file mode 100644 index 00000000..617a7269 --- /dev/null +++ b/testcases/syntaxrules/pass/keyword_forbidden_always_comb.sv @@ -0,0 +1,3 @@ +module M; + always @* z = x + y; +endmodule diff --git a/testcases/syntaxrules/pass/keyword_forbidden_always_ff.sv b/testcases/syntaxrules/pass/keyword_forbidden_always_ff.sv new file mode 100644 index 00000000..ee69ff55 --- /dev/null +++ b/testcases/syntaxrules/pass/keyword_forbidden_always_ff.sv @@ -0,0 +1,4 @@ +module M; + always @(posedge clk) + d <= q; +endmodule diff --git a/testcases/syntaxrules/pass/keyword_forbidden_always_latch.sv b/testcases/syntaxrules/pass/keyword_forbidden_always_latch.sv new file mode 100644 index 00000000..b77a3b7a --- /dev/null +++ b/testcases/syntaxrules/pass/keyword_forbidden_always_latch.sv @@ -0,0 +1,11 @@ +module M; + always @* + if (en) + d <= q; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(en) + if (en) + d <= q; +endmodule diff --git a/testcases/syntaxrules/pass/keyword_forbidden_logic.sv b/testcases/syntaxrules/pass/keyword_forbidden_logic.sv new file mode 100644 index 00000000..0951a62b --- /dev/null +++ b/testcases/syntaxrules/pass/keyword_forbidden_logic.sv @@ -0,0 +1,4 @@ +module M; + wire a; + reg b; +endmodule diff --git a/testcases/syntaxrules/pass/module_ansi_forbidden.sv b/testcases/syntaxrules/pass/module_ansi_forbidden.sv new file mode 100644 index 00000000..499f396f --- /dev/null +++ b/testcases/syntaxrules/pass/module_ansi_forbidden.sv @@ -0,0 +1,10 @@ +module M + ( a + , b + ); + input a; // Declaring ports outside the module header declaration + output b; // makes this a non-ANSI module. +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; // A module with no portlist is ANSI, but allowed. +endmodule diff --git a/testcases/syntaxrules/pass/non_ansi_module.sv b/testcases/syntaxrules/pass/module_nonansi_forbidden.sv similarity index 100% rename from testcases/syntaxrules/pass/non_ansi_module.sv rename to testcases/syntaxrules/pass/module_nonansi_forbidden.sv diff --git a/testcases/syntaxrules/pass/non_blocking_assignment_in_always_no_edge.sv b/testcases/syntaxrules/pass/non_blocking_assignment_in_always_no_edge.sv new file mode 100644 index 00000000..5ae5dce9 --- /dev/null +++ b/testcases/syntaxrules/pass/non_blocking_assignment_in_always_no_edge.sv @@ -0,0 +1,7 @@ +module M; + always @* a = b + c; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(*) a = b + c; +endmodule diff --git a/testcases/syntaxrules/pass/operator_incdec.sv b/testcases/syntaxrules/pass/operator_incdec.sv new file mode 100644 index 00000000..0b412701 --- /dev/null +++ b/testcases/syntaxrules/pass/operator_incdec.sv @@ -0,0 +1,29 @@ +module M; + always @(posedge clk) z = z - 1; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @(posedge clk) z = z + 1; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z = z - 1; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + always @* z = z + 1; +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + genvar i; + for (i = 4; i >= 0; i = i - 1) begin + assign z[i] = y[i] + x[i]; + end +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + genvar i; + for (i = 0; i < 5; i = i + 1) begin + assign z[i] = y[i] + x[i]; + end +endmodule diff --git a/testcases/syntaxrules/pass/operator_self_assignment.sv b/testcases/syntaxrules/pass/operator_self_assignment.sv new file mode 100644 index 00000000..25a7f7ed --- /dev/null +++ b/testcases/syntaxrules/pass/operator_self_assignment.sv @@ -0,0 +1,5 @@ +module M; + always @* + if (a == b) // Logical-equality operator is not an assignment. + z = y; // Simple assignment operator is allowed. +endmodule