Skip to content

Commit

Permalink
Merge pull request #276 from remes-codasip/rule-procedural-continuous…
Browse files Browse the repository at this point in the history
…-assignment

New rule `procedural_continuous_assignment`
  • Loading branch information
dalance authored Dec 27, 2023
2 parents 4622541 + cf91a86 commit 1200f0a
Show file tree
Hide file tree
Showing 16 changed files with 215 additions and 2 deletions.
5 changes: 3 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ Adding A New Rule
placeholder in this list of steps.
2. Write a short description in Markdown about what the new rule checks and why
it might be used.
Write this in `md/explanation-$RULENAME.md`, preferably keeping a similar
format to similar existing rules.
Write this in `md/(text|syntax)rules-explanation-$RULENAME.md`, preferably
keeping a similar format to similar existing rules.
3. Write at least one testcase which the new rule passes in
`testcases/(text|syntax)rules/pass/$RULENAME.sv`.
If you have more than one testcase which must pass, they should all go into
Expand Down Expand Up @@ -146,3 +146,4 @@ under the Developer Certificate of Origin <https://developercertificate.org/>.
- Damien Pretet (@dpretet)
- Taichi Ishitani (@taichi-ishitani)
- Sosuke Hosokawa (@so298)
- Jan Remes (@remes-codasip)
106 changes: 106 additions & 0 deletions MANUAL.md
Original file line number Diff line number Diff line change
Expand Up @@ -4809,6 +4809,107 @@ The most relevant clauses of IEEE1800-2017 are:



* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *

## Syntax Rule: `procedural_continuous_assignment`

### Hint

Move `assign` out of `always` block.

### Reason

Procedural continuous assigments are not synthesizable.

### Pass Example (1 of 4)
```systemverilog
module M;
assign c = a + b; // Continuous assignment
endmodule
```

### Pass Example (2 of 4)
```systemverilog
module M;
always_ff @(posedge clk)
c <= a + b; // Procedural non-blocking assignment
endmodule
```

### Pass Example (3 of 4)
```systemverilog
module M;
always_comb
c = a + b; // Procedural blocking assignment
endmodule
```

### Pass Example (4 of 4)
```systemverilog
module M;
always @*
c = a + b; // Procedural blocking assignment, Verilog 2001
endmodule
```

### Fail Example (1 of 4)
```systemverilog
module M;
always @*
assign c = a + b;
endmodule
```

### Fail Example (2 of 4)
```systemverilog
module M;
always_comb
assign c = a + b;
endmodule
```

### Fail Example (3 of 4)
```systemverilog
module M;
always_latch
assign c = a + b;
endmodule
```

### Fail Example (4 of 4)
```systemverilog
module M;
always_ff @(posedge clk)
assign c = a + b;
endmodule
```

### Explanation

Continuous assignment, e.g. `assign x = y;` outside of any `always` process,
continuously drives the LHS and changes with any change on the RHS.
The same keyword `assign` has different meaning when used within an `always`
process (or `always_ff`, `always_comb`, `initial`, etc.) where it can be used
to override procedural assignments.
Using this construct in a procedural block (`always*`) which is only triggered
on changes to signals in the sensitivity list may not be synthesizable.

This SystemVerilog language feature is being considered for deprecation, as
noted in IEEE1800-2017 Annex C, because it is easily abused and difficult to
implement while not providing additional capability.
Users are strongly encouraged to migrate their cod to use one of the alternate
methods of procedural or continuous assignments.

See also:
- **non_blocking_assignment_in_always_comb** - Useful companion rule.
- **blocking_assignment_in_always_ff** - Useful companion rule.

The most relevant clauses of IEEE1800-2017 are:
- 10.6.1 The assign and deassign procedural statements
- Annex C.4 Constructs identified for deprecation



* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *

## Syntax Rule: `sequential_block_in_always_comb`
Expand Down Expand Up @@ -11137,6 +11238,7 @@ syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
syntaxrules.general_always_no_edge = true
syntaxrules.operator_case_equality = true
syntaxrules.procedural_continuous_assignment = true

# Common to **ruleset-designintent**.
syntaxrules.action_block_with_side_effect = true
Expand Down Expand Up @@ -11873,6 +11975,7 @@ syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
#syntaxrules.general_always_no_edge = true # Redundant with keyword_forbidden_always.
syntaxrules.operator_case_equality = true
syntaxrules.procedural_continuous_assignment = true
```

This ruleset has further rules which don't depend on each other or combine
Expand Down Expand Up @@ -12010,6 +12113,7 @@ 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.procedural_continuous_assignment = true
```

Verilog does allow both ANSI and non-ANSI forms of module declaration, but
Expand Down Expand Up @@ -12156,6 +12260,7 @@ syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
syntaxrules.general_always_no_edge = true
syntaxrules.operator_case_equality = true
syntaxrules.procedural_continuous_assignment = true
```


Expand Down Expand Up @@ -12483,6 +12588,7 @@ syntaxrules.enum_with_type = true
syntaxrules.keyword_forbidden_priority = true
syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
syntaxrules.procedural_continuous_assignment = true
```

This ruleset has further rules which don't depend on each other or combine
Expand Down
1 change: 1 addition & 0 deletions md/ruleset-DaveMcEwan-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
syntaxrules.general_always_no_edge = true
syntaxrules.operator_case_equality = true
syntaxrules.procedural_continuous_assignment = true

# Common to **ruleset-designintent**.
syntaxrules.action_block_with_side_effect = true
Expand Down
1 change: 1 addition & 0 deletions md/ruleset-designintent.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
#syntaxrules.general_always_no_edge = true # Redundant with keyword_forbidden_always.
syntaxrules.operator_case_equality = true
syntaxrules.procedural_continuous_assignment = true
```

This ruleset has further rules which don't depend on each other or combine
Expand Down
1 change: 1 addition & 0 deletions md/ruleset-designintentV2001.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ 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.procedural_continuous_assignment = true
```

Verilog does allow both ANSI and non-ANSI forms of module declaration, but
Expand Down
1 change: 1 addition & 0 deletions md/ruleset-simsynth.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
syntaxrules.general_always_no_edge = true
syntaxrules.operator_case_equality = true
syntaxrules.procedural_continuous_assignment = true
```

1 change: 1 addition & 0 deletions md/ruleset-verifintent.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ syntaxrules.enum_with_type = true
syntaxrules.keyword_forbidden_priority = true
syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
syntaxrules.procedural_continuous_assignment = true
```

This ruleset has further rules which don't depend on each other or combine
Expand Down
21 changes: 21 additions & 0 deletions md/syntaxrules-explanation-procedural_continuous_assignment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
Continuous assignment, e.g. `assign x = y;` outside of any `always` process,
continuously drives the LHS and changes with any change on the RHS.
The same keyword `assign` has different meaning when used within an `always`
process (or `always_ff`, `always_comb`, `initial`, etc.) where it can be used
to override procedural assignments.
Using this construct in a procedural block (`always*`) which is only triggered
on changes to signals in the sensitivity list may not be synthesizable.

This SystemVerilog language feature is being considered for deprecation, as
noted in IEEE1800-2017 Annex C, because it is easily abused and difficult to
implement while not providing additional capability.
Users are strongly encouraged to migrate their cod to use one of the alternate
methods of procedural or continuous assignments.

See also:
- **non_blocking_assignment_in_always_comb** - Useful companion rule.
- **blocking_assignment_in_always_ff** - Useful companion rule.

The most relevant clauses of IEEE1800-2017 are:
- 10.6.1 The assign and deassign procedural statements
- Annex C.4 Constructs identified for deprecation
1 change: 1 addition & 0 deletions rulesets/DaveMcEwan-design.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
syntaxrules.general_always_no_edge = true
syntaxrules.operator_case_equality = true
syntaxrules.procedural_continuous_assignment = true

# Common to **ruleset-designintent**.
syntaxrules.action_block_with_side_effect = true
Expand Down
1 change: 1 addition & 0 deletions rulesets/designintent.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
#syntaxrules.general_always_no_edge = true # Redundant with keyword_forbidden_always.
syntaxrules.operator_case_equality = true
syntaxrules.procedural_continuous_assignment = true
syntaxrules.action_block_with_side_effect = true
syntaxrules.default_nettype_none = true
syntaxrules.function_same_as_system_function = true
Expand Down
1 change: 1 addition & 0 deletions rulesets/designintentV2001.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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.procedural_continuous_assignment = true
syntaxrules.module_ansi_forbidden = true
syntaxrules.general_always_level_sensitive = true
syntaxrules.blocking_assignment_in_always_at_edge = true
Expand Down
1 change: 1 addition & 0 deletions rulesets/simsynth.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
syntaxrules.general_always_no_edge = true
syntaxrules.operator_case_equality = true
syntaxrules.procedural_continuous_assignment = true
1 change: 1 addition & 0 deletions rulesets/verifintent.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ syntaxrules.enum_with_type = true
syntaxrules.keyword_forbidden_priority = true
syntaxrules.keyword_forbidden_unique = true
syntaxrules.keyword_forbidden_unique0 = true
syntaxrules.procedural_continuous_assignment = true
syntaxrules.action_block_with_side_effect = true
syntaxrules.default_nettype_none = true
syntaxrules.function_same_as_system_function = true
Expand Down
38 changes: 38 additions & 0 deletions src/syntaxrules/procedural_continuous_assignment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
use crate::config::ConfigOption;
use crate::linter::{SyntaxRule, SyntaxRuleResult};
use sv_parser::{NodeEvent, RefNode, SyntaxTree};

#[derive(Default)]
pub struct ProceduralContinuousAssignment;

impl SyntaxRule for ProceduralContinuousAssignment {
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::ProceduralContinuousAssignmentAssign(_) => SyntaxRuleResult::Fail,
_ => SyntaxRuleResult::Pass,
}
}

fn name(&self) -> String {
String::from("procedural_continuous_assignment")
}

fn hint(&self, _option: &ConfigOption) -> String {
String::from("Move `assign` out of `always` block.")
}

fn reason(&self) -> String {
String::from("Procedural continuous assigments are not synthesizable.")
}
}
19 changes: 19 additions & 0 deletions testcases/syntaxrules/fail/procedural_continuous_assignment.sv
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module M;
always @*
assign c = a + b;
endmodule
////////////////////////////////////////////////////////////////////////////////
module M;
always_comb
assign c = a + b;
endmodule
////////////////////////////////////////////////////////////////////////////////
module M;
always_latch
assign c = a + b;
endmodule
////////////////////////////////////////////////////////////////////////////////
module M;
always_ff @(posedge clk)
assign c = a + b;
endmodule
18 changes: 18 additions & 0 deletions testcases/syntaxrules/pass/procedural_continuous_assignment.sv
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
module M;
assign c = a + b; // Continuous assignment
endmodule
////////////////////////////////////////////////////////////////////////////////
module M;
always_ff @(posedge clk)
c <= a + b; // Procedural non-blocking assignment
endmodule
////////////////////////////////////////////////////////////////////////////////
module M;
always_comb
c = a + b; // Procedural blocking assignment
endmodule
////////////////////////////////////////////////////////////////////////////////
module M;
always @*
c = a + b; // Procedural blocking assignment, Verilog 2001
endmodule

0 comments on commit 1200f0a

Please sign in to comment.