From b96b20b9ab72da7b3561618ff79ae4517a7b50a0 Mon Sep 17 00:00:00 2001 From: Ronit Nallagatla Date: Thu, 15 Feb 2024 17:38:30 -0500 Subject: [PATCH 01/16] New Rule: Forbid unpacked array declarations --- md/syntaxrules-explanation-unpacked_array.md | 8 +++++ src/syntaxrules/unpacked_array.rs | 38 ++++++++++++++++++++ testcases/syntaxrules/fail/unpacked_array.sv | 11 ++++++ testcases/syntaxrules/pass/unpacked_array.sv | 11 ++++++ 4 files changed, 68 insertions(+) create mode 100644 md/syntaxrules-explanation-unpacked_array.md create mode 100644 src/syntaxrules/unpacked_array.rs create mode 100644 testcases/syntaxrules/fail/unpacked_array.sv create mode 100644 testcases/syntaxrules/pass/unpacked_array.sv diff --git a/md/syntaxrules-explanation-unpacked_array.md b/md/syntaxrules-explanation-unpacked_array.md new file mode 100644 index 0000000..92f9b2d --- /dev/null +++ b/md/syntaxrules-explanation-unpacked_array.md @@ -0,0 +1,8 @@ +This rule forbids unpacked array declarations. + +Unpacked arrays are not guaranteed to be represented as contiguous memory, and can cause issues with synthesis tools, especially with how multidimensional arrays are synthesized. For example, a synthesis tool might synthesize out unused memory locations of an unpacked array which is not the intended behavior. + +Additionally, packed arrays allow the user to intuitively index and slice the array and apply bitwise operations. + +The most relevant clauses of IEEE1800-2017 are: +- 7.4 Packed and unpacked arrays diff --git a/src/syntaxrules/unpacked_array.rs b/src/syntaxrules/unpacked_array.rs new file mode 100644 index 0000000..d1528f6 --- /dev/null +++ b/src/syntaxrules/unpacked_array.rs @@ -0,0 +1,38 @@ +use crate::config::ConfigOption; +use crate::linter::{SyntaxRule, SyntaxRuleResult}; +use sv_parser::{NodeEvent, RefNode, SyntaxTree}; + +#[derive(Default)] +pub struct UnpackedArray; + +impl SyntaxRule for UnpackedArray { + 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::UnpackedDimension(_) => SyntaxRuleResult::Fail, + _ => SyntaxRuleResult::Pass, + } + } + fn name(&self) -> String { + String::from("unpacked_array") + } + + fn hint(&self, _option: &ConfigOption) -> String { + String::from("Avoid using unpacked arrays in variable declarations.") + } + + fn reason(&self) -> String { + String::from("Unpacked arrays can lead to issues during synthesis.") + } +} diff --git a/testcases/syntaxrules/fail/unpacked_array.sv b/testcases/syntaxrules/fail/unpacked_array.sv new file mode 100644 index 0000000..3deb146 --- /dev/null +++ b/testcases/syntaxrules/fail/unpacked_array.sv @@ -0,0 +1,11 @@ +module M; + +logic a [7:0]; + +endmodule; +//////////////////////////////////////////////////////////////////////////////// +module M; + +logic [31:0] b [0:7]; + +endmodule; diff --git a/testcases/syntaxrules/pass/unpacked_array.sv b/testcases/syntaxrules/pass/unpacked_array.sv new file mode 100644 index 0000000..7bbd970 --- /dev/null +++ b/testcases/syntaxrules/pass/unpacked_array.sv @@ -0,0 +1,11 @@ +module M; + +logic [31:0] a; + +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + +logic [7:0][3:0] b; + +endmodule From 8ddba9a76e8d1dddb90d097ec8ab7edab14cbf3c Mon Sep 17 00:00:00 2001 From: ronitnallagatla Date: Thu, 15 Feb 2024 22:39:58 +0000 Subject: [PATCH 02/16] Commit from GitHub Actions (Run mdgen) --- MANUAL.md | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/MANUAL.md b/MANUAL.md index 5685047..df06c45 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -5326,6 +5326,67 @@ The most relevant clauses of IEEE1800-2017 are: - 12.7 Loop statements + +* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + +## Syntax Rule: `unpacked_array` + +### Hint + +Avoid using unpacked arrays in variable declarations. + +### Reason + +Unpacked arrays can lead to issues during synthesis. + +### Pass Example (1 of 2) +```systemverilog +module M; + +logic [31:0] a; + +endmodule +``` + +### Pass Example (2 of 2) +```systemverilog +module M; + +logic [7:0][3:0] b; + +endmodule +``` + +### Fail Example (1 of 2) +```systemverilog +module M; + +logic a [7:0]; + +endmodule; +``` + +### Fail Example (2 of 2) +```systemverilog +module M; + +logic [31:0] b [0:7]; + +endmodule; +``` + +### Explanation + +This rule forbids unpacked array declarations. + +Unpacked arrays are not guaranteed to be represented as contiguous memory, and can cause issues with synthesis tools, especially with how multidimensional arrays are synthesized. For example, a synthesis tool might synthesize out unused memory locations of an unpacked array which is not the intended behavior. + +Additionally, packed arrays allow the user to intuitively index and slice the array and apply bitwise operations. + +The most relevant clauses of IEEE1800-2017 are: +- 7.4 Packed and unpacked arrays + + # Naming Convention Syntax Rules Rules for checking against naming conventions are named with either the suffix From 365e0a063f644a1949215b9e21f0f1cf1488db90 Mon Sep 17 00:00:00 2001 From: Ronit Nallagatla Date: Mon, 19 Feb 2024 13:01:19 -0500 Subject: [PATCH 03/16] docs: Fix explanation line length. Add to contributors --- CONTRIBUTING.md | 1 + md/syntaxrules-explanation-unpacked_array.md | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9a51b28..3a2f899 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -147,3 +147,4 @@ under the Developer Certificate of Origin . - Taichi Ishitani (@taichi-ishitani) - Sosuke Hosokawa (@so298) - Jan Remes (@remes-codasip) +- Ronit Nallagatla (@ronitnallagatla) diff --git a/md/syntaxrules-explanation-unpacked_array.md b/md/syntaxrules-explanation-unpacked_array.md index 92f9b2d..061e033 100644 --- a/md/syntaxrules-explanation-unpacked_array.md +++ b/md/syntaxrules-explanation-unpacked_array.md @@ -1,8 +1,12 @@ This rule forbids unpacked array declarations. -Unpacked arrays are not guaranteed to be represented as contiguous memory, and can cause issues with synthesis tools, especially with how multidimensional arrays are synthesized. For example, a synthesis tool might synthesize out unused memory locations of an unpacked array which is not the intended behavior. +Unpacked arrays are not guaranteed to be represented as contiguous memory, and +can cause issues with synthesis tools, especially with how multidimensional +arrays are synthesized. For example, a synthesis tool might synthesize out +unused memory locations of an unpacked array which is not the intended behavior. -Additionally, packed arrays allow the user to intuitively index and slice the array and apply bitwise operations. +Additionally, packed arrays allow the user to intuitively index and slice the +array and apply bitwise operations. The most relevant clauses of IEEE1800-2017 are: - 7.4 Packed and unpacked arrays From 2dc72739db4b9e368dd7e5c2d94d9912112ce0c3 Mon Sep 17 00:00:00 2001 From: ronitnallagatla Date: Mon, 19 Feb 2024 18:03:16 +0000 Subject: [PATCH 04/16] Commit from GitHub Actions (Run mdgen) --- MANUAL.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/MANUAL.md b/MANUAL.md index df06c45..03833b7 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -5379,9 +5379,13 @@ endmodule; This rule forbids unpacked array declarations. -Unpacked arrays are not guaranteed to be represented as contiguous memory, and can cause issues with synthesis tools, especially with how multidimensional arrays are synthesized. For example, a synthesis tool might synthesize out unused memory locations of an unpacked array which is not the intended behavior. +Unpacked arrays are not guaranteed to be represented as contiguous memory, and +can cause issues with synthesis tools, especially with how multidimensional +arrays are synthesized. For example, a synthesis tool might synthesize out +unused memory locations of an unpacked array which is not the intended behavior. -Additionally, packed arrays allow the user to intuitively index and slice the array and apply bitwise operations. +Additionally, packed arrays allow the user to intuitively index and slice the +array and apply bitwise operations. The most relevant clauses of IEEE1800-2017 are: - 7.4 Packed and unpacked arrays From fd432a4a42e7f8e93e443cc0bad0eaabb44ff7ff Mon Sep 17 00:00:00 2001 From: Ronit Nallagatla Date: Mon, 19 Feb 2024 13:57:38 -0500 Subject: [PATCH 05/16] testcases: added more fail testcases for unpacked_array --- testcases/syntaxrules/fail/unpacked_array.sv | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/testcases/syntaxrules/fail/unpacked_array.sv b/testcases/syntaxrules/fail/unpacked_array.sv index 3deb146..876429e 100644 --- a/testcases/syntaxrules/fail/unpacked_array.sv +++ b/testcases/syntaxrules/fail/unpacked_array.sv @@ -9,3 +9,15 @@ module M; logic [31:0] b [0:7]; endmodule; +//////////////////////////////////////////////////////////////////////////////// +module M; + +localparam bit [7:0] ARRAY [0:3]; + +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M ( + input logic [7:0] a_in [0:5] +); + +endmodule From b9836ccaf3c9943a9eb43d32d5187e90aaeec545 Mon Sep 17 00:00:00 2001 From: ronitnallagatla Date: Mon, 19 Feb 2024 19:04:26 +0000 Subject: [PATCH 06/16] Commit from GitHub Actions (Run mdgen) --- MANUAL.md | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/MANUAL.md b/MANUAL.md index 03833b7..a0eb889 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -5357,7 +5357,7 @@ logic [7:0][3:0] b; endmodule ``` -### Fail Example (1 of 2) +### Fail Example (1 of 4) ```systemverilog module M; @@ -5366,7 +5366,7 @@ logic a [7:0]; endmodule; ``` -### Fail Example (2 of 2) +### Fail Example (2 of 4) ```systemverilog module M; @@ -5375,6 +5375,24 @@ logic [31:0] b [0:7]; endmodule; ``` +### Fail Example (3 of 4) +```systemverilog +module M; + +localparam bit [7:0] ARRAY [0:3]; + +endmodule +``` + +### Fail Example (4 of 4) +```systemverilog +module M ( + input logic [7:0] a_in [0:5] +); + +endmodule +``` + ### Explanation This rule forbids unpacked array declarations. From b912f19cb2c5377a351b5476ba499d6f31df5e8b Mon Sep 17 00:00:00 2001 From: Ronit Nallagatla Date: Tue, 9 Apr 2024 13:46:15 -0400 Subject: [PATCH 07/16] updated config option with unpacked_array opt struct --- src/config.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index a3defcc..24bb9c0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,4 +1,4 @@ -use crate::linter::{TextRule, SyntaxRule}; +use crate::linter::{SyntaxRule, TextRule}; use crate::rules::*; use regex::Regex; use serde_derive::{Deserialize, Serialize}; @@ -158,6 +158,35 @@ pub struct ConfigOption { pub re_forbidden_var_class: String, #[serde(default = "default_re_unconfigured")] pub re_forbidden_var_classmethod: String, + + #[serde(default)] + pub unpacked_array: UnpackedArrayOption, +} + +#[derive(Clone, Debug, Deserialize, Serialize, Default)] +pub struct UnpackedArrayOption { + #[serde(default = "default_as_false")] + pub localparam_decl: bool, + #[serde(default = "default_as_false")] + pub param_decl: bool, + #[serde(default = "default_as_false")] + pub specparam_decl: bool, + #[serde(default = "default_as_false")] + pub inout_decl: bool, + #[serde(default = "default_as_false")] + pub ansi_port_decl: bool, + #[serde(default = "default_as_false")] + pub input_decl: bool, + #[serde(default = "default_as_false")] + pub output_decl: bool, + #[serde(default = "default_as_false")] + pub intf_port_decl: bool, + #[serde(default = "default_as_false")] + pub ref_decl: bool, + #[serde(default = "default_as_false")] + pub data_decl: bool, + #[serde(default = "default_as_false")] + pub net_decl: bool, } include!(concat!(env!("OUT_DIR"), "/config_rules.rs")); From 800ee691af73252afd21304c1d142804e59506bb Mon Sep 17 00:00:00 2001 From: Ronit Nallagatla Date: Tue, 9 Apr 2024 13:47:48 -0400 Subject: [PATCH 08/16] modify unpacked_array impl to check option --- src/syntaxrules/unpacked_array.rs | 115 +++++++++++++++++++++++++++--- 1 file changed, 107 insertions(+), 8 deletions(-) diff --git a/src/syntaxrules/unpacked_array.rs b/src/syntaxrules/unpacked_array.rs index d1528f6..c23c24e 100644 --- a/src/syntaxrules/unpacked_array.rs +++ b/src/syntaxrules/unpacked_array.rs @@ -3,25 +3,124 @@ use crate::linter::{SyntaxRule, SyntaxRuleResult}; use sv_parser::{NodeEvent, RefNode, SyntaxTree}; #[derive(Default)] -pub struct UnpackedArray; +pub struct UnpackedArray { + under_localparam_decl: bool, + under_param_decl: bool, + under_specparam_decl: bool, + under_inout_decl: bool, + under_ansi_port_decl: bool, + under_input_decl: bool, + under_output_decl: bool, + under_intf_port_decl: bool, + under_ref_decl: bool, + under_data_decl: bool, + under_net_decl: bool, +} impl SyntaxRule for UnpackedArray { fn check( &mut self, _syntax_tree: &SyntaxTree, event: &NodeEvent, - _option: &ConfigOption, + option: &ConfigOption, ) -> SyntaxRuleResult { let node = match event { - NodeEvent::Enter(x) => x, - NodeEvent::Leave(_) => { + NodeEvent::Enter(x) => { + match x { + RefNode::LocalParameterDeclaration(_) => { + self.under_localparam_decl = true; + } + RefNode::ParameterDeclaration(_) => { + self.under_param_decl = true; + } + RefNode::SpecparamDeclaration(_) => { + self.under_specparam_decl = true; + } + RefNode::InoutDeclaration(_) => { + self.under_inout_decl = true; + } + RefNode::AnsiPortDeclaration(_) => { + self.under_ansi_port_decl = true; + } + RefNode::InputDeclaration(_) => { + self.under_input_decl = true; + } + RefNode::OutputDeclaration(_) => { + self.under_output_decl = true; + } + RefNode::InterfacePortDeclaration(_) => { + self.under_intf_port_decl = true; + } + RefNode::RefDeclaration(_) => { + self.under_ref_decl = true; + } + RefNode::DataDeclaration(_) => { + self.under_data_decl = true; + } + RefNode::NetDeclaration(_) => { + self.under_net_decl = true; + } + + _ => (), + } + x + } + NodeEvent::Leave(x) => { + match x { + RefNode::LocalParameterDeclaration(_) => { + self.under_localparam_decl = false; + } + RefNode::ParameterDeclaration(_) => { + self.under_param_decl = false; + } + RefNode::SpecparamDeclaration(_) => { + self.under_specparam_decl = false; + } + RefNode::InoutDeclaration(_) => { + self.under_inout_decl = false; + } + RefNode::InputDeclaration(_) => { + self.under_input_decl = false; + } + RefNode::OutputDeclaration(_) => { + self.under_output_decl = false; + } + RefNode::InterfacePortDeclaration(_) => { + self.under_intf_port_decl = false; + } + RefNode::RefDeclaration(_) => { + self.under_ref_decl = false; + } + RefNode::DataDeclaration(_) => { + self.under_data_decl = false; + } + RefNode::NetDeclaration(_) => { + self.under_net_decl = false; + } + + _ => (), + } return SyntaxRuleResult::Pass; } }; - match node { - RefNode::UnpackedDimension(_) => SyntaxRuleResult::Fail, - _ => SyntaxRuleResult::Pass, + if let (true, RefNode::UnpackedDimension(_)) = ( + (self.under_localparam_decl && option.unpacked_array.localparam_decl + || self.under_param_decl && option.unpacked_array.param_decl + || self.under_specparam_decl && option.unpacked_array.specparam_decl + || self.under_inout_decl && option.unpacked_array.inout_decl + || self.under_ansi_port_decl && option.unpacked_array.ansi_port_decl + || self.under_input_decl && option.unpacked_array.input_decl + || self.under_output_decl && option.unpacked_array.output_decl + || self.under_intf_port_decl && option.unpacked_array.intf_port_decl + || self.under_ref_decl && option.unpacked_array.ref_decl + || self.under_data_decl && option.unpacked_array.data_decl + || self.under_net_decl && option.unpacked_array.net_decl), + node, + ) { + SyntaxRuleResult::Fail + } else { + SyntaxRuleResult::Pass } } fn name(&self) -> String { @@ -29,7 +128,7 @@ impl SyntaxRule for UnpackedArray { } fn hint(&self, _option: &ConfigOption) -> String { - String::from("Avoid using unpacked arrays in variable declarations.") + String::from("Avoid using unpacked dimensions in declarations.") } fn reason(&self) -> String { From ddd1df74bd000a04a50be4fdbc10ff9325926692 Mon Sep 17 00:00:00 2001 From: Ronit Nallagatla Date: Tue, 9 Apr 2024 13:48:20 -0400 Subject: [PATCH 09/16] added more unpacked_array testcases --- testcases/syntaxrules/fail/unpacked_array.sv | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/testcases/syntaxrules/fail/unpacked_array.sv b/testcases/syntaxrules/fail/unpacked_array.sv index 876429e..fa437b0 100644 --- a/testcases/syntaxrules/fail/unpacked_array.sv +++ b/testcases/syntaxrules/fail/unpacked_array.sv @@ -19,5 +19,22 @@ endmodule module M ( input logic [7:0] a_in [0:5] ); +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + +parameter [3:0] ARRAY [0:1]; + +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + +wire [3:0] c [0:1]; + +endmodule +//////////////////////////////////////////////////////////////////////////////// +module M; + +var [3:0] d [0:1]; endmodule From 4f1106f87c7af2ddd3ccec837c551340750e7dda Mon Sep 17 00:00:00 2001 From: Ronit Nallagatla Date: Tue, 9 Apr 2024 16:16:16 -0400 Subject: [PATCH 10/16] changed defaults to true to pass cargo test --- src/config.rs | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/config.rs b/src/config.rs index 24bb9c0..db81a99 100644 --- a/src/config.rs +++ b/src/config.rs @@ -163,29 +163,18 @@ pub struct ConfigOption { pub unpacked_array: UnpackedArrayOption, } -#[derive(Clone, Debug, Deserialize, Serialize, Default)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct UnpackedArrayOption { - #[serde(default = "default_as_false")] pub localparam_decl: bool, - #[serde(default = "default_as_false")] pub param_decl: bool, - #[serde(default = "default_as_false")] pub specparam_decl: bool, - #[serde(default = "default_as_false")] pub inout_decl: bool, - #[serde(default = "default_as_false")] pub ansi_port_decl: bool, - #[serde(default = "default_as_false")] pub input_decl: bool, - #[serde(default = "default_as_false")] pub output_decl: bool, - #[serde(default = "default_as_false")] pub intf_port_decl: bool, - #[serde(default = "default_as_false")] pub ref_decl: bool, - #[serde(default = "default_as_false")] pub data_decl: bool, - #[serde(default = "default_as_false")] pub net_decl: bool, } @@ -209,6 +198,24 @@ impl Default for ConfigSyntaxRules { } } +impl Default for UnpackedArrayOption { + fn default() -> Self { + Self { + localparam_decl: true, + param_decl: true, + specparam_decl: true, + inout_decl: true, + ansi_port_decl: true, + input_decl: true, + output_decl: true, + intf_port_decl: true, + ref_decl: true, + data_decl: true, + net_decl: true, + } + } +} + #[allow(dead_code)] fn default_as_true() -> bool { true From c6f12ee76aa706cd6280e7ed0bb6ece2b50baea8 Mon Sep 17 00:00:00 2001 From: ronitnallagatla Date: Tue, 9 Apr 2024 20:18:15 +0000 Subject: [PATCH 11/16] Commit from GitHub Actions (Run mdgen) --- MANUAL.md | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/MANUAL.md b/MANUAL.md index a0eb889..95a95db 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -5333,7 +5333,7 @@ The most relevant clauses of IEEE1800-2017 are: ### Hint -Avoid using unpacked arrays in variable declarations. +Avoid using unpacked dimensions in declarations. ### Reason @@ -5357,7 +5357,7 @@ logic [7:0][3:0] b; endmodule ``` -### Fail Example (1 of 4) +### Fail Example (1 of 7) ```systemverilog module M; @@ -5366,7 +5366,7 @@ logic a [7:0]; endmodule; ``` -### Fail Example (2 of 4) +### Fail Example (2 of 7) ```systemverilog module M; @@ -5375,7 +5375,7 @@ logic [31:0] b [0:7]; endmodule; ``` -### Fail Example (3 of 4) +### Fail Example (3 of 7) ```systemverilog module M; @@ -5384,11 +5384,37 @@ localparam bit [7:0] ARRAY [0:3]; endmodule ``` -### Fail Example (4 of 4) +### Fail Example (4 of 7) ```systemverilog module M ( input logic [7:0] a_in [0:5] ); +endmodule +``` + +### Fail Example (5 of 7) +```systemverilog +module M; + +parameter [3:0] ARRAY [0:1]; + +endmodule +``` + +### Fail Example (6 of 7) +```systemverilog +module M; + +wire [3:0] c [0:1]; + +endmodule +``` + +### Fail Example (7 of 7) +```systemverilog +module M; + +var [3:0] d [0:1]; endmodule ``` From 4bdb62b55eb4c94a302d3ebf8bdcae287a5ba295 Mon Sep 17 00:00:00 2001 From: Ronit Nallagatla Date: Fri, 12 Apr 2024 15:00:23 -0400 Subject: [PATCH 12/16] unpacked_array: update explanation for new config options --- md/syntaxrules-explanation-unpacked_array.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/md/syntaxrules-explanation-unpacked_array.md b/md/syntaxrules-explanation-unpacked_array.md index 061e033..c13c2a8 100644 --- a/md/syntaxrules-explanation-unpacked_array.md +++ b/md/syntaxrules-explanation-unpacked_array.md @@ -8,5 +8,25 @@ unused memory locations of an unpacked array which is not the intended behavior. Additionally, packed arrays allow the user to intuitively index and slice the array and apply bitwise operations. +This rule by default targets data declarations, but can be configured to target +other declarations. To target a declaration, enable the corresponding boolean +option in the configuration file. + +``` toml +[option.unpacked_array] +localparam_declaration = false +param_declaration = false +specparam_declaration = false +inout_declaration = false +ansi_port_declaration = false +input_declaration = false +output_declaration = false +intf_port_declaration = false +ref_declaration = false +data_declaration = true # enabled by default +net_declaration = false +``` + The most relevant clauses of IEEE1800-2017 are: + - 7.4 Packed and unpacked arrays From f9fed223cbea719ec03d0365979d79ae272b07a8 Mon Sep 17 00:00:00 2001 From: ronitnallagatla Date: Fri, 12 Apr 2024 19:01:42 +0000 Subject: [PATCH 13/16] Commit from GitHub Actions (Run mdgen) --- MANUAL.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/MANUAL.md b/MANUAL.md index 95a95db..2ba2f17 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -5431,7 +5431,27 @@ unused memory locations of an unpacked array which is not the intended behavior. Additionally, packed arrays allow the user to intuitively index and slice the array and apply bitwise operations. +This rule by default targets data declarations, but can be configured to target +other declarations. To target a declaration, enable the corresponding boolean +option in the configuration file. + +``` toml +[option.unpacked_array] +localparam_declaration = false +param_declaration = false +specparam_declaration = false +inout_declaration = false +ansi_port_declaration = false +input_declaration = false +output_declaration = false +intf_port_declaration = false +ref_declaration = false +data_declaration = true # enabled by default +net_declaration = false +``` + The most relevant clauses of IEEE1800-2017 are: + - 7.4 Packed and unpacked arrays From ed6af0b9c3f13db656bb02c79cb4729c2bdab072 Mon Sep 17 00:00:00 2001 From: Ronit Nallagatla Date: Fri, 12 Apr 2024 15:04:21 -0400 Subject: [PATCH 14/16] unpacked_array: target data decl on default --- src/config.rs | 55 ++++++++++++-------- src/syntaxrules/unpacked_array.rs | 26 ++++----- testcases/syntaxrules/fail/unpacked_array.sv | 29 ----------- 3 files changed, 47 insertions(+), 63 deletions(-) diff --git a/src/config.rs b/src/config.rs index db81a99..1842ffe 100644 --- a/src/config.rs +++ b/src/config.rs @@ -165,17 +165,28 @@ pub struct ConfigOption { #[derive(Clone, Debug, Deserialize, Serialize)] pub struct UnpackedArrayOption { - pub localparam_decl: bool, - pub param_decl: bool, - pub specparam_decl: bool, - pub inout_decl: bool, - pub ansi_port_decl: bool, - pub input_decl: bool, - pub output_decl: bool, - pub intf_port_decl: bool, - pub ref_decl: bool, - pub data_decl: bool, - pub net_decl: bool, + #[serde(default)] + pub localparam_declaration: bool, + #[serde(default)] + pub param_declaration: bool, + #[serde(default)] + pub specparam_declaration: bool, + #[serde(default)] + pub inout_declaration: bool, + #[serde(default)] + pub ansi_port_declaration: bool, + #[serde(default)] + pub input_declaration: bool, + #[serde(default)] + pub output_declaration: bool, + #[serde(default)] + pub interface_port_declaration: bool, + #[serde(default)] + pub ref_declaration: bool, + #[serde(default)] + pub data_declaration: bool, + #[serde(default)] + pub net_declaration: bool, } include!(concat!(env!("OUT_DIR"), "/config_rules.rs")); @@ -201,17 +212,17 @@ impl Default for ConfigSyntaxRules { impl Default for UnpackedArrayOption { fn default() -> Self { Self { - localparam_decl: true, - param_decl: true, - specparam_decl: true, - inout_decl: true, - ansi_port_decl: true, - input_decl: true, - output_decl: true, - intf_port_decl: true, - ref_decl: true, - data_decl: true, - net_decl: true, + localparam_declaration: false, + param_declaration: false, + specparam_declaration: false, + inout_declaration: false, + ansi_port_declaration: false, + input_declaration: false, + output_declaration: false, + interface_port_declaration: false, + ref_declaration: false, + data_declaration: true, + net_declaration: false, } } } diff --git a/src/syntaxrules/unpacked_array.rs b/src/syntaxrules/unpacked_array.rs index c23c24e..7a40e41 100644 --- a/src/syntaxrules/unpacked_array.rs +++ b/src/syntaxrules/unpacked_array.rs @@ -105,17 +105,17 @@ impl SyntaxRule for UnpackedArray { }; if let (true, RefNode::UnpackedDimension(_)) = ( - (self.under_localparam_decl && option.unpacked_array.localparam_decl - || self.under_param_decl && option.unpacked_array.param_decl - || self.under_specparam_decl && option.unpacked_array.specparam_decl - || self.under_inout_decl && option.unpacked_array.inout_decl - || self.under_ansi_port_decl && option.unpacked_array.ansi_port_decl - || self.under_input_decl && option.unpacked_array.input_decl - || self.under_output_decl && option.unpacked_array.output_decl - || self.under_intf_port_decl && option.unpacked_array.intf_port_decl - || self.under_ref_decl && option.unpacked_array.ref_decl - || self.under_data_decl && option.unpacked_array.data_decl - || self.under_net_decl && option.unpacked_array.net_decl), + (self.under_localparam_decl && option.unpacked_array.localparam_declaration + || self.under_param_decl && option.unpacked_array.param_declaration + || self.under_specparam_decl && option.unpacked_array.specparam_declaration + || self.under_inout_decl && option.unpacked_array.inout_declaration + || self.under_ansi_port_decl && option.unpacked_array.ansi_port_declaration + || self.under_input_decl && option.unpacked_array.input_declaration + || self.under_output_decl && option.unpacked_array.output_declaration + || self.under_intf_port_decl && option.unpacked_array.interface_port_declaration + || self.under_ref_decl && option.unpacked_array.ref_declaration + || self.under_data_decl && option.unpacked_array.data_declaration + || self.under_net_decl && option.unpacked_array.net_declaration), node, ) { SyntaxRuleResult::Fail @@ -132,6 +132,8 @@ impl SyntaxRule for UnpackedArray { } fn reason(&self) -> String { - String::from("Unpacked arrays can lead to issues during synthesis.") + String::from( + "Unpacked arrays are not guaranteed to be contiguous and can lead to synthesis issues.", + ) } } diff --git a/testcases/syntaxrules/fail/unpacked_array.sv b/testcases/syntaxrules/fail/unpacked_array.sv index fa437b0..3deb146 100644 --- a/testcases/syntaxrules/fail/unpacked_array.sv +++ b/testcases/syntaxrules/fail/unpacked_array.sv @@ -9,32 +9,3 @@ module M; logic [31:0] b [0:7]; endmodule; -//////////////////////////////////////////////////////////////////////////////// -module M; - -localparam bit [7:0] ARRAY [0:3]; - -endmodule -//////////////////////////////////////////////////////////////////////////////// -module M ( - input logic [7:0] a_in [0:5] -); -endmodule -//////////////////////////////////////////////////////////////////////////////// -module M; - -parameter [3:0] ARRAY [0:1]; - -endmodule -//////////////////////////////////////////////////////////////////////////////// -module M; - -wire [3:0] c [0:1]; - -endmodule -//////////////////////////////////////////////////////////////////////////////// -module M; - -var [3:0] d [0:1]; - -endmodule From ed8a016d19cc00580679ee691305c5b0c8850465 Mon Sep 17 00:00:00 2001 From: ronitnallagatla Date: Fri, 12 Apr 2024 19:05:35 +0000 Subject: [PATCH 15/16] Commit from GitHub Actions (Run mdgen) --- MANUAL.md | 50 +++----------------------------------------------- 1 file changed, 3 insertions(+), 47 deletions(-) diff --git a/MANUAL.md b/MANUAL.md index 2ba2f17..14d3c05 100644 --- a/MANUAL.md +++ b/MANUAL.md @@ -5337,7 +5337,7 @@ Avoid using unpacked dimensions in declarations. ### Reason -Unpacked arrays can lead to issues during synthesis. +Unpacked arrays are not guaranteed to be contiguous and can lead to synthesis issues. ### Pass Example (1 of 2) ```systemverilog @@ -5357,7 +5357,7 @@ logic [7:0][3:0] b; endmodule ``` -### Fail Example (1 of 7) +### Fail Example (1 of 2) ```systemverilog module M; @@ -5366,7 +5366,7 @@ logic a [7:0]; endmodule; ``` -### Fail Example (2 of 7) +### Fail Example (2 of 2) ```systemverilog module M; @@ -5375,50 +5375,6 @@ logic [31:0] b [0:7]; endmodule; ``` -### Fail Example (3 of 7) -```systemverilog -module M; - -localparam bit [7:0] ARRAY [0:3]; - -endmodule -``` - -### Fail Example (4 of 7) -```systemverilog -module M ( - input logic [7:0] a_in [0:5] -); -endmodule -``` - -### Fail Example (5 of 7) -```systemverilog -module M; - -parameter [3:0] ARRAY [0:1]; - -endmodule -``` - -### Fail Example (6 of 7) -```systemverilog -module M; - -wire [3:0] c [0:1]; - -endmodule -``` - -### Fail Example (7 of 7) -```systemverilog -module M; - -var [3:0] d [0:1]; - -endmodule -``` - ### Explanation This rule forbids unpacked array declarations. From 5965cd0bbf33e135d738b42b40673672f1637c1a Mon Sep 17 00:00:00 2001 From: Ronit Nallagatla Date: Sun, 2 Jun 2024 22:06:40 -0400 Subject: [PATCH 16/16] fix contributing.md merge conflict --- CONTRIBUTING.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3a2f899..9a51b28 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -147,4 +147,3 @@ under the Developer Certificate of Origin . - Taichi Ishitani (@taichi-ishitani) - Sosuke Hosokawa (@so298) - Jan Remes (@remes-codasip) -- Ronit Nallagatla (@ronitnallagatla)