From 5d114db267efa5b3776471752844ad1eec2b7eba Mon Sep 17 00:00:00 2001 From: tignear Date: Wed, 7 Oct 2026 08:47:52 +0900 Subject: [PATCH 1/5] fix(sv): nested default patterns, int parameter sign, loop bound width, and $size dimensions - A `default:` item of an unpacked-array pattern now applies to each element of an unmatched subarray (IEEE 1800-2023 10.9.1) instead of the subarray. - `int` parameters keep their sign through two-state conversion. - A folded run-time `for` bound is compared at the common width of the loop variable and the bound. - `$size(, dim)` and `$size(, dim)` honor the dimension. --- crates/celox-frontend-sv/src/lowering.rs | 10 +- crates/celox-frontend-sv/src/lowering/comb.rs | 23 ++- .../src/lowering/procedural.rs | 21 +++ .../celox-sv-analyzer/src/ast/dimensions.rs | 114 +++++++++----- crates/celox-sv-analyzer/src/ast/patterns.rs | 112 +++++++++++-- crates/celox-test-suite/LIMITATIONS.md | 3 +- .../src/sv/cases/review_regressions.vtest | 148 ++++++++++++++++++ .../verification/sv/icarus.json | 72 ++++++++- .../verification/sv/limitations.json | 20 ++- .../verification/sv/limitations/icarus.json | 18 ++- .../verification/sv/verilator.json | 50 +++++- crates/celox/tests/comb_observer.rs | 2 +- crates/celox/tests/flip_flop.rs | 2 +- .../systemverilog/review_regressions.rs | 16 ++ crates/celox/tests/system_function.rs | 1 - .../celox/tests/veryl_context_regressions.rs | 3 - 16 files changed, 544 insertions(+), 71 deletions(-) diff --git a/crates/celox-frontend-sv/src/lowering.rs b/crates/celox-frontend-sv/src/lowering.rs index cb413e8d3b..52035d64e0 100644 --- a/crates/celox-frontend-sv/src/lowering.rs +++ b/crates/celox-frontend-sv/src/lowering.rs @@ -4960,7 +4960,10 @@ fn sv_glue_expr_is_signed( sv::ir::Expr::Unary { op, expr } => { matches!( op, - sv::ir::UnaryOp::Plus | sv::ir::UnaryOp::Minus | sv::ir::UnaryOp::BitNot + sv::ir::UnaryOp::Plus + | sv::ir::UnaryOp::Minus + | sv::ir::UnaryOp::BitNot + | sv::ir::UnaryOp::ToTwoState ) && sv_glue_expr_is_signed(expr, variables, name_to_id, parameter_types) } sv::ir::Expr::Binary { left, op, right } => match op { @@ -5019,7 +5022,10 @@ fn sv_expr_is_signed_with_parameters( sv::ir::Expr::Unary { op, expr } => { matches!( op, - sv::ir::UnaryOp::Plus | sv::ir::UnaryOp::Minus | sv::ir::UnaryOp::BitNot + sv::ir::UnaryOp::Plus + | sv::ir::UnaryOp::Minus + | sv::ir::UnaryOp::BitNot + | sv::ir::UnaryOp::ToTwoState ) && sv_expr_is_signed_with_parameters(expr, variables, name_to_id, parameter_types) } sv::ir::Expr::Binary { left, op, right } => match op { diff --git a/crates/celox-frontend-sv/src/lowering/comb.rs b/crates/celox-frontend-sv/src/lowering/comb.rs index b445230a74..3fc3b43cfc 100644 --- a/crates/celox-frontend-sv/src/lowering/comb.rs +++ b/crates/celox-frontend-sv/src/lowering/comb.rs @@ -2169,7 +2169,7 @@ impl<'p, 'a> Comb<'p, 'a> { } let mut probe = store.fork(); let (start, _) = self.eval(&mut probe, frames, &canonical.start, None)?; - let (end, _) = self.eval(&mut probe, frames, &canonical.end, None)?; + let (end, _) = self.loop_end(&mut probe, frames, canonical)?; let (Some((start, _)), Some((end, _))) = ( slt_const(self.arena, &mut self.consts, start), slt_const(self.arena, &mut self.consts, end), @@ -2187,6 +2187,25 @@ impl<'p, 'a> Comb<'p, 'a> { Ok(trips > (MAX_UNROLLED_ITERATIONS - self.unrolled.min(MAX_UNROLLED_ITERATIONS)) as f64) } + /// The bound of a counted loop. It is an operand of the relational loop + /// condition, so it is evaluated at the width of the comparison with the + /// loop variable, and signed only when both operands are (IEEE 1800-2023 + /// 11.6.1, 11.8.1 and 11.8.2). + fn loop_end( + &mut self, + store: &mut Store, + frames: &[Frame], + canonical: &CanonicalLoop, + ) -> Result { + let var = sv::ir::Expr::Ident(canonical.var.clone()); + let signed = self.expr_signed(&var) && self.expr_signed(&canonical.end); + let context = self + .m + .comparison_width(&var, &canonical.end) + .map(|width| (width, signed)); + self.eval(store, frames, &canonical.end, context) + } + #[allow(clippy::too_many_arguments)] fn fold_loop( &mut self, @@ -2215,7 +2234,7 @@ impl<'p, 'a> Comb<'p, 'a> { let start = coerce_node_width(self.arena, start, Some(loop_width), loop_signed) .map_err(slt_error)?; let end_signed = self.expr_signed(&canonical.end); - let (mut end, end_sources) = self.eval(&mut store, frames, &canonical.end, None)?; + let (mut end, end_sources) = self.loop_end(&mut store, frames, canonical)?; let mut inclusive = matches!( canonical.compare, sv::ir::BinaryOp::Le | sv::ir::BinaryOp::Ge diff --git a/crates/celox-frontend-sv/src/lowering/procedural.rs b/crates/celox-frontend-sv/src/lowering/procedural.rs index 442b8c6ac6..9f41eaeee2 100644 --- a/crates/celox-frontend-sv/src/lowering/procedural.rs +++ b/crates/celox-frontend-sv/src/lowering/procedural.rs @@ -402,6 +402,27 @@ impl<'a> ProcModule<'a> { ) } + /// The width at which `left` and `right` are compared: the larger of + /// their self-determined widths (IEEE 1800-2023 11.6.1 and 11.8.2), with + /// each user function call typed by its declared return type. + pub fn comparison_width(&self, left: &sv::ir::Expr, right: &sv::ir::Expr) -> Option { + let typed = |expr: &sv::ir::Expr| { + if self.calls(expr) { + self.typed_calls(expr) + } else { + expr.clone() + } + }; + sv_comparison_operand_width( + &typed(left), + &typed(right), + self.variables, + self.name_to_id, + self.constants, + self.parameter_types, + ) + } + /// `expr` with each user function call replaced by a literal of its /// return type. fn typed_calls(&self, expr: &sv::ir::Expr) -> sv::ir::Expr { diff --git a/crates/celox-sv-analyzer/src/ast/dimensions.rs b/crates/celox-sv-analyzer/src/ast/dimensions.rs index 5069f9f76a..03a8986cf8 100644 --- a/crates/celox-sv-analyzer/src/ast/dimensions.rs +++ b/crates/celox-sv-analyzer/src/ast/dimensions.rs @@ -24,10 +24,21 @@ pub(super) fn size_system_function_call_type( type_aliases: &HashMap, dimensions: Option<&PackedDimensions>, ) -> Option { - let (name, r#type) = match call { + // The dimension argument of `$size(x, n)`, a constant expression. + let dimension_argument = |dimension: &sv_parser::Expression| { + const_expr_from_expr(dimension, syntax_tree) + .ok() + .flatten() + .and_then(|dimension| eval_ast_const_expr(&dimension, const_env)) + }; + let (name, r#type, dimension) = match call { sv_parser::SystemTfCall::ArgDataType(call) => { let name = syntax_tree.get_str(&call.nodes.0.nodes.0)?; - let data_type = &call.nodes.1.nodes.1.0; + let (data_type, dimension) = &call.nodes.1.nodes.1; + let dimension = match dimension { + Some((_, dimension)) => Some(dimension_argument(dimension)?), + None => None, + }; let r#type = match data_type { sv_parser::DataType::Type(data_type) => { let name = @@ -41,7 +52,7 @@ pub(super) fn size_system_function_call_type( type_aliases, ), }?; - (name, r#type) + (name, r#type, dimension) } // sv-parser classifies an unqualified typedef argument as an // expression because its grammar cannot know whether the identifier @@ -58,11 +69,13 @@ pub(super) fn size_system_function_call_type( ConstExpr::Ident(identifier) => identifier, _ => return None, }; - let dimension = const_expr_from_expr(dimension, syntax_tree) - .ok() - .flatten() - .and_then(|dimension| eval_ast_const_expr(&dimension, const_env))?; - let variable = dimensions?.get(&identifier)?; + let dimension = dimension_argument(dimension)?; + let Some(variable) = dimensions.and_then(|dimensions| dimensions.get(&identifier)) + else { + // A type name that sv-parser parsed as an expression. + let r#type = type_aliases.get(&identifier)?.clone(); + return type_dimension_size(&r#type, Some(dimension), const_env); + }; let widths: Vec<&ConstExpr> = variable .unpacked .iter() @@ -146,7 +159,7 @@ pub(super) fn size_system_function_call_type( if let ConstExpr::Ident(alias) = &argument && let Some(r#type) = type_aliases.get(alias) { - (name, r#type.clone()) + (name, r#type.clone(), None) } else { if let ConstExpr::Ident(identifier) = &argument && let Some(width) = @@ -167,50 +180,65 @@ pub(super) fn size_system_function_call_type( } sv_parser::SystemTfCall::ArgOptionl(_) => return None, }; - // $bits covers every unpacked and packed dimension; $size covers only - // the outermost dimension, which is unpacked when one is present. - let first_dimension_only = name == "$size"; - if name != "$bits" && !first_dimension_only { - return None; + // $bits covers every unpacked and packed dimension; $size covers one + // dimension, by default the outermost one. + if name == "$size" { + return type_dimension_size(&r#type, dimension, const_env); } - if !first_dimension_only { - let unpacked_width = r#type - .unpacked_ranges() - .iter() - .try_fold(1usize, |width, range| { - let left = eval_ast_const_expr(range.left(), const_env)?; - let right = eval_ast_const_expr(range.right(), const_env)?; - width.checked_mul(usize::try_from(left.abs_diff(right)).ok()?.checked_add(1)?) - })?; - let packed_width = r#type - .packed_ranges() - .iter() - .try_fold(1usize, |width, range| { - let left = eval_ast_const_expr(range.left(), const_env)?; - let right = eval_ast_const_expr(range.right(), const_env)?; - width.checked_mul(usize::try_from(left.abs_diff(right)).ok()?.checked_add(1)?) - })?; - return Some(ExprType { - width: unpacked_width.checked_mul(packed_width)?.max(1), - signed: r#type.is_signed(), - }); + if name != "$bits" || dimension.is_some() { + return None; } - let range = r#type + let unpacked_width = r#type .unpacked_ranges() - .first() + .iter() + .try_fold(1usize, |width, range| { + let left = eval_ast_const_expr(range.left(), const_env)?; + let right = eval_ast_const_expr(range.right(), const_env)?; + width.checked_mul(usize::try_from(left.abs_diff(right)).ok()?.checked_add(1)?) + })?; + let packed_width = r#type + .packed_ranges() + .iter() + .try_fold(1usize, |width, range| { + let left = eval_ast_const_expr(range.left(), const_env)?; + let right = eval_ast_const_expr(range.right(), const_env)?; + width.checked_mul(usize::try_from(left.abs_diff(right)).ok()?.checked_add(1)?) + })?; + Some(ExprType { + width: unpacked_width.checked_mul(packed_width)?.max(1), + signed: r#type.is_signed(), + }) +} + +/// `$size` of a type: the number of elements of dimension `dimension` +/// (default 1), numbered from the slowest varying dimension, unpacked +/// dimensions first (IEEE 1800-2023 20.7). +fn type_dimension_size( + r#type: &Type, + dimension: Option, + const_env: &HashMap, +) -> Option { + let dimension = usize::try_from(dimension.unwrap_or(1)) + .ok()? + .checked_sub(1)?; + let mut ranges = r#type + .unpacked_ranges() + .iter() .map(|range| (range.left(), range.right())) - .or_else(|| { + .chain( r#type .packed_ranges() - .first() - .map(|range| (range.left(), range.right())) - }); - let Some((left, right)) = range else { + .iter() + .map(|range| (range.left(), range.right())), + ) + .peekable(); + if ranges.peek().is_none() && dimension == 0 { return Some(ExprType { width: 1, signed: r#type.is_signed(), }); - }; + } + let (left, right) = ranges.nth(dimension)?; let left = eval_ast_const_expr(left, const_env)?; let right = eval_ast_const_expr(right, const_env)?; let width = usize::try_from(left.abs_diff(right)).ok()?.checked_add(1)?; diff --git a/crates/celox-sv-analyzer/src/ast/patterns.rs b/crates/celox-sv-analyzer/src/ast/patterns.rs index 8c003d0f7a..8ac352abb1 100644 --- a/crates/celox-sv-analyzer/src/ast/patterns.rs +++ b/crates/celox-sv-analyzer/src/ast/patterns.rs @@ -94,6 +94,91 @@ fn element( }) } +/// The value a `default:` key gives an element of `shape`. An unpacked +/// subarray whose type the value does not match is not set as a whole: the +/// default applies recursively to each of its elements (IEEE 1800-2023 +/// 10.9.1), each evaluated in the context of that element's type. +fn default_element( + expr: &sv_parser::Expression, + shape: &VariableDimensions, + tree: &SyntaxTree, + dims: &PackedDimensions, +) -> Converted { + let Some((dimension, rest)) = shape.unpacked.split_first() else { + return element(expr, shape, tree, dims); + }; + if pattern_expression(expr).is_some() || unpacked_rank(expr, tree, dims) >= shape.unpacked.len() + { + return element(expr, shape, tree, dims); + } + let (_, _, count) = dimension_bounds(&dimension.left, &dimension.right, dims)?; + let subarray = VariableDimensions { + unpacked: rest.to_vec(), + ..shape.clone() + }; + let value = default_element(expr, &subarray, tree, dims)?; + Ok(match count { + 1 => value, + _ => Expr::Concat(vec![value; count]), + }) +} + +/// The number of unpacked dimensions of the self-determined type of `expr`: +/// those of a variable that its selects leave unselected. +fn unpacked_rank( + expr: &sv_parser::Expression, + tree: &SyntaxTree, + dims: &PackedDimensions, +) -> usize { + let sv_parser::Expression::Primary(primary) = expr else { + return 0; + }; + match &**primary { + sv_parser::Primary::MintypmaxExpression(paren) => match &paren.nodes.0.nodes.1 { + sv_parser::MintypmaxExpression::Expression(expr) => unpacked_rank(expr, tree, dims), + sv_parser::MintypmaxExpression::Ternary(_) => 0, + }, + sv_parser::Primary::Hierarchical(hierarchical) => { + let select = &hierarchical.nodes.2; + if select.nodes.0.is_some() { + // A member of a packed structure. + return 0; + } + let unpacked = + identifier_text(RefNode::HierarchicalIdentifier(&hierarchical.nodes.1), tree) + .and_then(|name| dims.get(&name)) + .map_or(0, |shape| shape.unpacked.len()); + let indices = select.nodes.1.nodes.0.len(); + match &select.nodes.2 { + // A slice keeps the dimension it selects from. + Some(_) if indices < unpacked => unpacked - indices, + Some(_) => 0, + None => unpacked.saturating_sub(indices), + } + } + _ => 0, + } +} + +/// The item of a pattern at one position. +#[derive(Clone, Copy)] +enum Item<'p> { + /// A list, repetition or index-keyed item: the value of the element. + Explicit(&'p sv_parser::Expression), + /// A `default:` value, which applies to each element of an unmatched + /// subarray rather than to the subarray as a whole (IEEE 1800-2023 + /// 10.9.1). + Default(&'p sv_parser::Expression), +} + +impl<'p> Item<'p> { + fn expression(self) -> &'p sv_parser::Expression { + match self { + Self::Explicit(expr) | Self::Default(expr) => expr, + } + } +} + /// The items of a pattern for `count` positions, keyed by position: list /// items in order, `n{...}` repetitions, index keys, and a `default`. fn positional_items<'p>( @@ -102,7 +187,7 @@ fn positional_items<'p>( index_offset: impl Fn(i128) -> Option, tree: &SyntaxTree, dims: &PackedDimensions, -) -> Converted> { +) -> Converted>> { let item_count = |items: usize| { unsupported(format!( "assignment pattern with {items} items for {count} elements" @@ -114,7 +199,7 @@ fn positional_items<'p>( if items.len() != count { return Err(item_count(items.len())); } - Ok(items) + Ok(items.into_iter().map(Item::Explicit).collect()) } sv_parser::AssignmentPattern::Repeat(repeat) => { let (times, items) = &repeat.nodes.0.nodes.1; @@ -128,14 +213,16 @@ fn positional_items<'p>( .and_then(|times| usize::try_from(times).ok()) .ok_or_else(|| unsupported("assignment pattern repetition count"))?; let items = items.nodes.1.contents(); - let repeated: Vec<_> = (0..times).flat_map(|_| items.iter().copied()).collect(); + let repeated: Vec<_> = (0..times) + .flat_map(|_| items.iter().copied().map(Item::Explicit)) + .collect(); if repeated.len() != count { return Err(item_count(repeated.len())); } Ok(repeated) } sv_parser::AssignmentPattern::Array(array) => { - let mut slots: Vec> = vec![None; count]; + let mut slots: Vec> = vec![None; count]; let mut default = None; for (key, _, value) in array.nodes.0.nodes.1.contents() { match key { @@ -155,10 +242,12 @@ fn positional_items<'p>( "assignment pattern index {index} outside the target" )) })?; - *slot = Some(value); + *slot = Some(Item::Explicit(value)); } sv_parser::ArrayPatternKey::AssignmentPatternKey(key) => match &**key { - sv_parser::AssignmentPatternKey::Default(_) => default = Some(value), + sv_parser::AssignmentPatternKey::Default(_) => { + default = Some(Item::Default(value)); + } sv_parser::AssignmentPatternKey::SimpleType(_) => { return Err(unsupported("assignment pattern type key")); } @@ -191,7 +280,7 @@ fn positional_items<'p>( } let default = default.ok_or_else(|| unsupported("array assignment pattern without a default"))?; - Ok(vec![default; count]) + Ok(vec![Item::Default(default); count]) } } } @@ -245,7 +334,10 @@ pub(super) fn expr_from_pattern( )?; let mut parts = items .into_iter() - .map(|item| element(item, &element_shape, tree, dims)) + .map(|item| match item { + Item::Explicit(expr) => element(expr, &element_shape, tree, dims), + Item::Default(expr) => default_element(expr, &element_shape, tree, dims), + }) .collect::>>()?; parts.reverse(); return Ok(match <[Expr; 1]>::try_from(parts) { @@ -288,7 +380,7 @@ pub(super) fn expr_from_pattern( )?; let parts = items .into_iter() - .map(|item| element(item, &element_shape, tree, dims)) + .map(|item| element(item.expression(), &element_shape, tree, dims)) .collect::>>()?; return Ok(Expr::Concat(parts)); } @@ -317,7 +409,7 @@ pub(super) fn expr_from_pattern( )?; let parts = items .into_iter() - .map(|item| element(item, &bit, tree, dims)) + .map(|item| element(item.expression(), &bit, tree, dims)) .collect::>>()?; Ok(match <[Expr; 1]>::try_from(parts) { Ok([part]) => part, diff --git a/crates/celox-test-suite/LIMITATIONS.md b/crates/celox-test-suite/LIMITATIONS.md index 69633848f8..48e36cd289 100644 --- a/crates/celox-test-suite/LIMITATIONS.md +++ b/crates/celox-test-suite/LIMITATIONS.md @@ -353,7 +353,7 @@ The SystemVerilog suite's exclusions are listed in [`verification/sv/limitations | `sv_icarus_generate_forward_reference` | execute | icarus | 1 | Icarus 13.0 resolves a name used before its declaration in the same generate block to the enclosing scope. Verilator (which warns VARHIDDEN) and Celox resolve it to the block's own declaration. | | `sv_icarus_generate_case_context` | execute | icarus | 1 | Icarus 13.0 compares a case-generate selector without the common width and signedness of the selector and all labels; IEEE 1800-2023 12.5 makes the comparison unsigned when any operand is unsigned. Verilator agrees with the expected value. | | `sv_icarus_input_port_width` | execute | icarus | 1 | Icarus 13.0 evaluates an input port connection at its self-determined width before padding, losing the carry; IEEE 1800-2023 10.8 and 11.8.2 widen the operands to the port first. Verilator agrees with the expected value. | -| `sv_icarus_syntax` | compile | icarus | 7 | Icarus 13.0 rejects syntax used by these designs: selects of concatenations and replications (IEEE 1800-2023 A.8.4, primary), keyed assignment patterns, and parameter type declarations in generate blocks. Verilator passes these cases. | +| `sv_icarus_syntax` | compile | icarus | 8 | Icarus 13.0 rejects syntax used by these designs: selects of concatenations and replications (IEEE 1800-2023 A.8.4, primary), keyed assignment patterns, and parameter type declarations in generate blocks. Verilator passes these cases. | | `sv_icarus_packed_array_parameters` | compile | icarus | 3 | Icarus 13.0 reports that packed array parameters are not supported, or fails a cast size that depends on a type alias dimension. Verilator passes or reports these four-state cases as unsupported. | | `sv_icarus_unpacked_array_parameters` | compile | icarus | 1 | Icarus 13.0 reports that unpacked array parameters are not supported (IEEE 1800-2023 6.20.2) and rejects the typed assignment pattern of a struct parameter. Verilator passes the case. | | `sv_icarus_internal_error` | compile | icarus | 3 | Icarus 13.0 crashes on these designs: a segmentation fault while compiling, or a vvp assertion (vthread.cc of_DISABLE_FLOW) while running. Verilator passes these cases. | @@ -364,6 +364,7 @@ The SystemVerilog suite's exclusions are listed in [`verification/sv/limitations | `sv_icarus_instance_array_fill_literal` | compile | icarus | 1 | Icarus 13.0 accepts a one-bit '0 connected to a wider port of an instance array, which IEEE 1800-2023 23.3.3.5 makes an error. Verilator rejects it. | | `sv_icarus_inside` | compile | icarus | 1 | Icarus 13.0 does not support inside expressions ("sorry: inside expressions not supported yet"). Verilator passes the case. | | `sv_icarus_packed_element_size` | execute | icarus | 1 | Icarus 13.0 returns 2 for $size of an element of a multidimensional packed struct member (value.m[0] of logic [1:0][3:0]); IEEE 1800-2023 20.7 makes it the element's size, 4. Verilator agrees with the expected value. | +| `sv_icarus_type_size_queries` | compile | icarus | 1 | Icarus 13.0 rejects a data type as the argument of $size ("Type names are not valid expressions here"), which IEEE 1800-2023 20.7 permits. Verilator passes the case. | | `sv_icarus_mixed_state_struct_members` | execute | icarus | 1 | Icarus 13.0 does not convert a 2-state member of a packed struct with 4-state members from four state when reading it, so a compound assignment to the member keeps X; IEEE 1800-2023 7.2.1 converts it. Verilator cannot check four-state values. | | `sv_icarus_unary_plus_unknown` | execute | icarus | 1 | Icarus 13.0 returns the operand of a unary plus unchanged when it has X or Z bits; IEEE 1800-2023 11.4.3 makes the result of an arithmetic operator all X. Verilator cannot check four-state values. | | `sv_icarus_runtime_packed_subselect` | compile | icarus | 1 | Icarus 13.0 rejects a further select after a runtime index into a multidimensional packed array (t[k][j], t[k][2:1]: "Array index expressions must be constant here"). Verilator passes the case. | diff --git a/crates/celox-test-suite/src/sv/cases/review_regressions.vtest b/crates/celox-test-suite/src/sv/cases/review_regressions.vtest index 7a0579ea03..034fc90c62 100644 --- a/crates/celox-test-suite/src/sv/cases/review_regressions.vtest +++ b/crates/celox-test-suite/src/sv/cases/review_regressions.vtest @@ -447,3 +447,151 @@ (modify (set a value)) (expand y (y0 y1 y2 y3 y4 y5) (assert_eq {y} value "{y}")))) + +(case default_pattern_fills_each_element_of_unpacked_subarrays + ; IEEE 1800-2023 10.9.1: `default:` applies recursively to each element of + ; an unmatched subarray, evaluated in the context of the element type, + ; unless the value matches the subarray type. + (source "nested_default_pattern.sv" #" + module Top(input logic [7:0] d, + output logic [7:0] y00, y01, y10, y11, + output logic [31:0] z00, z01, z10, z11); + logic signed [7:0] a [2][2]; + int b [2]; + int c [2][2]; + always_comb begin + a = '{default: 4'sh8}; + if (d[0]) a = '{1: '{default: d[3:0]}, default: 8'sh7f}; + y00 = a[0][0]; + y01 = a[0][1]; + y10 = a[1][0]; + y11 = a[1][1]; + end + always_comb begin + b = '{32'(d), 32'(d) + 1}; + c = '{default: b}; + z00 = c[0][0]; + z01 = c[0][1]; + z10 = c[1][0]; + z11 = c[1][1]; + end + endmodule + "#) + (top Top) + (modify (set d 0x20)) + (expand y (y00 y01 y10 y11) + (assert_eq {y} 0xf8 "{y}")) + (assert_eq z00 0x20) + (assert_eq z01 0x21) + (assert_eq z10 0x20) + (assert_eq z11 0x21) + (modify (set d 0x35)) + (assert_eq y00 0x7f) + (assert_eq y01 0x7f) + (assert_eq y10 0x05) + (assert_eq y11 0x05)) + +(case int_parameters_are_signed_in_comparisons_and_loop_bounds + ; IEEE 1800-2023 6.20.2 and 6.11: a parameter declared `int` has that + ; signed type, so comparing it with a signed operand is a signed + ; comparison (11.8.1). + (source "signed_int_parameter.sv" #" + module Top #(parameter int S = 2, parameter int unsigned U = 2) + (input logic clk, input int lo, + output logic lt_s, lt_u, + output int unsigned cs, cu, fs, fu); + always_comb begin + lt_s = lo < S; + lt_u = lo < U; + cs = 0; + for (int j = lo; j < S; j++) cs += 1; + cu = 0; + for (int j = lo; j < U; j++) cu += 1; + end + always_ff @(posedge clk) begin + int unsigned c; + c = 0; + for (int j = lo; j < S; j++) c += 1; + fs <= c; + c = 0; + for (int j = lo; j < U; j++) c += 1; + fu <= c; + end + endmodule + "#) + (top Top) + (modify (set lo (- 0x100000000 2))) + (tick clk) + (assert_eq lt_s 1) + (assert_eq lt_u 0) + (assert_eq cs 4) + (assert_eq cu 0) + (assert_eq fs 4) + (assert_eq fu 0) + (modify (set lo 1)) + (tick clk) + (assert_eq lt_s 1) + (assert_eq lt_u 1) + (assert_eq cs 1) + (assert_eq cu 1) + (assert_eq fs 1) + (assert_eq fu 1)) + +(case run_time_for_condition_uses_the_relational_operand_context + ; IEEE 1800-2023 11.6.1 and 11.8.2: the operands of a relational operator + ; are sized to the larger of the two, so `j < b + 8'd1` with an `int` j + ; adds at 32 bits and b = 255 runs 256 iterations. + (source "for_condition_context.sv" #" + module Top(input logic clk, input logic [7:0] b, + output logic [31:0] y0, y1, y2, f0); + always_comb begin + y0 = 0; + for (int j = 0; j < b + 8'd1; j++) y0 = y0 + 1; + end + always_comb begin + y1 = 0; + for (int j = 0; j <= b + 8'd1; j++) y1 = y1 + 1; + end + always_comb begin + y2 = 0; + for (int j = b + 8'd1; j > 0; j--) y2 = y2 + 1; + end + always_ff @(posedge clk) begin + logic [31:0] c; + c = 0; + for (int j = 0; j < b + 8'd1; j++) c = c + 1; + f0 <= c; + end + endmodule + "#) + (top Top) + (for value (list 0 1 254 255) + (modify (set b value)) + (tick clk) + (assert_eq y0 (+ value 1) "y0") + (assert_eq y1 (+ value 2) "y1") + (assert_eq y2 (+ value 1) "y2") + (assert_eq f0 (+ value 1) "f0"))) + +(case size_of_a_type_uses_the_dimension_argument + ; IEEE 1800-2023 20.7: $size(t, n) is the number of elements of dimension + ; n of t, numbered from the slowest varying dimension (1); an integer + ; type is a packed array with a single [n-1:0] dimension. + (source "size_type_dimension.sv" #" + module Top(output int p1, p2, t1, t2, t3, d); + typedef logic [3:0][5:0] pair_t; + assign p1 = $size(logic [9:0][19:0], 1); + assign p2 = $size(logic [9:0][19:0], 2); + assign t1 = $size(pair_t, 1); + assign t2 = $size(pair_t, 2); + assign t3 = $size(shortint, 1); + assign d = $size(logic [9:0][19:0]); + endmodule + "#) + (top Top) + (assert_eq p1 10) + (assert_eq p2 20) + (assert_eq t1 4) + (assert_eq t2 6) + (assert_eq t3 16) + (assert_eq d 10)) diff --git a/crates/celox-test-suite/verification/sv/icarus.json b/crates/celox-test-suite/verification/sv/icarus.json index d3804964be..2a7c0a8058 100644 --- a/crates/celox-test-suite/verification/sv/icarus.json +++ b/crates/celox-test-suite/verification/sv/icarus.json @@ -1672,6 +1672,29 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Regression", + "detail": "Icarus 13.0 rejects syntax used by these designs: selects of concatenations and replications (IEEE 1800-2023 A.8.4, primary), keyed assignment patterns, and parameter type declarations in generate blocks. Verilator passes these cases.", + "expectation": "Simulation", + "known_issue": { + "category": "simulator_compile_limitation", + "evidence": [ + "verification/sv/limitations/icarus.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "id": "sv_icarus_syntax", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "phase": "compile", + "reason": "Icarus 13.0 rejects syntax used by these designs: selects of concatenations and replications (IEEE 1800-2023 A.8.4, primary), keyed assignment patterns, and parameter type declarations in generate blocks. Verilator passes these cases.", + "upstream": [] + }, + "name": "review_regressions::default_pattern_fills_each_element_of_unpacked_subarrays", + "phase": "skip", + "status": "ignored", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Regression", "detail": "", @@ -1683,6 +1706,17 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Regression", + "detail": "", + "expectation": "Simulation", + "name": "review_regressions::int_parameters_are_signed_in_comparisons_and_loop_bounds", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Regression", "detail": "", @@ -1885,6 +1919,40 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Regression", + "detail": "", + "expectation": "Simulation", + "name": "review_regressions::run_time_for_condition_uses_the_relational_operand_context", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Regression", + "detail": "Icarus 13.0 rejects a data type as the argument of $size (\"Type names are not valid expressions here\"), which IEEE 1800-2023 20.7 permits. Verilator passes the case.", + "expectation": "Simulation", + "known_issue": { + "category": "simulator_compile_limitation", + "evidence": [ + "verification/sv/limitations/icarus.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "id": "sv_icarus_type_size_queries", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "phase": "compile", + "reason": "Icarus 13.0 rejects a data type as the argument of $size (\"Type names are not valid expressions here\"), which IEEE 1800-2023 20.7 permits. Verilator passes the case.", + "upstream": [] + }, + "name": "review_regressions::size_of_a_type_uses_the_dimension_argument", + "phase": "skip", + "status": "ignored", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Combinational", "detail": "", @@ -2393,9 +2461,9 @@ "counts": { "compile_error": 0, "emission_error": 0, - "ignored": 35, + "ignored": 37, "mismatch": 0, - "passed": 141, + "passed": 143, "rejected": 3, "runtime_error": 0, "unexpected_accept": 0, diff --git a/crates/celox-test-suite/verification/sv/limitations.json b/crates/celox-test-suite/verification/sv/limitations.json index a4978ad79a..25ebd55765 100644 --- a/crates/celox-test-suite/verification/sv/limitations.json +++ b/crates/celox-test-suite/verification/sv/limitations.json @@ -89,7 +89,8 @@ "indexed_select::indexed_runtime_replications_preserve_attached_selections", "indexed_select::indexed_select_multidimensional_packed_reads", "synthesizable::struct_assignment_patterns_follow_the_target_layout", - "review_regressions::preserves_four_state_shift_and_bit_select_constants" + "review_regressions::preserves_four_state_shift_and_bit_select_constants", + "review_regressions::default_pattern_fills_each_element_of_unpacked_subarrays" ] } }, @@ -270,6 +271,23 @@ ] } }, + { + "id": "sv_icarus_type_size_queries", + "category": "simulator_compile_limitation", + "phase": "compile", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "reason": "Icarus 13.0 rejects a data type as the argument of $size (\"Type names are not valid expressions here\"), which IEEE 1800-2023 20.7 permits. Verilator passes the case.", + "upstream": [], + "evidence": [ + "verification/sv/limitations/icarus.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "cases": { + "icarus": [ + "review_regressions::size_of_a_type_uses_the_dimension_argument" + ] + } + }, { "id": "sv_icarus_mixed_state_struct_members", "category": "simulator_disagreement", diff --git a/crates/celox-test-suite/verification/sv/limitations/icarus.json b/crates/celox-test-suite/verification/sv/limitations/icarus.json index 18393b7962..85dbad50d1 100644 --- a/crates/celox-test-suite/verification/sv/limitations/icarus.json +++ b/crates/celox-test-suite/verification/sv/limitations/icarus.json @@ -9,7 +9,7 @@ "unexpected_accept": 1, "mismatch": 7, "emission_error": 0, - "compile_error": 25, + "compile_error": 27, "runtime_error": 2, "unsupported": 0, "ignored": 0 @@ -294,6 +294,22 @@ "name": "indexed_select::out_of_range_inner_indices_stay_in_their_element", "phase": "execute", "status": "mismatch" + }, + { + "category": "Regression", + "detail": "Icarus build exit status: 6; see /build.log\n/source_0.sv:9: syntax error\n/source_0.sv:9: error: Malformed statement\n/source_0.sv:10: syntax error\n/source_0.sv:10: error: Malformed statement\n/source_0.sv:18: syntax error\n/source_0.sv:18: error: Malformed statement\n", + "expectation": "Simulation", + "name": "review_regressions::default_pattern_fills_each_element_of_unpacked_subarrays", + "phase": "compile", + "status": "compile_error" + }, + { + "category": "Regression", + "detail": "Icarus build exit status: 18; see /build.log\n/source_0.sv:4: error: Type names are not valid expressions here.\n/source_0.sv:4: internal error: I do not know how to elaborate this expression. \n/source_0.sv:4: : Expression is: \n/source_0.sv:4: : Expression type: 10PETypename\n/source_0.sv:4: error: Unable to elaborate r-value: $size(, 'sd1)\n/source_0.sv:5: error: Type names are not valid expressions here.\n/source_0.sv:5: internal error: I do not know how to elaborate this expression. \n/source_0.sv:5: : Expression is: \n/source_0.sv:5: : Expression type: 10PETypename\n/source_0.sv:5: error: Unable to elaborate r-value: $size(, 'sd2)\n/source_0.sv:6: error: Type names are not valid expressions here.\n/source_0.sv:6: internal error: I do not know how to elaborate this expression. \n/source_0.sv:6: : Expression is: \n/source_0.sv:6: : Expression type: 10PETypename\n/source_0.sv:6: error: Unable to elaborate r-value: $size(, 'sd1)\n/source_0.sv:7: error: Type names are not valid expressions here.\n/source_0.sv:7: internal error: I do not know how to elaborate this expression. \n/source_0.sv:7: : Expression is: \n/source_0.sv:7: : Expression type: 10PETypename\n/source_0.sv:7: error: Unable to elaborate r-value: $size(, 'sd2)\n/source_0.sv:8: error: Type names are not valid expressions here.\n/source_0.sv:8: internal error: I do not know how to elaborate this expression. \n/source_0.sv:8: : Expression is: \n/source_0.sv:8: : Expression type: 10PETypename\n/source_0.sv:8: error:", + "expectation": "Simulation", + "name": "review_regressions::size_of_a_type_uses_the_dimension_argument", + "phase": "compile", + "status": "compile_error" } ] } diff --git a/crates/celox-test-suite/verification/sv/verilator.json b/crates/celox-test-suite/verification/sv/verilator.json index e0b0cf0261..69c3f84ea3 100644 --- a/crates/celox-test-suite/verification/sv/verilator.json +++ b/crates/celox-test-suite/verification/sv/verilator.json @@ -1468,6 +1468,17 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Regression", + "detail": "", + "expectation": "Simulation", + "name": "review_regressions::default_pattern_fills_each_element_of_unpacked_subarrays", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Regression", "detail": "four-state expectations", @@ -1479,6 +1490,17 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Regression", + "detail": "", + "expectation": "Simulation", + "name": "review_regressions::int_parameters_are_signed_in_comparisons_and_loop_bounds", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Regression", "detail": "", @@ -1633,6 +1655,28 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Regression", + "detail": "", + "expectation": "Simulation", + "name": "review_regressions::run_time_for_condition_uses_the_relational_operand_context", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Regression", + "detail": "", + "expectation": "Simulation", + "name": "review_regressions::size_of_a_type_uses_the_dimension_argument", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Combinational", "detail": "", @@ -1801,7 +1845,7 @@ }, { "category": "Combinational", - "detail": "Verilator build exit status: 1; see /build.log\n%Error: /source_0.sv:6:41: Input port connection 'b' as part of a module instance array requires 8 or 16 bits, but connection's CONST ''0' generates 1 bits. (IEEE 1800-2023 23.3.3)\n : ... note: In instance 'Top'\n 6 | Pass u[1:0](.a(a), .b('0), .y(y));\n | ^\n ... See the manual at https://verilator.org/verilator_doc.html?v=5.052 for more assistance.\n%Error: Exiting due to 1 error(s)\n", + "detail": "Verilator build exit status: 1; see /build.log\n%Error: /source_0.sv:6:41: Input port connection 'b' as part of a module instance array requires 8 or 16 bits, but connection's CONST ''0' generates 1 bits. (IEEE 1800-2023 23.3.3)\n : ... note: In instance 'Top'\n 6 | Pass u[1:0](.a(a), .b('0), .y(y));\n | ^\n ... See the manual at https://verilator.org/verilator_doc.html?v=5.052 for more assistance.\n%Error: Exiting due to 1 error(s)\n", "expectation": "CompilationError", "name": "synthesizable::instance_arrays_reject_a_fill_literal_tie_off", "phase": "compile", @@ -1812,7 +1856,7 @@ }, { "category": "Combinational", - "detail": "Verilator build exit status: 1; see /build.log\n%Error: /source_0.sv:6:41: Input port connection 'b' as part of a module instance array requires 8 or 16 bits, but connection's CONST '?32?sh0' generates 32 bits. (IEEE 1800-2023 23.3.3)\n : ... note: In instance 'Top'\n 6 | Pass u[1:0](.a(a), .b(0), .y(y));\n | ^\n ... See the manual at https://verilator.org/verilator_doc.html?v=5.052 for more assistance.\n%Error: Exiting due to 1 error(s)\n", + "detail": "Verilator build exit status: 1; see /build.log\n%Error: /source_0.sv:6:41: Input port connection 'b' as part of a module instance array requires 8 or 16 bits, but connection's CONST '?32?sh0' generates 32 bits. (IEEE 1800-2023 23.3.3)\n : ... note: In instance 'Top'\n 6 | Pass u[1:0](.a(a), .b(0), .y(y));\n | ^\n ... See the manual at https://verilator.org/verilator_doc.html?v=5.052 for more assistance.\n%Error: Exiting due to 1 error(s)\n", "expectation": "CompilationError", "name": "synthesizable::instance_arrays_reject_an_unsized_constant_tie_off", "phase": "compile", @@ -2047,7 +2091,7 @@ "emission_error": 0, "ignored": 6, "mismatch": 0, - "passed": 134, + "passed": 138, "rejected": 3, "runtime_error": 0, "unexpected_accept": 0, diff --git a/crates/celox/tests/comb_observer.rs b/crates/celox/tests/comb_observer.rs index be90ee3ce3..66753b3b2b 100644 --- a/crates/celox/tests/comb_observer.rs +++ b/crates/celox/tests/comb_observer.rs @@ -2582,7 +2582,7 @@ fn test_comb_function_array_literal_converts_scalar_items_per_element(sim) { } fn test_comb_function_nested_array_scalar_default_converts_each_element(sim) { - @ignore_on(veryl, sv); + @ignore_on(veryl); @case "comb_observer::test_comb_function_nested_array_scalar_default_converts_each_element"; } diff --git a/crates/celox/tests/flip_flop.rs b/crates/celox/tests/flip_flop.rs index 68a6adb60b..5bdb8f2362 100644 --- a/crates/celox/tests/flip_flop.rs +++ b/crates/celox/tests/flip_flop.rs @@ -4276,7 +4276,7 @@ fn test_ff_function_call_array_literal_default_fill_matches_formal_shape(sim) { } fn test_ff_function_call_multidim_array_literal_default_fill_matches_formal_shape(sim) { - @ignore_on(veryl, sv); + @ignore_on(veryl); @case "flip_flop::test_ff_function_call_multidim_array_literal_default_fill_matches_formal_shape"; } diff --git a/crates/celox/tests/frontends/systemverilog/review_regressions.rs b/crates/celox/tests/frontends/systemverilog/review_regressions.rs index fd57896b09..1c077c4e2e 100644 --- a/crates/celox/tests/frontends/systemverilog/review_regressions.rs +++ b/crates/celox/tests/frontends/systemverilog/review_regressions.rs @@ -7467,6 +7467,22 @@ sv_backends! { fn lowers_parameter_casts_in_loop_generate(sim) { @case "review_regressions::lowers_parameter_casts_in_loop_generate"; } + + fn default_pattern_fills_each_element_of_unpacked_subarrays(sim) { + @case "review_regressions::default_pattern_fills_each_element_of_unpacked_subarrays"; + } + + fn int_parameters_are_signed_in_comparisons_and_loop_bounds(sim) { + @case "review_regressions::int_parameters_are_signed_in_comparisons_and_loop_bounds"; + } + + fn run_time_for_condition_uses_the_relational_operand_context(sim) { + @case "review_regressions::run_time_for_condition_uses_the_relational_operand_context"; + } + + fn size_of_a_type_uses_the_dimension_argument(sim) { + @case "review_regressions::size_of_a_type_uses_the_dimension_argument"; + } } #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] diff --git a/crates/celox/tests/system_function.rs b/crates/celox/tests/system_function.rs index c9f6c91fb4..c58adf12dc 100644 --- a/crates/celox/tests/system_function.rs +++ b/crates/celox/tests/system_function.rs @@ -82,7 +82,6 @@ all_backends! { } fn test_direct_ff_size_packed_multidimensional_type_system_function(sim) { - @ignore_on(sv); @case "system_function::test_direct_ff_size_packed_multidimensional_type_system_function"; } diff --git a/crates/celox/tests/veryl_context_regressions.rs b/crates/celox/tests/veryl_context_regressions.rs index 1f7d70adb0..ceca335440 100644 --- a/crates/celox/tests/veryl_context_regressions.rs +++ b/crates/celox/tests/veryl_context_regressions.rs @@ -41,7 +41,6 @@ all_backends! { @case "veryl_context_regressions::folded_const_select_keeps_its_sign"; } fn runtime_for_bound_keeps_its_type(sim) { - @ignore_on(sv); @case "veryl_context_regressions::runtime_for_bound_keeps_its_type"; } fn case_compares_each_label_as_an_if_does(sim) { @@ -51,7 +50,6 @@ all_backends! { @case "veryl_context_regressions::runtime_case_target_uses_comparison_context"; } fn runtime_for_bound_arithmetic_uses_int_context(sim) { - @ignore_on(sv); @case "veryl_context_regressions::runtime_for_bound_arithmetic_uses_int_context"; } fn dynamic_param_array_read_uses_its_elements(sim) { @@ -69,7 +67,6 @@ all_backends! { all_backends! { fn ff_loop_bound_keeps_its_type(sim) { - @ignore_on(sv); @setup { let code = r#" module Top #( param UNSIGNED_END: u32 = 2, From 308bf613ac8923372754a856b546b71696ccf512 Mon Sep 17 00:00:00 2001 From: tignear Date: Wed, 7 Oct 2026 08:47:52 +0900 Subject: [PATCH 2/5] fix(sv): widen branchless mux arms and reject incompatible unpacked arrays The branchless SLT mux lowering emitted a SIR mux whose arms had different widths, which failed SIR verification for a run-time read of an unpacked array of 1-bit elements. Unpacked arrays whose element types are not equivalent were flattened and their bits reinterpreted. Assignment-like contexts (IEEE 1800-2023 10.8, 7.6) now reject them: subroutine arguments, assignment pattern items, procedural and continuous assignments, and port connections. --- crates/celox-frontend-sv/src/lowering.rs | 57 +++++++ crates/celox-slt/src/lower/mux.rs | 4 + crates/celox-slt/src/lower/tests/mux.rs | 32 ++++ crates/celox-sv-analyzer/src/ast.rs | 4 + .../src/ast/array_compatibility.rs | 154 ++++++++++++++++++ .../celox-sv-analyzer/src/ast/comb_process.rs | 33 ++++ .../celox-sv-analyzer/src/ast/expressions.rs | 11 ++ crates/celox-sv-analyzer/src/ast/patterns.rs | 17 +- .../celox-sv-analyzer/src/ast/procedural.rs | 33 +++- crates/celox-sv-analyzer/src/lib.rs | 11 ++ crates/celox-sv-analyzer/src/tests.rs | 64 ++++++++ crates/celox-sv-analyzer/src/typecheck.rs | 39 +++++ crates/celox-test-suite/LIMITATIONS.md | 3 + crates/celox-test-suite/src/icarus.rs | 4 + .../src/sv/cases/indexed_select.vtest | 20 +++ .../celox-test-suite/src/sv/cases/types.vtest | 143 ++++++++++++++++ crates/celox-test-suite/src/verilator.rs | 18 +- .../tests/source_rejection.rs | 65 ++++++++ .../verification/sv/icarus.json | 130 ++++++++++++++- .../verification/sv/limitations.json | 53 ++++++ .../verification/sv/limitations/icarus.json | 26 ++- .../sv/limitations/verilator.json | 18 +- .../verification/sv/verilator.json | 118 +++++++++++++- crates/celox/tests/comb_observer.rs | 12 ++ crates/celox/tests/flip_flop.rs | 12 ++ .../frontends/systemverilog/indexed_select.rs | 4 + .../systemverilog/review_regressions.rs | 126 +++++++++++++- .../tests/frontends/systemverilog/types.rs | 28 ++++ crates/celox/tests/nba_dynamic_array.rs | 2 - 29 files changed, 1220 insertions(+), 21 deletions(-) create mode 100644 crates/celox-sv-analyzer/src/ast/array_compatibility.rs diff --git a/crates/celox-frontend-sv/src/lowering.rs b/crates/celox-frontend-sv/src/lowering.rs index 52035d64e0..bebe04c94f 100644 --- a/crates/celox-frontend-sv/src/lowering.rs +++ b/crates/celox-frontend-sv/src/lowering.rs @@ -1917,6 +1917,62 @@ type SvGlue = ( SLTNodeArena, ); +/// The type of `variable` when it is an unpacked array. +fn unpacked_array_type(variable: &SvVariable) -> Option { + let element_width = unpacked_element_width(variable)?; + Some(sv::typecheck::UnpackedArrayType { + dims: variable.array_dims.clone(), + element_width, + signed: variable.signed, + four_state: variable.is_4state, + }) +} + +/// Rejects an unpacked array connected to an unpacked array input or output +/// port of an incompatible type: such a connection is an assignment-like +/// context (IEEE 1800-2023 10.8, 23.3.3), which requires equivalent element +/// types and equal element counts (7.6). +fn check_unpacked_port_connection( + connection: &LoweredSvPortConnection, + child: &LoweredSvModule, + parent_variables: &HashMap, + parent_signal_names: &HashMap, +) -> Result<(), ParserError> { + let Some(sv::ir::Expr::Ident(actual)) = connection.actual_expr.as_ref() else { + return Ok(()); + }; + let Some(port) = child + .signal_names + .get(&connection.formal) + .and_then(|id| child.variables.get(id)) + .filter(|port| matches!(port.kind, VariableKind::Input | VariableKind::Output)) + else { + return Ok(()); + }; + let (Some(target), Some(actual_type)) = ( + unpacked_array_type(port), + parent_signal_names + .get(actual) + .and_then(|id| parent_variables.get(id)) + .and_then(unpacked_array_type), + ) else { + return Ok(()); + }; + if actual_type.is_assignment_compatible_with(&target) { + return Ok(()); + } + let error = sv::AnalyzerError::IncompatibleUnpackedArray { + context: format!("connection of port `{}`", connection.formal), + actual: actual_type, + target, + }; + Err(ParserError::illegal_context( + "systemverilog port connection", + error.to_string(), + None, + )) +} + fn build_instance_glue( parent_variables: &HashMap, parent_signal_names: &HashMap, @@ -1946,6 +2002,7 @@ fn build_instance_glue( None, )); } + check_unpacked_port_connection(connection, child, parent_variables, parent_signal_names)?; } for child_port_id in &child.port_order { diff --git a/crates/celox-slt/src/lower/mux.rs b/crates/celox-slt/src/lower/mux.rs index aa44a8d46b..f766e89246 100644 --- a/crates/celox-slt/src/lower/mux.rs +++ b/crates/celox-slt/src/lower/mux.rs @@ -368,8 +368,12 @@ impl SLTToSIRLowerer { ); } + // An SLT mux is as wide as its wider arm, while a SIR mux needs arms of + // its own width; widen the narrower arm as the folded and CFG paths do. let then_val = self.lower_inner(builder, then_expr, arena, cache, env, allow_cache); + let then_val = self.cast_reg_width(builder, then_val, res_width); let else_val = self.lower_inner(builder, else_expr, arena, cache, env, allow_cache); + let else_val = self.cast_reg_width(builder, else_val, res_width); // Use Mux instruction: preserves Z in 4-state, branchless select in 2-state. // Backends handle value and mask selection independently. diff --git a/crates/celox-slt/src/lower/tests/mux.rs b/crates/celox-slt/src/lower/tests/mux.rs index 0a125e97f8..74071fe7a4 100644 --- a/crates/celox-slt/src/lower/tests/mux.rs +++ b/crates/celox-slt/src/lower/tests/mux.rs @@ -24,6 +24,38 @@ fn cheap_mux_stays_branchless() { ); } +#[test] +fn branchless_mux_widens_the_narrower_arm() { + // An SLT mux is as wide as its wider arm; the SIR mux needs both arms at + // that width. + let mut arena = SLTNodeArena::new(); + let cond = input(&mut arena, 0, 1); + let then_expr = input(&mut arena, 1, 32); + let else_expr = constant(&mut arena, 0, 64); + let mux = arena + .alloc(SLTNode::Mux { + cond, + then_expr, + else_expr, + }) + .unwrap(); + let mut builder = SIRBuilder::new(); + let result = SLTToSIRLowerer::new(false).lower( + &mut builder, + mux, + &arena, + &mut crate::HashMap::default(), + ); + assert_eq!(builder.register(&result).width(), 64); + let eu = finish_lowering(builder); + + assert_eq!(branch_count(&eu), 0); + assert_eq!( + instruction_count(&eu, |inst| matches!(inst, SIRInstruction::Mux(..))), + 1 + ); +} + #[test] fn expected_cost_uses_static_equality_probability() { let even = StaticBranchProbability::EVEN; diff --git a/crates/celox-sv-analyzer/src/ast.rs b/crates/celox-sv-analyzer/src/ast.rs index 7b6cc99a5f..5544581066 100644 --- a/crates/celox-sv-analyzer/src/ast.rs +++ b/crates/celox-sv-analyzer/src/ast.rs @@ -19,6 +19,7 @@ use sv_parser::{Locate, RefNode, SyntaxTree, unwrap_node}; use crate::{AnalyzerError, system_functions, typecheck}; +mod array_compatibility; mod array_parameters; mod assignment_analysis; mod case; @@ -47,6 +48,9 @@ mod statements; mod types; mod validation; +use array_compatibility::{ + check_unpacked_array_assignment, net_lvalue_unpacked_shape, variable_lvalue_unpacked_shape, +}; use assignment_analysis::two_state_conditions_are_complements; use case::expr_is_two_state; use casts::{ diff --git a/crates/celox-sv-analyzer/src/ast/array_compatibility.rs b/crates/celox-sv-analyzer/src/ast/array_compatibility.rs new file mode 100644 index 0000000000..25fa844999 --- /dev/null +++ b/crates/celox-sv-analyzer/src/ast/array_compatibility.rs @@ -0,0 +1,154 @@ +//! Assignment compatibility of unpacked arrays (IEEE 1800-2023 7.6). +//! +//! An unpacked array value is copied element by element, so it can only be +//! assigned to an array whose element type is equivalent (6.22.2) and whose +//! dimensions have the same element counts. Every assignment-like context +//! (10.8) follows this rule, including subroutine arguments of any direction +//! and the items of an assignment pattern. + +use super::*; + +use typecheck::UnpackedArrayType; + +/// The type of an unpacked array of `shape`, or `None` when `shape` is not an +/// unpacked array or its type cannot be resolved. +fn unpacked_array_type( + shape: &VariableDimensions, + dims: &PackedDimensions, +) -> Option { + if shape.unpacked.is_empty() { + return None; + } + let count = |width: &ConstExpr| { + eval_ast_const_expr(width, &dims.const_env).and_then(|width| usize::try_from(width).ok()) + }; + Some(UnpackedArrayType { + dims: shape + .unpacked + .iter() + .map(|dimension| count(&dimension.width)) + .collect::>()?, + element_width: shape.packed.iter().try_fold(1usize, |width, dimension| { + width.checked_mul(count(&dimension.width)?) + })?, + signed: shape.signed, + four_state: !shape.is_2state, + }) +} + +/// The shape of a variable `name` after `indices` index selects, when that +/// is still an unpacked array. +fn selected_unpacked_shape( + name: &str, + indices: usize, + dims: &PackedDimensions, +) -> Option { + let shape = dims.get(name)?; + let unpacked = shape + .unpacked + .get(indices..) + .filter(|rest| !rest.is_empty())?; + Some(VariableDimensions { + unpacked: unpacked.to_vec(), + ..shape.clone() + }) +} + +/// The shape of the unpacked array an expression names: a variable, or one +/// of its subarrays selected by indices. Other expressions (slices, member +/// selects, calls, concatenations) are not resolved. +fn expression_unpacked_shape( + expr: &sv_parser::Expression, + tree: &SyntaxTree, + dims: &PackedDimensions, +) -> Option { + let sv_parser::Expression::Primary(primary) = expr else { + return None; + }; + match &**primary { + sv_parser::Primary::MintypmaxExpression(grouped) => { + let sv_parser::MintypmaxExpression::Expression(inner) = &grouped.nodes.0.nodes.1 else { + return None; + }; + expression_unpacked_shape(inner, tree, dims) + } + sv_parser::Primary::Hierarchical(hierarchical) => { + let select = &hierarchical.nodes.2; + if select.nodes.0.is_some() || select.nodes.2.is_some() { + return None; + } + let name = + identifier_text(RefNode::HierarchicalIdentifier(&hierarchical.nodes.1), tree)?; + selected_unpacked_shape(&name, select.nodes.1.nodes.0.len(), dims) + } + _ => None, + } +} + +/// The shape of an assignment target when it is an unpacked array. +pub(super) fn variable_lvalue_unpacked_shape( + lvalue: &sv_parser::VariableLvalue, + tree: &SyntaxTree, + dims: &PackedDimensions, +) -> Option { + let sv_parser::VariableLvalue::Identifier(identifier) = lvalue else { + return None; + }; + let select = &identifier.nodes.2; + if identifier.nodes.0.is_some() || select.nodes.0.is_some() || select.nodes.2.is_some() { + return None; + } + let name = identifier_text( + RefNode::HierarchicalVariableIdentifier(&identifier.nodes.1), + tree, + )?; + selected_unpacked_shape(&name, select.nodes.1.nodes.0.len(), dims) +} + +/// The shape of a net assignment target when it is an unpacked array. +pub(super) fn net_lvalue_unpacked_shape( + lvalue: &sv_parser::NetLvalue, + tree: &SyntaxTree, + dims: &PackedDimensions, +) -> Option { + let sv_parser::NetLvalue::Identifier(identifier) = lvalue else { + return None; + }; + let select = &identifier.nodes.1; + if select.nodes.0.is_some() || select.nodes.2.is_some() { + return None; + } + let name = identifier_text( + RefNode::PsOrHierarchicalNetIdentifier(&identifier.nodes.0), + tree, + )?; + selected_unpacked_shape(&name, select.nodes.1.nodes.0.len(), dims) +} + +/// Rejects `value` in an assignment-like context with an unpacked array of +/// `target` shape when `value` is an unpacked array of an incompatible type. +/// `context` names the assignment-like context for the diagnostic. +pub(super) fn check_unpacked_array_assignment( + value: &sv_parser::Expression, + target: &VariableDimensions, + context: impl FnOnce() -> String, + tree: &SyntaxTree, + dims: &PackedDimensions, +) -> Converted<()> { + let Some(target) = unpacked_array_type(target, dims) else { + return Ok(()); + }; + let Some(actual) = expression_unpacked_shape(value, tree, dims) + .and_then(|shape| unpacked_array_type(&shape, dims)) + else { + return Ok(()); + }; + if actual.is_assignment_compatible_with(&target) { + return Ok(()); + } + Err(AnalyzerError::IncompatibleUnpackedArray { + context: context(), + actual, + target, + }) +} diff --git a/crates/celox-sv-analyzer/src/ast/comb_process.rs b/crates/celox-sv-analyzer/src/ast/comb_process.rs index 6bdc7a3ab3..1b96903bf8 100644 --- a/crates/celox-sv-analyzer/src/ast/comb_process.rs +++ b/crates/celox-sv-analyzer/src/ast/comb_process.rs @@ -181,6 +181,15 @@ fn net_declaration_assignments( }; let name = identifier_text(RefNode::NetIdentifier(&assignment.nodes.0), syntax_tree) .ok_or_else(|| AnalyzerError::Unsupported("net declaration assignment".to_string()))?; + if let Some(target) = packed_dimensions.get(&name) { + check_unpacked_array_assignment( + expression, + target, + || "net declaration assignment".to_string(), + syntax_tree, + packed_dimensions, + )?; + } let rhs = expr_from_expression_with_types(expression, syntax_tree, packed_dimensions)?; assignments.push(Assignment::new(LValue::Ident(name), rhs)); } @@ -201,6 +210,17 @@ fn assignments_from_continuous_assign( .contents() .into_iter() .map(|assignment| { + if let Some(target) = + net_lvalue_unpacked_shape(&assignment.nodes.0, syntax_tree, packed_dimensions) + { + check_unpacked_array_assignment( + &assignment.nodes.2, + &target, + || "continuous assignment".to_string(), + syntax_tree, + packed_dimensions, + )?; + } let lhs = net_lvalue_from_node(&assignment.nodes.0, syntax_tree, packed_dimensions)?; let rhs = expr_from_expression_for_lvalue( @@ -231,6 +251,19 @@ fn assignments_from_continuous_assign( .contents() .into_iter() .map(|assignment| { + if let Some(target) = variable_lvalue_unpacked_shape( + &assignment.nodes.0, + syntax_tree, + packed_dimensions, + ) { + check_unpacked_array_assignment( + &assignment.nodes.2, + &target, + || "continuous assignment".to_string(), + syntax_tree, + packed_dimensions, + )?; + } let lhs = variable_lvalue_from_node(&assignment.nodes.0, syntax_tree, packed_dimensions)?; let rhs = expr_from_expression_for_lvalue( diff --git a/crates/celox-sv-analyzer/src/ast/expressions.rs b/crates/celox-sv-analyzer/src/ast/expressions.rs index 5cedc234ad..075bc53e7e 100644 --- a/crates/celox-sv-analyzer/src/ast/expressions.rs +++ b/crates/celox-sv-analyzer/src/ast/expressions.rs @@ -675,6 +675,17 @@ fn expr_from_tf_call( .subroutine_param_shapes .get(&name) .and_then(|shapes| shapes.get(position)); + if let Some(shape) = shape { + // Passing an argument is an assignment-like context + // (IEEE 1800-2023 10.8). + check_unpacked_array_assignment( + expr, + shape, + || format!("argument {} of `{name}`", position + 1), + syntax_tree, + packed_dimensions, + )?; + } let lowered_arg = match (patterns::pattern_expression(expr), shape) { (Some(pattern), Some(shape)) if pattern.nodes.0.is_none() => { patterns::expr_from_pattern( diff --git a/crates/celox-sv-analyzer/src/ast/patterns.rs b/crates/celox-sv-analyzer/src/ast/patterns.rs index 8ac352abb1..fb2b2c5b8f 100644 --- a/crates/celox-sv-analyzer/src/ast/patterns.rs +++ b/crates/celox-sv-analyzer/src/ast/patterns.rs @@ -180,7 +180,8 @@ impl<'p> Item<'p> { } /// The items of a pattern for `count` positions, keyed by position: list -/// items in order, `n{...}` repetitions, index keys, and a `default`. +/// items in order, `n{...}` repetitions, index keys, and a `default`. Each +/// item is paired with whether it comes from `default`. fn positional_items<'p>( pattern: &'p sv_parser::AssignmentPattern, count: usize, @@ -335,7 +336,19 @@ pub(super) fn expr_from_pattern( let mut parts = items .into_iter() .map(|item| match item { - Item::Explicit(expr) => element(expr, &element_shape, tree, dims), + Item::Explicit(expr) => { + // A nondefault item corresponds to its element in an + // assignment-like context (IEEE 1800-2023 10.8), and the + // element may itself be an unpacked array. + check_unpacked_array_assignment( + expr, + &element_shape, + || "assignment pattern item".to_string(), + tree, + dims, + )?; + element(expr, &element_shape, tree, dims) + } Item::Default(expr) => default_element(expr, &element_shape, tree, dims), }) .collect::>>()?; diff --git a/crates/celox-sv-analyzer/src/ast/procedural.rs b/crates/celox-sv-analyzer/src/ast/procedural.rs index c3b79a7376..7988c9df27 100644 --- a/crates/celox-sv-analyzer/src/ast/procedural.rs +++ b/crates/celox-sv-analyzer/src/ast/procedural.rs @@ -346,6 +346,17 @@ impl<'s, 't, 'a> BodyBuilder<'s, 't, 'a> { nonblocking, }); } + if op == "=" + && let Some(target) = variable_lvalue_unpacked_shape(lvalue, self.tree, &self.dims) + { + check_unpacked_array_assignment( + rhs, + &target, + || "assignment".to_string(), + self.tree, + &self.dims, + )?; + } let (written, lhs) = self.lvalue(lvalue)?; let mut rhs = if op == "=" { expr_from_expression_for_lvalue(rhs, &written, self.tree, &self.dims)? @@ -817,13 +828,25 @@ impl<'s, 't, 'a> BodyBuilder<'s, 't, 'a> { position: usize, arg: &sv_parser::Expression, ) -> Result { + let shape = self + .dims + .subroutine_param_shapes + .get(name) + .and_then(|shapes| shapes.get(position)); + if let Some(shape) = shape { + // Passing an argument in any direction is an assignment-like + // context (IEEE 1800-2023 10.8). + check_unpacked_array_assignment( + arg, + shape, + || format!("argument {} of `{name}`", position + 1), + self.tree, + &self.dims, + )?; + } if let Some(pattern) = patterns::pattern_expression(arg).filter(|pattern| pattern.nodes.0.is_none()) - && let Some(shape) = self - .dims - .subroutine_param_shapes - .get(name) - .and_then(|shapes| shapes.get(position)) + && let Some(shape) = shape { let mut lowered = patterns::expr_from_pattern(&pattern.nodes.1, shape, self.tree, &self.dims)?; diff --git a/crates/celox-sv-analyzer/src/lib.rs b/crates/celox-sv-analyzer/src/lib.rs index 277ff32530..280bb22967 100644 --- a/crates/celox-sv-analyzer/src/lib.rs +++ b/crates/celox-sv-analyzer/src/lib.rs @@ -60,6 +60,17 @@ pub enum AnalyzerError { /// such as a task used as a value or a wrong number of arguments. #[error("invalid call of `{name}`: {detail}")] InvalidSystemTfCall { name: String, detail: String }, + /// An unpacked array assigned, passed as a subroutine argument, connected + /// to a port, or used as an assignment pattern item where its type is not + /// assignment compatible with the target array (IEEE 1800-2023 7.6, 10.8). + #[error( + "{context}: an unpacked array of type `{actual}` is not assignment compatible with `{target}`" + )] + IncompatibleUnpackedArray { + context: String, + actual: typecheck::UnpackedArrayType, + target: typecheck::UnpackedArrayType, + }, } impl miette::Diagnostic for AnalyzerError {} diff --git a/crates/celox-sv-analyzer/src/tests.rs b/crates/celox-sv-analyzer/src/tests.rs index 81ae45f91e..e992f293e0 100644 --- a/crates/celox-sv-analyzer/src/tests.rs +++ b/crates/celox-sv-analyzer/src/tests.rs @@ -1925,6 +1925,70 @@ fn rejects_nonpositive_implicit_unpacked_array_dimensions() { } } +#[test] +fn rejects_unpacked_arrays_of_nonequivalent_element_types() { + // IEEE 1800-2023 6.22.2 and 7.6: an unpacked array assignment needs + // equivalent element types (same width, state count and signedness) and + // equal element counts. + let source = r#" + module Top(output logic [7:0] q); + function automatic logic [7:0] first(input logic [7:0] x [2][2]); + return x[0][0]; + endfunction + logic signed [7:0] row [2]; + always_comb begin + row[0] = 8'sd1; + row[1] = 8'sd2; + q = first('{row, '{8'd3, 8'd4}}); + end + endmodule + "#; + let error = analyze_source(source, Path::new("nonequivalent_unpacked.sv")) + .expect_err("a signed row is not equivalent to an unsigned one"); + let row = |signed| typecheck::UnpackedArrayType { + dims: vec![2], + element_width: 8, + signed, + four_state: true, + }; + assert_eq!( + error, + AnalyzerError::IncompatibleUnpackedArray { + context: "assignment pattern item".to_string(), + actual: row(true), + target: row(false), + } + ); +} + +#[test] +fn accepts_unpacked_arrays_of_equivalent_element_types() { + // Bounds, the packed range direction and a packed structure of the same + // width, state count and signedness do not matter (IEEE 1800-2023 + // 6.22.2, 7.6). + let source = r#" + module Top(output logic [7:0] q, output logic [7:0] r); + typedef struct packed { logic [3:0] hi; logic [3:0] lo; } pair_t; + function automatic logic [7:0] first(input logic [7:0] x [1:0][2]); + return x[1][0]; + endfunction + logic [0:7] row [5:6]; + pair_t pairs [2]; + logic [7:0] grid [2][2]; + always_comb begin + row[5] = 8'd1; + row[6] = 8'd2; + pairs = row; + grid = '{row, '{8'd0, 8'd0}}; + q = first('{pairs, row}); + r = grid[0][0]; + end + endmodule + "#; + analyze_source(source, Path::new("equivalent_unpacked.sv")) + .expect("equivalent unpacked array types are assignment compatible"); +} + #[test] fn flattens_partial_unpacked_array_selections() { let ir = analyze_source( diff --git a/crates/celox-sv-analyzer/src/typecheck.rs b/crates/celox-sv-analyzer/src/typecheck.rs index 7466f14288..fc142a9790 100644 --- a/crates/celox-sv-analyzer/src/typecheck.rs +++ b/crates/celox-sv-analyzer/src/typecheck.rs @@ -14,6 +14,45 @@ pub struct IntegralLiteral { pub mask: BigUint, } +/// The type of a fixed-size unpacked array of an integral element type. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct UnpackedArrayType { + /// The element count of each unpacked dimension, slowest varying first. + pub dims: Vec, + /// The width of the integral element type. + pub element_width: usize, + pub signed: bool, + pub four_state: bool, +} + +impl UnpackedArrayType { + /// Whether a value of this type can be assigned to an array of `target` + /// (IEEE 1800-2023 7.6): the element types are equivalent, which for + /// integral types means the same width, state count and signedness + /// (6.22.2), and every dimension has the same element count; the bounds + /// themselves do not matter. + pub fn is_assignment_compatible_with(&self, target: &Self) -> bool { + self == target + } +} + +impl std::fmt::Display for UnpackedArrayType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(if self.four_state { "logic" } else { "bit" })?; + if self.signed { + f.write_str(" signed")?; + } + if self.element_width > 1 { + write!(f, " [{}:0]", self.element_width - 1)?; + } + f.write_str(" ")?; + for count in &self.dims { + write!(f, "[{count}]")?; + } + Ok(()) + } +} + /// Width and signedness of the supported bit vector system functions (20.9). pub fn bit_vector_function_return_type(name: &str, arity: usize) -> Option<(usize, bool)> { if arity != 1 { diff --git a/crates/celox-test-suite/LIMITATIONS.md b/crates/celox-test-suite/LIMITATIONS.md index 48e36cd289..a096945778 100644 --- a/crates/celox-test-suite/LIMITATIONS.md +++ b/crates/celox-test-suite/LIMITATIONS.md @@ -369,9 +369,12 @@ The SystemVerilog suite's exclusions are listed in [`verification/sv/limitations | `sv_icarus_unary_plus_unknown` | execute | icarus | 1 | Icarus 13.0 returns the operand of a unary plus unchanged when it has X or Z bits; IEEE 1800-2023 11.4.3 makes the result of an arithmetic operator all X. Verilator cannot check four-state values. | | `sv_icarus_runtime_packed_subselect` | compile | icarus | 1 | Icarus 13.0 rejects a further select after a runtime index into a multidimensional packed array (t[k][j], t[k][2:1]: "Array index expressions must be constant here"). Verilator passes the case. | | `sv_icarus_packed_inner_index_range` | execute | icarus | 1 | Icarus 13.0 flattens an out-of-range inner index of a multidimensional packed array into the neighbouring element: a read gives a known value instead of X, and a write lands in that element; IEEE 1800-2023 7.4.6 and 11.5.1 make the select out of range. Verilator cannot check four-state values, and it truncates out-of-range packed indices. | +| `sv_icarus_unpacked_subroutine_ports` | compile | icarus | 2 | Icarus 13.0 reports that subroutine ports with unpacked dimensions are not yet supported. Verilator passes the valid case. | +| `sv_icarus_unpacked_array_patterns` | compile | icarus | 1 | Icarus 13.0 reports that an assignment pattern with unpacked array items assigned to a multidimensional unpacked array is not yet supported ("Procedural assignment of array or array slice"), so the run cannot count it as a language rejection. The design is invalid (IEEE 1800-2023 7.6, 10.8). | | `sv_verilator_constant_unknown_bits` | execute | verilator | 1 | Verilator 5.052 is two-state: unknown bits in constant system function arguments ($onehot(2'bx1)) are not kept unknown, so a generate condition differs from IEEE 1800-2023 20.9. Icarus agrees with the expected values. | | `sv_verilator_generate_case_unknown_label` | compile | verilator | 1 | Verilator 5.052 rejects an x or ? digit in a case-generate label ("no such thing as generate casez"). Icarus passes the case. | | `sv_verilator_self_determined_index` | execute | verilator | 1 | Verilator 5.052 does not wrap a self-determined index expression (2'd3 + 2'd1) to its width before selecting; IEEE 1800-2023 11.5.1 and 11.6.1 make the index 0. Icarus agrees with the expected value. | | `sv_verilator_function_outputs` | compile | verilator | 1 | Verilator 5.052 fails with an internal error (V3DfgSynthesize: Non-ReadOnly reference) on output and inout function arguments. | | `sv_verilator_generate_name_collision` | compile | verilator | 1 | Verilator 5.052 rejects a generate block named like a port with an "Unsupported" diagnostic instead of a duplicate-name error, so the run cannot count it as a language rejection. The design is invalid (IEEE 1800-2023 5.6.1, 23.9) and Icarus rejects it as a duplicate declaration. | | `sv_verilator_part_select_write_overhang` | execute | verilator | 1 | Verilator 5.052 writes the out-of-range bit of a partially out-of-range indexed part-select (minus_write[i -: 2] with i = 0) into bit 7 instead of discarding it; IEEE 1800-2023 11.5.1 ignores writes to out-of-range bits. Icarus agrees with the expected value. | +| `sv_verilator_unpacked_array_extension` | compile | verilator | 2 | Verilator 5.052 rejects an unpacked array argument or assignment pattern item with a narrower element type through "EXTEND unexpected in assignment to unpacked array" (EXTENDS for a signed element), a diagnostic about its own width extension rather than the element types, so the run cannot count it as a language rejection. The designs are invalid (IEEE 1800-2023 7.6, 10.8). | diff --git a/crates/celox-test-suite/src/icarus.rs b/crates/celox-test-suite/src/icarus.rs index 0da7b58ce4..318685ba76 100644 --- a/crates/celox-test-suite/src/icarus.rs +++ b/crates/celox-test-suite/src/icarus.rs @@ -143,6 +143,10 @@ pub fn is_source_rejection(code: Option, log: &str, sources: &[PathBuf]) -> "Bit select expressions must be a constant integral value." | "Indexed part select base expression must be a constant integral value in this context." | "Output port expression must support a continuous assignment." + // IEEE 1800-2023 6.22.2 and 7.6: unpacked arrays of + // nonequivalent element types or different sizes. + | "Element types are not compatible in array assignment." + | "Unpacked dimensions are not compatible in array assignment." ) || (error.starts_with("A reference to a net or variable (`") && error.ends_with("') is not allowed in a constant expression.")) || (error.starts_with("Array ") && error.ends_with(" needs an array index here.")) diff --git a/crates/celox-test-suite/src/sv/cases/indexed_select.vtest b/crates/celox-test-suite/src/sv/cases/indexed_select.vtest index 4a198e563f..b95b58d8b2 100644 --- a/crates/celox-test-suite/src/sv/cases/indexed_select.vtest +++ b/crates/celox-test-suite/src/sv/cases/indexed_select.vtest @@ -741,3 +741,23 @@ (modify (set i value) (set j j)) (assert_eq b (? (< j 4) (bit value j) (fourstate 1 1)) "b") (assert_eq v (? (< j 4) (<< 1 j) 0) "v")))) + +(case runtime_index_reads_single_bit_unpacked_elements + ; A run-time index selects one element of an unpacked array of 1-bit + ; elements; `[32]` declares `[0:31]` (IEEE 1800-2023 7.4.2, 7.4.6). + (source "single_bit_unpacked_index.sv" #" + module Top(input logic clk, input logic [4:0] p, input logic [31:0] d, + output logic q, output logic q_ff); + logic v [32]; + always_comb for (int i = 0; i < 32; i++) v[i] = d[i]; + always_comb q = v[p]; + always_ff @(posedge clk) q_ff <= v[p]; + endmodule + "#) + (top Top) + (for value (list 0xa5c30f81 0x5a3cf07e) + (for p (range 0 32) + (modify (set d value) (set p p)) + (assert_eq q (bit value p) "q") + (tick clk) + (assert_eq q_ff (bit value p) "q_ff")))) diff --git a/crates/celox-test-suite/src/sv/cases/types.vtest b/crates/celox-test-suite/src/sv/cases/types.vtest index fd263a7cb8..a0a4db4cc0 100644 --- a/crates/celox-test-suite/src/sv/cases/types.vtest +++ b/crates/celox-test-suite/src/sv/cases/types.vtest @@ -134,3 +134,146 @@ (eval) (expand (name expected) ((y0 0xfffa) (y1 1) (y2 0xfffb) (y3 1) (y4 2) (y5 7)) (assert_eq {name} {expected} "{name}"))) + +(case passes_unpacked_arrays_of_equivalent_element_types + ; An unpacked array assignment or port connection needs equivalent + ; element types and equal element counts, not equal bounds; elements + ; correspond left to right (IEEE 1800-2023 6.22.2, 7.6, 23.3.3). + ; `logic [0:7]` and the packed structure are equivalent to `logic [7:0]`. + (source "equivalent_unpacked_arrays.sv" #" + module Child(input logic [7:0] x [1:0], output logic [7:0] left, right); + assign left = x[1]; + assign right = x[0]; + endmodule + module Top(input logic [15:0] d, output logic [7:0] left, right, + output logic [7:0] s_left, s_right, c_left); + typedef struct packed { logic [3:0] hi; logic [3:0] lo; } pair_t; + logic [0:7] v [2:3]; + pair_t s [2]; + logic [7:0] c [2]; + assign v[2] = d[15:8]; + assign v[3] = d[7:0]; + assign s[0] = d[7:0]; + assign s[1] = d[15:8]; + assign c = v; + assign c_left = c[0]; + Child uv(.x(v), .left(left), .right(right)); + Child us(.x(s), .left(s_left), .right(s_right)); + endmodule + "#) + (top Top) + (modify (set d 0x12c4)) + (assert_eq left 0x12) + (assert_eq right 0xc4) + (assert_eq s_left 0xc4) + (assert_eq s_right 0x12) + (assert_eq c_left 0x12)) + +(case passes_unpacked_arguments_of_equivalent_element_types + ; Passing an argument is an assignment-like context (IEEE 1800-2023 + ; 10.8): bounds and the packed range direction of the elements do not + ; matter (6.22.2, 7.6). + (source "equivalent_unpacked_arguments.sv" #" + module Top(input logic [15:0] d, output logic [7:0] left, right); + function automatic logic [7:0] pick(input logic [7:0] x [1:0], input logic first); + return first ? x[1] : x[0]; + endfunction + logic [0:7] v [2:3]; + always_comb begin + v[2] = d[15:8]; + v[3] = d[7:0]; + left = pick(v, 1'b1); + right = pick(v, 1'b0); + end + endmodule + "#) + (top Top) + (modify (set d 0x12c4)) + (assert_eq left 0x12) + (assert_eq right 0xc4)) + +(case rejects_unpacked_argument_of_a_wider_element_type + ; Passing an argument is an assignment-like context, and an unpacked + ; array needs an equivalent element type (IEEE 1800-2023 7.6, 10.8). + (source "wider_unpacked_argument.sv" #" + module Top(output logic signed [7:0] q); + function automatic logic signed [7:0] pick(input logic signed [7:0] x [2]); + return x[0]; + endfunction + logic signed [3:0] narrow [2]; + always_comb begin + narrow[0] = 4'sh8; + narrow[1] = 4'sh1; + q = pick(narrow); + end + endmodule + "#) + (top Top) + (expect reject)) + +(case rejects_unpacked_assignment_of_a_different_state_count + ; `bit [7:0]` and `logic [7:0]` are not equivalent (IEEE 1800-2023 + ; 6.22.2, 7.6). + (source "two_state_unpacked_assignment.sv" #" + module Top(input logic [7:0] a, output logic [7:0] q); + bit [7:0] two [2]; + logic [7:0] four [2]; + assign two[0] = a; + assign two[1] = a; + assign four = two; + assign q = four[0]; + endmodule + "#) + (top Top) + (expect reject)) + +(case rejects_unpacked_assignment_of_a_different_element_count + ; A fixed-size target needs as many elements as the source (IEEE + ; 1800-2023 7.6). + (source "unpacked_assignment_count.sv" #" + module Top(input logic [7:0] a, output logic [7:0] q); + logic [7:0] three [3]; + logic [7:0] two [2]; + assign three[0] = a; + assign three[1] = a; + assign three[2] = a; + assign two = three; + assign q = two[0]; + endmodule + "#) + (top Top) + (expect reject)) + +(case rejects_unpacked_port_connection_of_a_narrower_element_type + ; A port connection is an assignment-like context (IEEE 1800-2023 7.6, + ; 10.8, 23.3.3). + (source "narrow_unpacked_port.sv" #" + module Child(input logic [7:0] x [2], output logic [7:0] y); + assign y = x[0]; + endmodule + module Top(input logic [3:0] a, output logic [7:0] q); + logic [3:0] narrow [2]; + assign narrow[0] = a; + assign narrow[1] = a; + Child u(.x(narrow), .y(q)); + endmodule + "#) + (top Top) + (expect reject)) + +(case rejects_assignment_pattern_item_of_a_narrower_array_type + ; Each nondefault item of an assignment pattern is in an assignment-like + ; context with its element, here an unpacked array (IEEE 1800-2023 7.6, + ; 10.8). + (source "narrow_pattern_item.sv" #" + module Top(input logic [3:0] a, output logic [7:0] q); + logic [3:0] row [2]; + logic [7:0] grid [2][2]; + assign row[0] = a; + assign row[1] = a; + assign grid = '{row, row}; + assign q = grid[0][0]; + endmodule + "#) + (top Top) + (expect reject)) diff --git a/crates/celox-test-suite/src/verilator.rs b/crates/celox-test-suite/src/verilator.rs index 6d5782781b..c702c6bca6 100644 --- a/crates/celox-test-suite/src/verilator.rs +++ b/crates/celox-test-suite/src/verilator.rs @@ -137,6 +137,13 @@ pub fn is_source_rejection(code: Option, log: &str, sources: &[PathBuf]) -> let reviewed = source_diagnostic(error, sources).is_some_and(|diagnostic| { diagnostic == "Illegal assignment: types are not assignment compatible (IEEE 1800-2023 7.6)" + // IEEE 1800-2023 6.22.2 and 7.6: unpacked arrays of + // nonequivalent element types or different sizes. + || diagnostic + == "Illegal assignment: Array element types are not equivalent (IEEE 1800-2023 6.22.2)" + || diagnostic + == "Assignment between 2-state and 4-state types requires equivalent element types (IEEE 1800-2023 6.22.2, 7.6)" + || diagnostic.starts_with("Illegal assignment: Unmatched array sizes in dimension ") // IEEE 1800-2023 23.3.3.5: an instance array connection width. || (diagnostic.starts_with("Input port connection '") && diagnostic.contains("' as part of a module instance array requires ")) @@ -151,9 +158,10 @@ pub fn is_source_rejection(code: Option, log: &str, sources: &[PathBuf]) -> rejected = true; source_context = true; } else if let Some(warning) = line.strip_prefix("%Warning-WIDTHEXPAND: ") { - if !source_diagnostic(warning, sources) - .is_some_and(|text| text.starts_with("Operator ASSIGN expects ")) - { + if !source_diagnostic(warning, sources).is_some_and(|text| { + text.starts_with("Operator ASSIGN expects ") + || text.starts_with("Input port connection '") + }) { return false; } source_context = true; @@ -186,6 +194,10 @@ fn is_diagnostic_context(line: &str) -> bool { ": ... note: In instance '", ": ... Left-hand data type: '", ": ... Right-hand data type: '", + ": ... Left-hand type: '", + ": ... Right-hand type: '", + ": ... Pin data type: '", + ": ... Expression data type: '", "... See the manual at https://verilator.org/verilator_doc.html?", "... For warning description see https://verilator.org/warn/WIDTHEXPAND?", "... Use \"/* verilator lint_off WIDTHEXPAND */\" and lint_on around source to disable this message.", diff --git a/crates/celox-test-suite/tests/source_rejection.rs b/crates/celox-test-suite/tests/source_rejection.rs index 1c9220bbbf..862e9c5760 100644 --- a/crates/celox-test-suite/tests/source_rejection.rs +++ b/crates/celox-test-suite/tests/source_rejection.rs @@ -2,6 +2,18 @@ #![cfg(any(feature = "verilator", feature = "icarus"))] use std::path::PathBuf; +/// The emitted sources a retained diagnostic names. +fn logged_sources(log: &str) -> Vec { + let mut sources: Vec = log + .split(|c: char| c.is_whitespace() || c == ':') + .filter(|word| word.starts_with("/") && word.ends_with(".sv")) + .map(PathBuf::from) + .collect(); + sources.sort(); + sources.dedup(); + sources +} + #[cfg(feature = "icarus")] mod icarus { use super::*; @@ -35,6 +47,36 @@ mod icarus { assert_eq!(checked, 8); } + #[test] + fn retained_systemverilog_negative_diagnostics_are_source_rejections() { + let report: serde_json::Value = + serde_json::from_str(include_str!("../verification/sv/icarus.json")).unwrap(); + let mut checked = 0; + for case in report["cases"].as_array().unwrap() { + if case["status"] != "rejected" { + continue; + } + let (status, log) = case["detail"].as_str().unwrap().split_once('\n').unwrap(); + let code = status + .strip_prefix("Icarus build exit status: ") + .unwrap() + .split(';') + .next() + .unwrap() + .parse() + .unwrap(); + let sources = logged_sources(log); + assert!( + is_source_rejection(Some(code), log, &sources), + "{}", + case["name"] + ); + assert!(!is_source_rejection(Some(code), log, &["other.sv".into()])); + checked += 1; + } + assert_eq!(checked, 6); + } + #[test] fn incomplete_or_unattributed_diagnostics_do_not_prove_rejection() { let sources = [PathBuf::from("design.sv")]; @@ -99,4 +141,27 @@ mod verilator { } assert_eq!(checked, 1); } + + #[test] + fn retained_systemverilog_negative_diagnostics_are_source_rejections() { + let report: serde_json::Value = + serde_json::from_str(include_str!("../verification/sv/verilator.json")).unwrap(); + let mut checked = 0; + for case in report["cases"].as_array().unwrap() { + if case["status"] != "rejected" { + continue; + } + let (_, log) = case["detail"].as_str().unwrap().split_once('\n').unwrap(); + let sources = logged_sources(log); + assert!( + is_source_rejection(Some(1), log, &sources), + "{}", + case["name"] + ); + assert!(!is_source_rejection(Some(1), log, &["other.sv".into()])); + assert!(!is_source_rejection(None, log, &sources)); + checked += 1; + } + assert_eq!(checked, 6); + } } diff --git a/crates/celox-test-suite/verification/sv/icarus.json b/crates/celox-test-suite/verification/sv/icarus.json index 2a7c0a8058..903e617f59 100644 --- a/crates/celox-test-suite/verification/sv/icarus.json +++ b/crates/celox-test-suite/verification/sv/icarus.json @@ -952,6 +952,17 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Arrays", + "detail": "", + "expectation": "Simulation", + "name": "indexed_select::runtime_index_reads_single_bit_unpacked_elements", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Arrays", "detail": "", @@ -2424,6 +2435,40 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Types", + "detail": "Icarus 13.0 reports that subroutine ports with unpacked dimensions are not yet supported. Verilator passes the valid case.", + "expectation": "Simulation", + "known_issue": { + "category": "simulator_unsupported", + "evidence": [ + "verification/sv/limitations/icarus.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "id": "sv_icarus_unpacked_subroutine_ports", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "phase": "compile", + "reason": "Icarus 13.0 reports that subroutine ports with unpacked dimensions are not yet supported. Verilator passes the valid case.", + "upstream": [] + }, + "name": "types::passes_unpacked_arguments_of_equivalent_element_types", + "phase": "skip", + "status": "ignored", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Types", + "detail": "", + "expectation": "Simulation", + "name": "types::passes_unpacked_arrays_of_equivalent_element_types", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Types", "detail": "", @@ -2435,6 +2480,85 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Types", + "detail": "Icarus 13.0 reports that an assignment pattern with unpacked array items assigned to a multidimensional unpacked array is not yet supported (\"Procedural assignment of array or array slice\"), so the run cannot count it as a language rejection. The design is invalid (IEEE 1800-2023 7.6, 10.8).", + "expectation": "CompilationError", + "known_issue": { + "category": "simulator_unsupported", + "evidence": [ + "verification/sv/limitations/icarus.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "id": "sv_icarus_unpacked_array_patterns", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "phase": "compile", + "reason": "Icarus 13.0 reports that an assignment pattern with unpacked array items assigned to a multidimensional unpacked array is not yet supported (\"Procedural assignment of array or array slice\"), so the run cannot count it as a language rejection. The design is invalid (IEEE 1800-2023 7.6, 10.8).", + "upstream": [] + }, + "name": "types::rejects_assignment_pattern_item_of_a_narrower_array_type", + "phase": "skip", + "status": "ignored", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Types", + "detail": "Icarus 13.0 reports that subroutine ports with unpacked dimensions are not yet supported. Verilator passes the valid case.", + "expectation": "CompilationError", + "known_issue": { + "category": "simulator_unsupported", + "evidence": [ + "verification/sv/limitations/icarus.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "id": "sv_icarus_unpacked_subroutine_ports", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "phase": "compile", + "reason": "Icarus 13.0 reports that subroutine ports with unpacked dimensions are not yet supported. Verilator passes the valid case.", + "upstream": [] + }, + "name": "types::rejects_unpacked_argument_of_a_wider_element_type", + "phase": "skip", + "status": "ignored", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Types", + "detail": "Icarus build exit status: 1; see /build.log\n/source_0.sv:8: error: Unpacked dimensions are not compatible in array assignment.\n1 error(s) during elaboration.\n", + "expectation": "CompilationError", + "name": "types::rejects_unpacked_assignment_of_a_different_element_count", + "phase": "compile", + "status": "rejected", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Types", + "detail": "Icarus build exit status: 1; see /build.log\n/source_0.sv:7: error: Element types are not compatible in array assignment.\n1 error(s) during elaboration.\n", + "expectation": "CompilationError", + "name": "types::rejects_unpacked_assignment_of_a_different_state_count", + "phase": "compile", + "status": "rejected", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Types", + "detail": "Icarus build exit status: 1; see /build.log\n/source_0.sv:9: error: Element types are not compatible in array assignment.\n/source_0.sv:9: : Port 1 (x) of Child is connected to narrow\n1 error(s) during elaboration.\n", + "expectation": "CompilationError", + "name": "types::rejects_unpacked_port_connection_of_a_narrower_element_type", + "phase": "compile", + "status": "rejected", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Types", "detail": "", @@ -2461,10 +2585,10 @@ "counts": { "compile_error": 0, "emission_error": 0, - "ignored": 37, + "ignored": 40, "mismatch": 0, - "passed": 143, - "rejected": 3, + "passed": 145, + "rejected": 6, "runtime_error": 0, "unexpected_accept": 0, "unsupported": 0 diff --git a/crates/celox-test-suite/verification/sv/limitations.json b/crates/celox-test-suite/verification/sv/limitations.json index 25ebd55765..7d11ca825b 100644 --- a/crates/celox-test-suite/verification/sv/limitations.json +++ b/crates/celox-test-suite/verification/sv/limitations.json @@ -356,6 +356,41 @@ ] } }, + { + "id": "sv_icarus_unpacked_subroutine_ports", + "category": "simulator_unsupported", + "phase": "compile", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "reason": "Icarus 13.0 reports that subroutine ports with unpacked dimensions are not yet supported. Verilator passes the valid case.", + "upstream": [], + "evidence": [ + "verification/sv/limitations/icarus.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "cases": { + "icarus": [ + "types::passes_unpacked_arguments_of_equivalent_element_types", + "types::rejects_unpacked_argument_of_a_wider_element_type" + ] + } + }, + { + "id": "sv_icarus_unpacked_array_patterns", + "category": "simulator_unsupported", + "phase": "compile", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "reason": "Icarus 13.0 reports that an assignment pattern with unpacked array items assigned to a multidimensional unpacked array is not yet supported (\"Procedural assignment of array or array slice\"), so the run cannot count it as a language rejection. The design is invalid (IEEE 1800-2023 7.6, 10.8).", + "upstream": [], + "evidence": [ + "verification/sv/limitations/icarus.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "cases": { + "icarus": [ + "types::rejects_assignment_pattern_item_of_a_narrower_array_type" + ] + } + }, { "id": "sv_verilator_constant_unknown_bits", "category": "simulator_two_state", @@ -457,5 +492,23 @@ "indexed_select::runtime_indexed_writes_keep_unselected_bits_and_clip_overhang" ] } + }, + { + "id": "sv_verilator_unpacked_array_extension", + "category": "simulator_compile_limitation", + "phase": "compile", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "reason": "Verilator 5.052 rejects an unpacked array argument or assignment pattern item with a narrower element type through \"EXTEND unexpected in assignment to unpacked array\" (EXTENDS for a signed element), a diagnostic about its own width extension rather than the element types, so the run cannot count it as a language rejection. The designs are invalid (IEEE 1800-2023 7.6, 10.8).", + "upstream": [], + "evidence": [ + "verification/sv/limitations/verilator.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "cases": { + "verilator": [ + "types::rejects_assignment_pattern_item_of_a_narrower_array_type", + "types::rejects_unpacked_argument_of_a_wider_element_type" + ] + } } ] diff --git a/crates/celox-test-suite/verification/sv/limitations/icarus.json b/crates/celox-test-suite/verification/sv/limitations/icarus.json index 85dbad50d1..4d92619676 100644 --- a/crates/celox-test-suite/verification/sv/limitations/icarus.json +++ b/crates/celox-test-suite/verification/sv/limitations/icarus.json @@ -9,7 +9,7 @@ "unexpected_accept": 1, "mismatch": 7, "emission_error": 0, - "compile_error": 27, + "compile_error": 30, "runtime_error": 2, "unsupported": 0, "ignored": 0 @@ -295,6 +295,30 @@ "phase": "execute", "status": "mismatch" }, + { + "category": "Types", + "detail": "Icarus build exit status: 3; see /build.log\n/source_0.sv:3: sorry: Subroutine ports with unpacked dimensions are not yet supported.\n/source_0.sv:10: error: Array v needs an array index here.\n/source_0.sv:11: error: Array v needs an array index here.\n/source_0.sv:8: sorry: constant selects in always_* processes are not fully supported (the process will be sensitive to all bits in 'd[15:0]').\n/source_0.sv:9: sorry: constant selects in always_* processes are not fully supported (the process will be sensitive to all bits in 'd[15:0]').\n3 error(s) during elaboration.\n", + "expectation": "Simulation", + "name": "types::passes_unpacked_arguments_of_equivalent_element_types", + "phase": "compile", + "status": "compile_error" + }, + { + "category": "Types", + "detail": "Icarus build exit status: 2; see /build.log\n/source_0.sv:7: sorry: Procedural assignment of array or array slice is not yet supported.\n/source_0.sv:7: sorry: Procedural assignment of array or array slice is not yet supported.\n2 error(s) during elaboration.\n", + "expectation": "CompilationError", + "name": "types::rejects_assignment_pattern_item_of_a_narrower_array_type", + "phase": "compile", + "status": "compile_error" + }, + { + "category": "Types", + "detail": "Icarus build exit status: 2; see /build.log\n/source_0.sv:3: sorry: Subroutine ports with unpacked dimensions are not yet supported.\n/source_0.sv:10: error: Array narrow needs an array index here.\n/source_0.sv:7: warning: always_comb process has no sensitivities.\n2 error(s) during elaboration.\n", + "expectation": "CompilationError", + "name": "types::rejects_unpacked_argument_of_a_wider_element_type", + "phase": "compile", + "status": "compile_error" + }, { "category": "Regression", "detail": "Icarus build exit status: 6; see /build.log\n/source_0.sv:9: syntax error\n/source_0.sv:9: error: Malformed statement\n/source_0.sv:10: syntax error\n/source_0.sv:10: error: Malformed statement\n/source_0.sv:18: syntax error\n/source_0.sv:18: error: Malformed statement\n", diff --git a/crates/celox-test-suite/verification/sv/limitations/verilator.json b/crates/celox-test-suite/verification/sv/limitations/verilator.json index 549066a0c4..2435b25fe6 100644 --- a/crates/celox-test-suite/verification/sv/limitations/verilator.json +++ b/crates/celox-test-suite/verification/sv/limitations/verilator.json @@ -9,7 +9,7 @@ "unexpected_accept": 0, "mismatch": 2, "emission_error": 0, - "compile_error": 3, + "compile_error": 5, "runtime_error": 1, "unsupported": 0, "ignored": 0 @@ -62,6 +62,22 @@ "name": "indexed_select::runtime_indexed_writes_keep_unselected_bits_and_clip_overhang", "phase": "execute", "status": "mismatch" + }, + { + "category": "Types", + "detail": "Verilator build exit status: 1; see /build.log\n%Warning-WIDTHEXPAND: /source_0.sv:7:37: Operator PATMEMBER expects 8 bits on the Pattern value, but Pattern value's VARREF 'row' generates 4 bits.\n : ... note: In instance 'Top'\n 7 | assign grid = '{row, row};\n | ^~~\n ... For warning description see https://verilator.org/warn/WIDTHEXPAND?v=5.052\n ... Use \"/* verilator lint_off WIDTHEXPAND */\" and lint_on around source to disable this message.\n%Warning-WIDTHEXPAND: /source_0.sv:7:42: Operator PATMEMBER expects 8 bits on the Pattern value, but Pattern value's VARREF 'row' generates 4 bits.\n : ... note: In instance 'Top'\n 7 | assign grid = '{row, row};\n | ^~~\n%Error: /source_0.sv:7:37: EXTEND unexpected in assignment to unpacked array\n 7 | assign grid = '{row, row};\n | ^~~\n ... See the manual at https://verilator.org/verilator_doc.html?v=5.052 for more assistance.\n%Error: /source_0.sv:7:42: EXTEND unexpected in assignment to unpacked array\n 7 | assign grid = '{row, row};\n | ^~~\n%Error: Exiting due to 2 error(s)\n", + "expectation": "CompilationError", + "name": "types::rejects_assignment_pattern_item_of_a_narrower_array_type", + "phase": "compile", + "status": "compile_error" + }, + { + "category": "Types", + "detail": "Verilator build exit status: 1; see /build.log\n%Warning-WIDTHEXPAND: /source_0.sv:10:29: Operator FUNCREF 'pick' expects 8 bits on the Function Argument, but Function Argument's VARREF 'narrow' generates 4 bits.\n : ... note: In instance 'Top'\n 10 | q = pick(narrow);\n | ^~~~\n ... For warning description see https://verilator.org/warn/WIDTHEXPAND?v=5.052\n ... Use \"/* verilator lint_off WIDTHEXPAND */\" and lint_on around source to disable this message.\n%Error: /source_0.sv:10:34: EXTENDS unexpected in assignment to unpacked array\n 10 | q = pick(narrow);\n | ^~~~~~\n ... See the manual at https://verilator.org/verilator_doc.html?v=5.052 for more assistance.\n%Error: Exiting due to 1 error(s)\n", + "expectation": "CompilationError", + "name": "types::rejects_unpacked_argument_of_a_wider_element_type", + "phase": "compile", + "status": "compile_error" } ] } diff --git a/crates/celox-test-suite/verification/sv/verilator.json b/crates/celox-test-suite/verification/sv/verilator.json index 69c3f84ea3..7744d16eaa 100644 --- a/crates/celox-test-suite/verification/sv/verilator.json +++ b/crates/celox-test-suite/verification/sv/verilator.json @@ -796,6 +796,17 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Arrays", + "detail": "", + "expectation": "Simulation", + "name": "indexed_select::runtime_index_reads_single_bit_unpacked_elements", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Arrays", "detail": "", @@ -2052,6 +2063,28 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Types", + "detail": "", + "expectation": "Simulation", + "name": "types::passes_unpacked_arguments_of_equivalent_element_types", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Types", + "detail": "", + "expectation": "Simulation", + "name": "types::passes_unpacked_arrays_of_equivalent_element_types", + "phase": "execute", + "status": "passed", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Types", "detail": "four-state expectations", @@ -2063,6 +2096,85 @@ "tag_reasons": {}, "tags": [] }, + { + "category": "Types", + "detail": "Verilator 5.052 rejects an unpacked array argument or assignment pattern item with a narrower element type through \"EXTEND unexpected in assignment to unpacked array\" (EXTENDS for a signed element), a diagnostic about its own width extension rather than the element types, so the run cannot count it as a language rejection. The designs are invalid (IEEE 1800-2023 7.6, 10.8).", + "expectation": "CompilationError", + "known_issue": { + "category": "simulator_compile_limitation", + "evidence": [ + "verification/sv/limitations/verilator.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "id": "sv_verilator_unpacked_array_extension", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "phase": "compile", + "reason": "Verilator 5.052 rejects an unpacked array argument or assignment pattern item with a narrower element type through \"EXTEND unexpected in assignment to unpacked array\" (EXTENDS for a signed element), a diagnostic about its own width extension rather than the element types, so the run cannot count it as a language rejection. The designs are invalid (IEEE 1800-2023 7.6, 10.8).", + "upstream": [] + }, + "name": "types::rejects_assignment_pattern_item_of_a_narrower_array_type", + "phase": "skip", + "status": "ignored", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Types", + "detail": "Verilator 5.052 rejects an unpacked array argument or assignment pattern item with a narrower element type through \"EXTEND unexpected in assignment to unpacked array\" (EXTENDS for a signed element), a diagnostic about its own width extension rather than the element types, so the run cannot count it as a language rejection. The designs are invalid (IEEE 1800-2023 7.6, 10.8).", + "expectation": "CompilationError", + "known_issue": { + "category": "simulator_compile_limitation", + "evidence": [ + "verification/sv/limitations/verilator.json", + "LIMITATIONS.md#systemverilog-suite" + ], + "id": "sv_verilator_unpacked_array_extension", + "observed_version": "Celox SV frontend; Verilator 5.052 / Icarus 13.0", + "phase": "compile", + "reason": "Verilator 5.052 rejects an unpacked array argument or assignment pattern item with a narrower element type through \"EXTEND unexpected in assignment to unpacked array\" (EXTENDS for a signed element), a diagnostic about its own width extension rather than the element types, so the run cannot count it as a language rejection. The designs are invalid (IEEE 1800-2023 7.6, 10.8).", + "upstream": [] + }, + "name": "types::rejects_unpacked_argument_of_a_wider_element_type", + "phase": "skip", + "status": "ignored", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Types", + "detail": "Verilator build exit status: 1; see /build.log\n%Error: /source_0.sv:8:32: Illegal assignment: Unmatched array sizes in dimension 0 (2 vs 3)\n : ... note: In instance 'Top'\n 8 | assign two = three;\n | ^\n ... See the manual at https://verilator.org/verilator_doc.html?v=5.052 for more assistance.\n%Error: Exiting due to 1 error(s)\n", + "expectation": "CompilationError", + "name": "types::rejects_unpacked_assignment_of_a_different_element_count", + "phase": "compile", + "status": "rejected", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Types", + "detail": "Verilator build exit status: 1; see /build.log\n%Error: /source_0.sv:7:33: Assignment between 2-state and 4-state types requires equivalent element types (IEEE 1800-2023 6.22.2, 7.6)\n : ... note: In instance 'Top'\n : ... Left-hand type: 'logic[7:0]$[0:1]'(4-state)\n : ... Right-hand type: 'bit[7:0]$[0:1]'(2-state)\n 7 | assign four = two;\n | ^\n ... See the manual at https://verilator.org/verilator_doc.html?v=5.052 for more assistance.\n%Error: /source_0.sv:7:33: Illegal assignment: Array element types are not equivalent (IEEE 1800-2023 6.22.2)\n : ... note: In instance 'Top'\n : ... Left-hand data type: 'logic[7:0]$[0:1]' (4-state)\n : ... Right-hand data type: 'bit[7:0]$[0:1]' (2-state)\n 7 | assign four = two;\n | ^\n%Error: Exiting due to 2 error(s)\n", + "expectation": "CompilationError", + "name": "types::rejects_unpacked_assignment_of_a_different_state_count", + "phase": "compile", + "status": "rejected", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, + { + "category": "Types", + "detail": "Verilator build exit status: 1; see /build.log\n%Error: /source_0.sv:9:30: Illegal assignment: Array element types are not equivalent (IEEE 1800-2023 6.22.2)\n : ... note: In instance 'Top'\n : ... Pin data type: 'logic[7:0]$[0:1]' (4-state)\n : ... Expression data type: 'logic[3:0]$[0:1]' (4-state)\n 9 | Child u(.x(narrow), .y(q));\n | ^\n ... See the manual at https://verilator.org/verilator_doc.html?v=5.052 for more assistance.\n%Warning-WIDTHEXPAND: /source_0.sv:9:30: Input port connection 'x' expects 8 bits on the pin connection, but pin connection's VARREF 'narrow' generates 4 bits.\n : ... note: In instance 'Top'\n 9 | Child u(.x(narrow), .y(q));\n | ^\n ... For warning description see https://verilator.org/warn/WIDTHEXPAND?v=5.052\n ... Use \"/* verilator lint_off WIDTHEXPAND */\" and lint_on around source to disable this message.\n%Error: Exiting due to 1 error(s)\n", + "expectation": "CompilationError", + "name": "types::rejects_unpacked_port_connection_of_a_narrower_element_type", + "phase": "compile", + "status": "rejected", + "stronger_than_sv": false, + "tag_reasons": {}, + "tags": [] + }, { "category": "Types", "detail": "four-state expectations", @@ -2089,10 +2201,10 @@ "counts": { "compile_error": 0, "emission_error": 0, - "ignored": 6, + "ignored": 8, "mismatch": 0, - "passed": 138, - "rejected": 3, + "passed": 141, + "rejected": 6, "runtime_error": 0, "unexpected_accept": 0, "unsupported": 36 diff --git a/crates/celox/tests/comb_observer.rs b/crates/celox/tests/comb_observer.rs index 66753b3b2b..7b6d2554aa 100644 --- a/crates/celox/tests/comb_observer.rs +++ b/crates/celox/tests/comb_observer.rs @@ -2587,6 +2587,10 @@ fn test_comb_function_nested_array_scalar_default_converts_each_element(sim) { } fn test_comb_function_array_literal_array_item_preserves_element_type(sim) { + // sv: Veryl emits a call that passes an unpacked array whose element type differs from the + // formal's, which IEEE 1800-2023 7.6 and 10.8 make illegal. The SV frontend rejects it + // ("assignment pattern item: an unpacked array of type `logic signed [3:0] [2]` is not + // assignment compatible with `logic signed [7:0] [2]`") instead of reinterpreting its bits. @ignore_on(veryl, sv); @case "comb_observer::test_comb_function_array_literal_array_item_preserves_element_type"; } @@ -2597,6 +2601,10 @@ fn test_comb_function_array_literal_accepts_array_returning_items(sim) { } fn test_comb_function_direct_array_argument_converts_each_element(sim) { + // sv: Veryl emits a call that passes an unpacked array whose element type differs from the + // formal's, which IEEE 1800-2023 7.6 and 10.8 make illegal. The SV frontend rejects it + // ("argument 1 of `pick`: an unpacked array of type `logic signed [3:0] [2]` is not + // assignment compatible with `logic signed [7:0] [2]`") instead of reinterpreting its bits. @ignore_on(veryl, sv); @case "comb_observer::test_comb_function_direct_array_argument_converts_each_element"; } @@ -2607,6 +2615,10 @@ fn test_comb_function_direct_array_return_preserves_all_elements(sim) { } fn test_comb_statement_function_direct_array_argument_converts_each_element(sim) { + // sv: Veryl emits a call that passes an unpacked array whose element type differs from the + // formal's, which IEEE 1800-2023 7.6 and 10.8 make illegal. The SV frontend rejects it + // ("argument 1 of `capture`: an unpacked array of type `logic signed [3:0] [2]` is not + // assignment compatible with `logic signed [7:0] [2]`") instead of reinterpreting its bits. @ignore_on(veryl, sv); @case "comb_observer::test_comb_statement_function_direct_array_argument_converts_each_element"; } diff --git a/crates/celox/tests/flip_flop.rs b/crates/celox/tests/flip_flop.rs index 5bdb8f2362..6846aa2ac2 100644 --- a/crates/celox/tests/flip_flop.rs +++ b/crates/celox/tests/flip_flop.rs @@ -4254,11 +4254,19 @@ fn test_ff_function_call_snapshots_pure_array_items_before_later_effect(sim) { } fn test_ff_function_call_converts_array_literal_view_for_wider_nested_formal(sim) { + // sv: Veryl emits a call that passes an unpacked array whose element type differs from the + // formal's, which IEEE 1800-2023 7.6 and 10.8 make illegal. The SV frontend rejects it + // ("argument 1 of `inner`: an unpacked array of type `logic [3:0] [2]` is not assignment + // compatible with `logic [7:0] [2]`") instead of reinterpreting its bits. @ignore_on(sv); @case "flip_flop::test_ff_function_call_converts_array_literal_view_for_wider_nested_formal"; } fn test_ff_function_call_converts_forwarded_static_array_element(sim) { + // sv: Veryl emits a call that passes an unpacked array whose element type differs from the + // formal's, which IEEE 1800-2023 7.6 and 10.8 make illegal. The SV frontend rejects it + // ("argument 1 of `inner`: an unpacked array of type `logic [3:0] [2]` is not assignment + // compatible with `logic [7:0] [2]`") instead of reinterpreting its bits. @ignore_on(sv); @case "flip_flop::test_ff_function_call_converts_forwarded_static_array_element"; } @@ -4285,6 +4293,10 @@ fn test_ff_function_call_multidim_array_literal_indexing_preserves_element_order } fn test_ff_function_call_dynamic_multidim_indexing_accepts_array_valued_items(sim) { + // sv: Veryl emits a call that passes an unpacked array whose element type differs from the + // formal's, which IEEE 1800-2023 7.6 and 10.8 make illegal. The SV frontend rejects it + // ("assignment pattern item: an unpacked array of type `logic [3:0] [2]` is not assignment + // compatible with `logic [7:0] [2]`") instead of reinterpreting its bits. @ignore_on(veryl, sv); @case "flip_flop::test_ff_function_call_dynamic_multidim_indexing_accepts_array_valued_items"; } diff --git a/crates/celox/tests/frontends/systemverilog/indexed_select.rs b/crates/celox/tests/frontends/systemverilog/indexed_select.rs index 29a3d34dc6..c30217e57c 100644 --- a/crates/celox/tests/frontends/systemverilog/indexed_select.rs +++ b/crates/celox/tests/frontends/systemverilog/indexed_select.rs @@ -19,6 +19,10 @@ sv_backends! { @case "indexed_select::out_of_range_inner_indices_stay_in_their_element"; } + fn runtime_index_reads_single_bit_unpacked_elements(sim) { + @case "indexed_select::runtime_index_reads_single_bit_unpacked_elements"; + } + fn indexed_select_parameterized_generate_and_ff(sim) { @case "indexed_select::indexed_select_parameterized_generate_and_ff"; } diff --git a/crates/celox/tests/frontends/systemverilog/review_regressions.rs b/crates/celox/tests/frontends/systemverilog/review_regressions.rs index 1c077c4e2e..feba7a1b16 100644 --- a/crates/celox/tests/frontends/systemverilog/review_regressions.rs +++ b/crates/celox/tests/frontends/systemverilog/review_regressions.rs @@ -4114,11 +4114,14 @@ fn preserves_masked_parameter_guards_before_latch_detection() { #[test] fn coerces_whole_unpacked_array_writes_to_the_flattened_width() { + // The element types are equivalent, as an unpacked array assignment + // requires (IEEE 1800-2023 6.22.2, 7.6); signed elements must not + // extend into the neighboring element. let source = r#" module Top( input logic c, input logic signed [7:0] a[2], b[2], - output logic [7:0] x[2] + output logic signed [7:0] x[2] ); always_comb begin if (c) @@ -8085,3 +8088,124 @@ fn run_time_bound_loop_return_reports_unsupported_on_a_small_stack() { "{error}" ); } + +#[test] +fn reports_incompatible_unpacked_arrays_in_each_assignment_like_context() { + // An unpacked array needs an equivalent element type and equal element + // counts wherever it is assigned (IEEE 1800-2023 7.6, 10.8); the + // diagnostic names the context and both array types. + let cases = [ + ( + r#" + module Top(output logic signed [7:0] q); + function automatic logic signed [7:0] pick(input logic signed [7:0] x [2]); + return x[0]; + endfunction + logic signed [3:0] narrow [2]; + always_comb begin + narrow[0] = 4'sh8; + narrow[1] = 4'sh1; + q = pick(narrow); + end + endmodule + "#, + "argument 1 of `pick`: an unpacked array of type `logic signed [3:0] [2]` is not \ + assignment compatible with `logic signed [7:0] [2]`", + ), + ( + r#" + module Top(output logic [3:0] q); + task automatic fill(output logic [7:0] x [2]); + x[0] = 8'h12; + x[1] = 8'h34; + endtask + logic [3:0] narrow [2]; + always_comb begin + fill(narrow); + q = narrow[0]; + end + endmodule + "#, + "argument 1 of `fill`: an unpacked array of type `logic [3:0] [2]` is not \ + assignment compatible with `logic [7:0] [2]`", + ), + ( + r#" + module Top(input logic a, output logic q); + function automatic void first(input logic x [2], output logic y); + y = x[0]; + endfunction + bit two [2]; + always_comb begin + two[0] = a; + two[1] = a; + first(.y(q), .x(two)); + end + endmodule + "#, + "argument 1 of `first`: an unpacked array of type `bit [2]` is not assignment \ + compatible with `logic [2]`", + ), + ( + r#" + module Top(input logic [7:0] a, output logic [7:0] q); + logic [7:0] grid [2][3]; + logic [7:0] rows [2][2]; + always_comb begin + grid = '{default: a}; + rows = grid; + q = rows[0][0]; + end + endmodule + "#, + "assignment: an unpacked array of type `logic [7:0] [2][3]` is not assignment \ + compatible with `logic [7:0] [2][2]`", + ), + ( + r#" + module Top(input logic [7:0] a, output logic [7:0] q); + logic [7:0] row [3]; + logic [7:0] grid [2][2]; + assign row[0] = a; + assign row[1] = a; + assign row[2] = a; + assign grid[1] = row; + assign grid[0] = '{a, a}; + assign q = grid[1][0]; + endmodule + "#, + "continuous assignment: an unpacked array of type `logic [7:0] [3]` is not \ + assignment compatible with `logic [7:0] [2]`", + ), + ( + r#" + module Child(output logic signed [7:0] y [2]); + assign y[0] = 8'sd1; + assign y[1] = -8'sd1; + endmodule + module Top(output logic [7:0] q); + logic [7:0] wide [2]; + Child u(.y(wide)); + assign q = wide[1]; + endmodule + "#, + "connection of port `y`: an unpacked array of type `logic [7:0] [2]` is not \ + assignment compatible with `logic signed [7:0] [2]`", + ), + ]; + for (source, expected) in cases { + let error = match Simulator::from_sv_sources(vec![(source, Path::new("review.sv"))], "Top") + .build_cranelift() + { + Ok(_) => panic!("incompatible unpacked arrays unexpectedly compiled:\n{source}"), + Err(error) => error, + }; + match error.kind() { + celox::SimulatorErrorKind::SIRParser(celox::ParserError::IllegalContext { + detail, + .. + }) => assert_eq!(detail, expected), + _ => panic!("expected an illegal-context error, got {error:?}"), + } + } +} diff --git a/crates/celox/tests/frontends/systemverilog/types.rs b/crates/celox/tests/frontends/systemverilog/types.rs index 903a2ea197..6286cab494 100644 --- a/crates/celox/tests/frontends/systemverilog/types.rs +++ b/crates/celox/tests/frontends/systemverilog/types.rs @@ -1,4 +1,32 @@ sv_backends! { + fn passes_unpacked_arrays_of_equivalent_element_types(sim) { + @case "types::passes_unpacked_arrays_of_equivalent_element_types"; + } + + fn passes_unpacked_arguments_of_equivalent_element_types(sim) { + @case "types::passes_unpacked_arguments_of_equivalent_element_types"; + } + + fn rejects_unpacked_argument_of_a_wider_element_type(sim) { + @case "types::rejects_unpacked_argument_of_a_wider_element_type"; + } + + fn rejects_unpacked_assignment_of_a_different_state_count(sim) { + @case "types::rejects_unpacked_assignment_of_a_different_state_count"; + } + + fn rejects_unpacked_assignment_of_a_different_element_count(sim) { + @case "types::rejects_unpacked_assignment_of_a_different_element_count"; + } + + fn rejects_unpacked_port_connection_of_a_narrower_element_type(sim) { + @case "types::rejects_unpacked_port_connection_of_a_narrower_element_type"; + } + + fn rejects_assignment_pattern_item_of_a_narrower_array_type(sim) { + @case "types::rejects_assignment_pattern_item_of_a_narrower_array_type"; + } + fn packed_arrays_of_signed_named_types_have_signed_elements(sim) { @case "types::packed_arrays_of_signed_named_types_have_signed_elements"; } diff --git a/crates/celox/tests/nba_dynamic_array.rs b/crates/celox/tests/nba_dynamic_array.rs index f48253b9cb..043d6e9669 100644 --- a/crates/celox/tests/nba_dynamic_array.rs +++ b/crates/celox/tests/nba_dynamic_array.rs @@ -5,7 +5,6 @@ mod test_utils; all_backends! { fn test_subbyte_arithmetic_padding_does_not_corrupt_concat(sim) { - @ignore_on(sv); @case "nba_dynamic_array::test_subbyte_arithmetic_padding_does_not_corrupt_concat"; } @@ -46,7 +45,6 @@ all_backends! { } fn test_dynamic_ff_array_partial_squash_preserves_head_and_branch(sim) { - @ignore_on(sv); @case "nba_dynamic_array::test_dynamic_ff_array_partial_squash_preserves_head_and_branch"; } From c7da38f4d00993ac4f88579bdb7c905376f2b8df Mon Sep 17 00:00:00 2001 From: tignear Date: Wed, 7 Oct 2026 08:47:52 +0900 Subject: [PATCH 3/5] test: record why each SystemVerilog arm is ignored and drop stale ignores Every remaining `@ignore_on(...)` that covers the SV arm now states the observed failure. Eleven SV arms that already pass are no longer ignored. --- crates/celox/tests/advanced_interface.rs | 9 +++++ crates/celox/tests/array_literal.rs | 8 ++++ crates/celox/tests/basic.rs | 4 ++ crates/celox/tests/checkpoint.rs | 3 +- crates/celox/tests/comb_observer.rs | 39 ++++++++++++++++++- crates/celox/tests/concurrent_initial.rs | 10 +++++ crates/celox/tests/duplicate_varpath.rs | 4 ++ crates/celox/tests/expression_semantics.rs | 1 - crates/celox/tests/flip_flop.rs | 10 +++++ crates/celox/tests/function_arguments.rs | 2 + crates/celox/tests/function_bodies.rs | 2 + crates/celox/tests/hierarchical_assignment.rs | 6 +++ crates/celox/tests/hierarchical_readmem.rs | 7 ++++ crates/celox/tests/hierarchy.rs | 18 +++++++++ crates/celox/tests/interface.rs | 2 + crates/celox/tests/nba_dynamic_array.rs | 26 ++++++------- crates/celox/tests/state_cast_semantics.rs | 2 + crates/celox/tests/synth_dynamic_loop.rs | 2 + .../celox/tests/veryl_context_regressions.rs | 4 +- crates/celox/tests/veryl_upstream.rs | 6 ++- 20 files changed, 142 insertions(+), 23 deletions(-) diff --git a/crates/celox/tests/advanced_interface.rs b/crates/celox/tests/advanced_interface.rs index d83b6de648..582222a9dc 100644 --- a/crates/celox/tests/advanced_interface.rs +++ b/crates/celox/tests/advanced_interface.rs @@ -8,18 +8,24 @@ all_backends! { // Interface with multiple modport signals and bidirectional data flow. fn test_interface_bidirectional(sim) { + // SV frontend does not support interfaces: module instantiation of interface + // `Handshake` (#88). @ignore_on(sv); @case "advanced_interface::test_interface_bidirectional"; } // Multiple interface instances used in parallel. fn test_multiple_interface_instances(sim) { + // SV frontend does not support interfaces: module instantiation of interface `DataBus` + // (#88). @ignore_on(sv); @case "advanced_interface::test_multiple_interface_instances"; } // Interface with wide (multi-bit) signals. fn test_interface_wide_signal(sim) { + // SV frontend does not support interfaces: module instantiation of interface `WideBus` + // (#88). @ignore_on(sv); @case "advanced_interface::test_interface_wide_signal"; } @@ -27,6 +33,8 @@ all_backends! { // Parametric interface array: verify array_dims are populated for parametric-type members. fn test_parametric_interface_array(sim) { @omit_veryl; + // SV frontend does not support interfaces: "unsupported port data type" for the + // interface port (#88). @ignore_on(sv); @setup { let code = r#" interface Bus:: { @@ -78,6 +86,7 @@ assign out = bus[0].data + bus[1].data; // Tests that generic type parameters are correctly propagated across multiple // levels of the module hierarchy (a pattern that has been buggy in the past). fn test_transitive_generics(sim) { + // SV frontend does not support interfaces: "structure member `a_bus.data`" (#88). @ignore_on(sv); @case "advanced_interface::test_transitive_generics"; } diff --git a/crates/celox/tests/array_literal.rs b/crates/celox/tests/array_literal.rs index c805be6234..348db93d88 100644 --- a/crates/celox/tests/array_literal.rs +++ b/crates/celox/tests/array_literal.rs @@ -9,11 +9,15 @@ all_backends! { } fn test_array_literal_default_comb_assignment(sim) { + // Veryl emits `'{8'h12, default: 8'hAA}`, mixing positional items with `default:`, + // which is not legal SV; the SV parser rejects it. @ignore_on(sv); @case "array_literal::test_array_literal_default_comb_assignment"; } fn test_array_literal_nested_default_multidim_assignment(sim) { + // Veryl emits `'{8'h11, default: 8'h22}`, mixing positional items with `default:`, + // which is not legal SV; the SV parser rejects it. @ignore_on(sv); @case "array_literal::test_array_literal_nested_default_multidim_assignment"; } @@ -38,12 +42,16 @@ all_backends! { // Two explicit elements + default: remaining slots filled correctly. fn test_array_literal_two_explicit_plus_default(sim) { + // Veryl emits `'{a, b, default: c}`, mixing positional items with `default:`, which is + // not legal SV; the SV parser rejects it. @ignore_on(sv); @case "array_literal::test_array_literal_two_explicit_plus_default"; } // repeat + default: '{val repeat 2, default: 0} in a size-4 array. fn test_array_literal_repeat_plus_default(sim) { + // Veryl emits `'{2{a}, default: b}`, mixing positional items with `default:`, which is + // not legal SV; the SV parser rejects it. @ignore_on(sv); @case "array_literal::test_array_literal_repeat_plus_default"; } diff --git a/crates/celox/tests/basic.rs b/crates/celox/tests/basic.rs index 0c17a931a5..7292b20488 100644 --- a/crates/celox/tests/basic.rs +++ b/crates/celox/tests/basic.rs @@ -212,11 +212,15 @@ module Top ( } fn test_comb_function_call_with_output_argument_in_index_expression(sim) { + // SV frontend rejects a function call in a select index: + // "select index `f(sel, q_output)`" (#88). @ignore_on(sv); @case "basic::test_comb_function_call_with_output_argument_in_index_expression"; } fn test_comb_function_call_with_output_argument_in_destination_index(sim) { + // SV frontend rejects a destination index that calls a function with an output + // argument: "assignment target `data`" (#88). @ignore_on(sv); @case "basic::test_comb_function_call_with_output_argument_in_destination_index"; } diff --git a/crates/celox/tests/checkpoint.rs b/crates/celox/tests/checkpoint.rs index 9881f038b2..0cbc1a0326 100644 --- a/crates/celox/tests/checkpoint.rs +++ b/crates/celox/tests/checkpoint.rs @@ -67,6 +67,7 @@ macro_rules! outputs_now { all_backends! { fn restore_replays_the_same_cycles(sim) { @omit_veryl; + // SV frontend rejects a part-select used as an index: "select index `c[1:0]`" (#88). @ignore_on(sv); @build Simulator::builder(DESIGN, "Top"); run_cycles!(sim, 0..5); @@ -82,7 +83,6 @@ all_backends! { fn restore_keeps_four_state_values(sim) { @omit_veryl; - @ignore_on(sv); @build Simulator::builder( r#" module Top (clk: input clock, d: input logic<8>, q: output logic<8>) { @@ -106,7 +106,6 @@ all_backends! { fn restore_replays_runtime_events(sim) { @omit_veryl; - @ignore_on(sv); @build Simulator::builder( r#" module Top (clk: input clock, a: input logic<8>, q: output logic<8>) { diff --git a/crates/celox/tests/comb_observer.rs b/crates/celox/tests/comb_observer.rs index 7b6d2554aa..7f07b59fbc 100644 --- a/crates/celox/tests/comb_observer.rs +++ b/crates/celox/tests/comb_observer.rs @@ -162,6 +162,8 @@ module Top ( fn test_comb_callee_observer_snapshots_formal_before_later_actual_writeback(sim) { @omit_veryl; + // SV frontend applies `inner`'s output writeback before reading the earlier argument + // (`first=11`, expected `first=1`); SV leaves this order to the implementation. @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -278,6 +280,8 @@ module Top ( fn test_comb_output_destination_observer_uses_return_aware_loop_value(sim) { @omit_veryl; + // SV frontend rejects a function call in a select index: + // "select index `observe_index(tmp)`" (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -374,7 +378,6 @@ module Top ( fn test_comb_condition_runtime_effect_respects_short_circuit(sim) { @omit_veryl; - @ignore_on(sv); @build Simulator::builder(r#" module Top ( and_enable: input logic, @@ -470,6 +473,8 @@ module Top ( fn test_comb_runtime_effect_inside_assignment_destination_is_collected(sim) { @omit_veryl; + // SV frontend rejects a destination index that calls a function with an output argument: + // "assignment target `data`" (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -580,6 +585,7 @@ module Top ( fn test_comb_runtime_effect_inside_loop_bound_is_collected(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -793,6 +799,8 @@ module Top { } fn test_veryl_adapter_modify_rechecks_constant_initial_fatal(sim) { + // Only the Veryl reference adapter evaluates on `modify`; Celox backends, the SV arm + // included, defer evaluation, so `modify` returns Ok. @ignore_on(native, cranelift, wasm, interp, sv); @build Simulator::builder(r#" module Top ( @@ -1767,6 +1775,7 @@ module Top ( fn test_comb_display_inside_dynamic_for_reactivates_after_assign_chain(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -2081,6 +2090,7 @@ module Top ( fn test_comb_capture_before_dynamic_for_backedge_keeps_loop_state(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -2406,6 +2416,7 @@ module Top ( fn test_return_before_dynamic_loop_suppresses_loop_effects(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -2569,6 +2580,8 @@ module Top ( } fn test_comb_function_packed_array_literal_preserves_source_order(sim) { + // SV arm: Veryl emits `'{default: a, b}`, mixing positional items with `default:`, which is + // not legal SV; the SV parser rejects it. @ignore_on(veryl, sv); @case "comb_observer::test_comb_function_packed_array_literal_preserves_source_order"; } @@ -2646,17 +2659,20 @@ module Top ( } fn test_named_function_inputs_evaluate_in_source_order(sim) { + // SV frontend rejects named function arguments: "named arguments of `add`" (#88). @ignore_on(sv); @case "comb_observer::test_named_function_inputs_evaluate_in_source_order"; } fn test_named_function_outputs_apply_in_source_order(sim) { + // SV frontend rejects named function arguments: "named arguments of `write_outputs`" (#88). @ignore_on(sv); @case "comb_observer::test_named_function_outputs_apply_in_source_order"; } fn test_nested_dynamic_function_loops_preserve_effect_runners(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -3474,6 +3490,7 @@ module Top ( fn test_comb_display_inside_dynamic_for_runs_each_iteration(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -3527,6 +3544,7 @@ module Top ( fn test_comb_display_inside_dynamic_for_remaps_site_after_prior_comb_event(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -3587,6 +3605,7 @@ module Top ( fn test_comb_display_inside_dynamic_for_preserves_repeated_identical_events(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -3624,6 +3643,7 @@ module Top ( fn test_comb_display_inside_dynamic_for_with_multiple_updates_emits_once_per_iteration(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -3678,6 +3698,7 @@ module Top ( fn test_comb_display_preserves_order_around_dynamic_for(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -4121,6 +4142,8 @@ module Top ( fn test_comb_display_function_output_dynamic_actual_excludes_only_prefix(sim) { @omit_veryl; + // SV frontend rejects a run-time select `mem[1][idx]` as an output argument: + // "assignment target `mem`" (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -4350,6 +4373,8 @@ module Top (a: input logic<8>, out: output logic<8>) { fn test_comb_runtime_effect_in_function_output_destination_is_detected(sim) { @omit_veryl; + // SV frontend rejects a function call in a select index: + // "select index `choose_index(index, seen)`" (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top (index: input logic<2>, value: input logic<8>, out: output logic<8>) { @@ -4445,6 +4470,8 @@ fn test_comb_function_loop_skips_conditions_after_break(sim) { fn test_comb_function_output_preview_honors_loop_break(sim) { @omit_veryl; + // SV frontend rejects a function call in a select index: "select index `observe(first)`" + // (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -4498,6 +4525,8 @@ module Top ( fn test_comb_outputless_function_output_preview_honors_loop_break(sim) { @omit_veryl; + // SV frontend rejects a function call in a select index: "select index `observe(first)`" + // (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -4549,6 +4578,7 @@ module Top ( fn test_comb_return_aware_function_loop_collects_bound_effects(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top (value: input logic<8>, out: output logic<8>) { @@ -4587,6 +4617,8 @@ module Top (value: input logic<8>, out: output logic<8>) { fn test_comb_variable_indices_observe_prior_index_output_write(sim) { @omit_veryl; + // SV frontend rejects a function call in a select index: + // "select index `set_index(value, seen)`" (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top (value: input logic, out: output logic) { @@ -4621,6 +4653,7 @@ module Top (value: input logic, out: output logic) { fn test_comb_function_loop_bound_write_is_guarded_after_return(sim) { @omit_veryl; + // SV frontend rejects `$display` inside a combinational loop with a run-time bound (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( @@ -4670,6 +4703,8 @@ module Top ( fn test_comb_concat_destination_observes_prior_destination_write(sim) { @omit_veryl; + // SV frontend rejects `{data[observe(tmp)], tmp} = ...`, whose index calls a function: + // "assignment target `data`" (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top (trigger: input logic, out: output logic<2>) { @@ -4697,6 +4732,8 @@ module Top (trigger: input logic, out: output logic<2>) { fn test_comb_function_output_concat_observes_prior_destination_write(sim) { @omit_veryl; + // SV frontend rejects a function call in a select index: "select index `observe(tmp)`" + // (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top (trigger: input logic, out: output logic<2>) { diff --git a/crates/celox/tests/concurrent_initial.rs b/crates/celox/tests/concurrent_initial.rs index 7af9d3af7e..4dd361654e 100644 --- a/crates/celox/tests/concurrent_initial.rs +++ b/crates/celox/tests/concurrent_initial.rs @@ -6,51 +6,61 @@ mod test_utils; all_backends! { fn concurrent_initial_shared_clock(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "concurrent_initial::shared_clock"; } fn concurrent_initial_control_flow(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "concurrent_initial::control_flow"; } fn concurrent_initial_clock_periods(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "concurrent_initial::clock_periods"; } fn concurrent_initial_reset_only_clock(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "concurrent_initial::reset_only_clock"; } fn concurrent_initial_simultaneous_edges(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "concurrent_initial::simultaneous_edges"; } fn concurrent_initial_reset(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "concurrent_initial::reset"; } fn concurrent_initial_reset_between_edges(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "concurrent_initial::reset_between_edges"; } fn concurrent_initial_hierarchy(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "concurrent_initial::hierarchy"; } fn concurrent_initial_mixed_edges(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "concurrent_initial::mixed_edges"; } fn concurrent_initial_gated_drive(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "concurrent_initial::gated_drive"; } diff --git a/crates/celox/tests/duplicate_varpath.rs b/crates/celox/tests/duplicate_varpath.rs index ab269ec980..561f740acf 100644 --- a/crates/celox/tests/duplicate_varpath.rs +++ b/crates/celox/tests/duplicate_varpath.rs @@ -20,6 +20,8 @@ all_backends! { // The Veryl analyzer assigns different VarIds but identical VarPaths to these scoped // variables. Without the fix this panics during JIT compilation. fn test_duplicate_scoped_var_in_always_comb(sim) { + // SV frontend rejects same-named block-local variables in two processes: + // "duplicate internal signal `tmp`" (#445). @ignore_on(sv); @case "duplicate_varpath::test_duplicate_scoped_var_in_always_comb"; } @@ -29,6 +31,8 @@ fn test_duplicate_scoped_var_in_always_comb(sim) { // internal vars, and always_comb inside uses `var flag: logic;` in multiple // for-loop scopes. fn test_duplicate_scoped_var_with_generate_for(sim) { + // SV frontend rejects same-named block-local variables in two processes: + // "duplicate internal signal `flag`" (#445). @ignore_on(sv); @case "duplicate_varpath::test_duplicate_scoped_var_with_generate_for"; } diff --git a/crates/celox/tests/expression_semantics.rs b/crates/celox/tests/expression_semantics.rs index 36974c5234..83c97eb58f 100644 --- a/crates/celox/tests/expression_semantics.rs +++ b/crates/celox/tests/expression_semantics.rs @@ -8,7 +8,6 @@ all_backends! { } fn parent_context_and_self_determined_boundaries_match_between_comb_and_ff(sim) { - @ignore_on(sv); @case "expression_semantics::parent_context_and_self_determined_boundaries_match_between_comb_and_ff"; } diff --git a/crates/celox/tests/flip_flop.rs b/crates/celox/tests/flip_flop.rs index 6846aa2ac2..6d38936e14 100644 --- a/crates/celox/tests/flip_flop.rs +++ b/crates/celox/tests/flip_flop.rs @@ -70,6 +70,8 @@ fn test_ff_assert_message_output_argument_is_eager(sim) { fn test_ff_assert_message_runtime_effect_is_eager(sim) { @omit_veryl; + // SV frontend evaluates `$assert_continue` message arguments only when the check fails, so + // the `$display` in `message_value` is missing; Veryl evaluates them eagerly. @ignore_on(sv); @setup { let code = r#" module Top (clk: input clock, ok: input logic, d: input logic<8>) { @@ -327,6 +329,8 @@ fn test_ff_runtime_effect_function_snapshots_input_that_aliases_output(sim) { fn test_ff_case_pattern_runtime_effect_is_eager(sim) { @omit_veryl; + // SV frontend evaluates `$assert_continue` message arguments only when the check fails, so + // the `$display` in the case pattern is missing; Veryl evaluates them eagerly. @ignore_on(sv); @setup { let code = r#" module Top (clk: input clock, ok: input logic, d: input logic<8>) { @@ -3693,6 +3697,8 @@ fn test_ff_i32_or_step_with_only_existing_low_bits_reports_true_loop(sim) { } fn test_ff_i32_mul_step_overflow_reports_true_loop(sim) { + // SV arm: in the emitted `for (int i = start; ...; i *= 2)` the `int` wraps and the loop + // ends, so no non-progressing-loop error is reported. @ignore_on(veryl, sv); @setup { let code = r#" module Top ( @@ -4056,6 +4062,8 @@ fn test_ff_function_call_array_literal_element_uses_formal_context_width(sim) { } fn test_ff_function_call_array_literal_supports_dynamic_multidim_indexing(sim) { + // Veryl emits `'{1{'{...}}, default: '{...}}`, mixing positional items with `default:`, + // which is not legal SV; the SV parser rejects it. @ignore_on(sv); @case "flip_flop::test_ff_function_call_array_literal_supports_dynamic_multidim_indexing"; } @@ -4096,6 +4104,8 @@ fn test_ff_function_call_array_literal_snapshots_scalar_before_callee_write(sim) fn test_ff_case_range_skips_effectful_upper_bound_when_lower_is_false(sim) { @omit_veryl; + // Veryl emits `case (target) inside [8'd5:(observed_upper())-1]`, which evaluates both + // bounds, so `upper` is displayed even when the lower bound fails. @ignore_on(sv); @setup { let code = r#" module Top ( diff --git a/crates/celox/tests/function_arguments.rs b/crates/celox/tests/function_arguments.rs index 19c6eb1f12..78efe1dde8 100644 --- a/crates/celox/tests/function_arguments.rs +++ b/crates/celox/tests/function_arguments.rs @@ -77,6 +77,8 @@ fn test_comb_expression_output_copyout_uses_unsigned_formal_for_signed_body(sim) fn test_comb_output_copyout_observer_sees_formal_sign_extension(sim) { @omit_veryl; + // SV frontend rejects a function call in a select index: "select index `observe(copied)`" + // (#88). @ignore_on(sv); @build Simulator::builder(r#" module Top ( diff --git a/crates/celox/tests/function_bodies.rs b/crates/celox/tests/function_bodies.rs index 118c03482a..0d81b1d342 100644 --- a/crates/celox/tests/function_bodies.rs +++ b/crates/celox/tests/function_bodies.rs @@ -38,6 +38,8 @@ fn test_runtime_bounded_loop_in_function(sim) { } fn test_runtime_effects_nested_in_function_statements(sim) { + // SV frontend rejects a destination index that calls a function with `$display` + // (`r[traced(idx)]`): "assignment target `r@7`" (#88). @ignore_on(sv); @case "function_bodies::test_runtime_effects_nested_in_function_statements"; } diff --git a/crates/celox/tests/hierarchical_assignment.rs b/crates/celox/tests/hierarchical_assignment.rs index d6a64b3be6..c507250ccc 100644 --- a/crates/celox/tests/hierarchical_assignment.rs +++ b/crates/celox/tests/hierarchical_assignment.rs @@ -10,6 +10,7 @@ all_backends! { fn hierarchical_assignment_settles_logic_and_clocked_consumers(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let code = r#" @@ -54,12 +55,14 @@ fn hierarchical_assignment_settles_logic_and_clocked_consumers(sim) { fn hierarchical_assignment_resolves_nested_instances_and_dynamic_selections(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "hierarchical_assignment::selections"; } fn hierarchical_assignment_preserves_unselected_four_state_bits(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let code = r#" @@ -90,6 +93,7 @@ fn hierarchical_assignment_preserves_unselected_four_state_bits(sim) { fn hierarchical_assignment_uses_function_result(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let code = r#" @@ -124,12 +128,14 @@ all_backends! { fn hierarchical_assignment_to_disjoint_loop_state(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @case "hierarchical_assignment::disjoint_loop"; } fn hierarchical_assignment_updates_register_state(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let code = r#" diff --git a/crates/celox/tests/hierarchical_readmem.rs b/crates/celox/tests/hierarchical_readmem.rs index d7ecfaf2be..4c5f8cbfe3 100644 --- a/crates/celox/tests/hierarchical_readmem.rs +++ b/crates/celox/tests/hierarchical_readmem.rs @@ -17,6 +17,7 @@ all_backends! { fn hierarchical_readmemh_runs_in_statement_order_across_branches_and_clock_edges(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let (_first_directory, first) = memory_file("2a\n11\n"); @@ -62,6 +63,7 @@ fn hierarchical_readmemh_runs_in_statement_order_across_branches_and_clock_edges fn hierarchical_readmemh_initializes_child_and_settles_comb(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let (_directory, path) = memory_file("1\n2\n3\n4\n"); @@ -93,6 +95,7 @@ fn hierarchical_readmemh_initializes_child_and_settles_comb(sim) { fn hierarchical_readmemh_resolves_nested_parameterized_memories(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let expected = (BigUint::from(1u8) << 64) | BigUint::from(0x1abu32); @@ -131,6 +134,7 @@ fn hierarchical_readmemh_resolves_nested_parameterized_memories(sim) { fn hierarchical_readmemh_uses_each_calling_instance_and_preserves_write_order(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let (_first_directory, first) = memory_file("11\n22\n"); @@ -173,6 +177,7 @@ fn hierarchical_readmemh_uses_each_calling_instance_and_preserves_write_order(si fn hierarchical_readmemh_preserves_four_state_and_sparse_addresses(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let (_directory, path) = memory_file("/* skip first element */ @1\nx5\nz0\n"); @@ -203,6 +208,7 @@ fn hierarchical_readmemh_preserves_four_state_and_sparse_addresses(sim) { fn hierarchical_readmemh_honors_constant_branches_and_loops(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let (_directory, path) = memory_file("ab\n"); @@ -240,6 +246,7 @@ fn hierarchical_readmemh_honors_constant_branches_and_loops(sim) { fn hierarchical_readmemh_flattens_multidimensional_arrays(sim) { @omit_veryl; + // Veryl does not emit `#[test]` testbench modules, so the emitted SV has no `Top`. @ignore_on(sv); @setup { let (_directory, path) = memory_file("11\n22\n33\n44\n"); diff --git a/crates/celox/tests/hierarchy.rs b/crates/celox/tests/hierarchy.rs index 4426528991..7a06108904 100644 --- a/crates/celox/tests/hierarchy.rs +++ b/crates/celox/tests/hierarchy.rs @@ -10,6 +10,8 @@ all_backends! { // and child_signal access works. fn test_for_loop_instance_hierarchy(sim) { @omit_veryl; + // On the SV path `named_hierarchy()` names generate-block instances `g[0].u_sub` and + // `g[1].u_sub`, giving two child groups instead of one `u_sub` group. @ignore_on(sv); @setup { let code = r#" module Sub ( @@ -76,16 +78,25 @@ o_data: top_out[i], } fn test_instance_input_function_output_writeback(sim) { + // Veryl emits a call with an output argument in an input port connection, which IEEE + // 1800 13.4 forbids outside procedural code; the SV frontend rejects it + // ("systemverilog input port connection", #88). @ignore_on(sv); @case "hierarchy::test_instance_input_function_output_writeback"; } fn test_instance_input_function_output_concat_dynamic_writeback(sim) { + // SV arm: Veryl emits a call with an output argument in an input port connection, which + // IEEE 1800 13.4 forbids outside procedural code; the SV frontend rejects it + // ("systemverilog input port connection", #88). @ignore_on(veryl, sv); @case "hierarchy::test_instance_input_function_output_concat_dynamic_writeback"; } fn test_inactive_instance_input_output_call_adds_no_parent_driver(sim) { + // Veryl emits a call with an output argument in an input port connection, which IEEE 1800 + // 13.4 forbids outside procedural code; the SV frontend rejects it + // ("systemverilog input port connection", #88). @ignore_on(sv); @case "hierarchy::test_inactive_instance_input_output_call_adds_no_parent_driver"; } @@ -94,6 +105,9 @@ fn test_inactive_instance_input_output_call_adds_no_parent_driver(sim) { // veryl-simulator does not write the connection's function output // actual back. @omit_veryl; + // Veryl emits a call with an output argument in an input port connection, which IEEE + // 1800 13.4 forbids outside procedural code; the SV frontend rejects it + // ("systemverilog input port connection", #88). @ignore_on(sv); @setup { let code = r#" module Child ( @@ -142,6 +156,8 @@ assign seen_o = seen; } fn test_instance_output_dynamic_index_function_output_writeback(sim) { + // SV arm: the case expects a language rejection, but the SV frontend reports + // Unsupported "select index `choose_index(sel, tmp)`" (#88) instead. @ignore_on(veryl, sv); @case "hierarchy::test_instance_output_dynamic_index_function_output_writeback"; } @@ -151,6 +167,8 @@ assign seen_o = seen; fn test_instance_output_dynamic_index_composes_aliasing_writeback(sim) { + // SV arm: the case expects a language rejection, but the SV frontend reports + // Unsupported "select index `choose_index(sel, mem[0])`" (#88) instead. @ignore_on(veryl, sv); @case "hierarchy::test_instance_output_dynamic_index_composes_aliasing_writeback"; } diff --git a/crates/celox/tests/interface.rs b/crates/celox/tests/interface.rs index 0332212a2b..a9495b8010 100644 --- a/crates/celox/tests/interface.rs +++ b/crates/celox/tests/interface.rs @@ -7,6 +7,8 @@ mod test_utils; // instantiation [tracking issue #88]` for the emitted interface instance). all_backends! { fn test_interface_connection(sim) { + // SV frontend does not support interfaces: module instantiation of interface `Bus` + // (#88). @ignore_on(sv); @case "interface::test_interface_connection"; } diff --git a/crates/celox/tests/nba_dynamic_array.rs b/crates/celox/tests/nba_dynamic_array.rs index 043d6e9669..847e5aceda 100644 --- a/crates/celox/tests/nba_dynamic_array.rs +++ b/crates/celox/tests/nba_dynamic_array.rs @@ -61,17 +61,16 @@ all_backends! { } // The Veryl reference simulator writes through an unknown index instead - // of ignoring the write (IEEE 1800-2023 7.4.6). The SV frontend rejects - // four-state always_ff event signals. + // of ignoring the write (IEEE 1800-2023 7.4.6). fn test_out_of_range_dynamic_ff_access_four_state(sim) { - @ignore_on(veryl, sv); + @ignore_on(veryl); @case "nba_dynamic_array::test_out_of_range_dynamic_ff_access_four_state"; } // The Veryl reference simulator reads an existing element through an - // invalid index; the SV frontend rejects the emitted design. + // invalid index. fn test_out_of_range_dynamic_comb_access(sim) { - @ignore_on(veryl, sv); + @ignore_on(veryl); @case "nba_dynamic_array::test_out_of_range_dynamic_comb_access"; } @@ -215,10 +214,9 @@ all_backends! { // and keep the in-range bits of a `-:` part select that starts below bit 0 // (IEEE 1800-2023 7.4.6, 11.5.1). In a two-state simulation an invalid read, // like any X, reads 0. The Veryl reference simulator applies `grid[0][3]` to -// another element, and the SV frontend rejects a dynamic unpacked-array write -// after a partial write to the same array. +// another element. fn test_out_of_range_dynamic_comb_access_two_state(sim) { - @ignore_on(veryl, sv); + @ignore_on(veryl); @setup { let source = r#" module Top ( @@ -308,10 +306,8 @@ fn test_out_of_range_dynamic_comb_access_two_state(sim) { all_backends! { // A loop variable's known range removes only the checks it proves // unnecessary: `src[i + off]` can still leave the array when `off` is large, -// while `dst[i * 8 +: 8]` in `0..4` cannot. The SV frontend rejects the -// emitted accumulating `always_comb` assignment. +// while `dst[i * 8 +: 8]` in `0..4` cannot. fn test_loop_variable_range_keeps_needed_checks(sim) { - @ignore_on(sv); @setup { let source = r#" module Top ( @@ -369,10 +365,10 @@ fn test_loop_variable_range_keeps_needed_checks(sim) { all_backends! { // Functions read their array arguments through the same checks, in // combinational and sequential code: an invalid index reads X. Icarus 13.0 -// and the SV frontend do not support unpacked-array function formals, and -// the Veryl reference simulator reads an existing element. +// does not support unpacked-array function formals, and the Veryl reference +// simulator reads an existing element. fn test_out_of_range_dynamic_read_in_function_is_unknown(sim) { - @ignore_on(veryl, sv); + @ignore_on(veryl); @setup { let source = r#" module Top ( @@ -525,7 +521,7 @@ all_backends! { // is an always_ff local or a function argument held in a register // (IEEE 1800-2023 7.4.6). fn test_invalid_reads_agree_across_paths_in_four_state(sim) { - @ignore_on(veryl, sv); + @ignore_on(veryl); @setup { let source = r#" module Top ( diff --git a/crates/celox/tests/state_cast_semantics.rs b/crates/celox/tests/state_cast_semantics.rs index 87db42c8e6..1be0481987 100644 --- a/crates/celox/tests/state_cast_semantics.rs +++ b/crates/celox/tests/state_cast_semantics.rs @@ -12,6 +12,8 @@ all_backends! { fn constant_type_cast_clears_unknown_bits(sim) { @omit_veryl; + // SV frontend rejects a cast of an X/Z-valued constant in a localparam initializer: + // "constant cast expression" (#88). @ignore_on(sv); @case "state_cast_semantics::constant_type_cast_clears_unknown_bits"; } diff --git a/crates/celox/tests/synth_dynamic_loop.rs b/crates/celox/tests/synth_dynamic_loop.rs index 85dc0f951a..f66a90d765 100644 --- a/crates/celox/tests/synth_dynamic_loop.rs +++ b/crates/celox/tests/synth_dynamic_loop.rs @@ -298,6 +298,8 @@ fn test_runtime_bounds_stalled_step_reports_true_loop(sim) { } fn test_runtime_bounds_preserve_loop_carried_state_for_indexed_reads(sim) { + // SV frontend rejects the loop-carried `x[i + 1] = x[i]` in a run-time-bounded loop: + // "combinational expression assigned to `x`" (#88). @ignore_on(sv); @case "synth_dynamic_loop::test_runtime_bounds_preserve_loop_carried_state_for_indexed_reads"; } diff --git a/crates/celox/tests/veryl_context_regressions.rs b/crates/celox/tests/veryl_context_regressions.rs index ceca335440..c83e0b24ca 100644 --- a/crates/celox/tests/veryl_context_regressions.rs +++ b/crates/celox/tests/veryl_context_regressions.rs @@ -16,7 +16,6 @@ all_backends! { @case "veryl_context_regressions::wide_logical_operand_keeps_result_type"; } fn constant_ternary_keeps_both_arm_types(sim) { - @ignore_on(sv); @case "veryl_context_regressions::constant_ternary_keeps_both_arm_types"; } fn signed_cast_of_folded_constant_sign_extends(sim) { @@ -53,7 +52,8 @@ all_backends! { @case "veryl_context_regressions::runtime_for_bound_arithmetic_uses_int_context"; } fn dynamic_param_array_read_uses_its_elements(sim) { - // SV frontend rejects the emitted unpacked array parameter as unsupported_simulator_parser. + // SV frontend rejects the unpacked-array parameter override `#(.Q('{-3, -5}))`: + // "parameter override expression" (#88). @ignore_on(sv); @case "veryl_context_regressions::dynamic_param_array_read_uses_its_elements"; } diff --git a/crates/celox/tests/veryl_upstream.rs b/crates/celox/tests/veryl_upstream.rs index 0f10dcae3d..77b36e18ff 100644 --- a/crates/celox/tests/veryl_upstream.rs +++ b/crates/celox/tests/veryl_upstream.rs @@ -19,7 +19,8 @@ all_backends! { @case "veryl_language::parameter_expression_type_cast_widths"; } fn packed_union_members_alias(sim) { - // SV frontend issue #64: unsupported packed struct/union types. + // SV frontend does not support packed unions: + // "unpacked struct, union, or unsupported packed struct member" (#440). @ignore_on(sv); @case "veryl_language::packed_union_members_alias"; } @@ -39,7 +40,8 @@ all_backends! { @case "veryl_regressions::wide_ternary_narrow_branch_no_spill"; } fn nested_array_index_const_array(sim) { - // SV frontend issue #64 rejects the constant-array assignment expression. + // SV frontend rejects `mem[A[idx]]` (index read from an unpacked-array localparam): + // "combinational expression assigned to `nested`" (#88). @ignore_on(sv); @case "veryl_regressions::nested_array_index_const_array"; } From 740f969402818f7d2552d0991d5c96247c456533 Mon Sep 17 00:00:00 2001 From: tignear Date: Thu, 8 Oct 2026 09:16:34 +0900 Subject: [PATCH 4/5] perf(sv-analyzer): share scope metadata and reuse parsed sources --- crates/celox-frontend-sv/src/lowering.rs | 59 ++-- crates/celox-sv-analyzer/examples/scaling.rs | 153 +++++++++ crates/celox-sv-analyzer/src/ast.rs | 101 ++++-- .../celox-sv-analyzer/src/ast/comb_process.rs | 16 +- crates/celox-sv-analyzer/src/ast/generate.rs | 16 +- crates/celox-sv-analyzer/src/ast/inlining.rs | 8 +- crates/celox-sv-analyzer/src/ast/instances.rs | 24 +- .../celox-sv-analyzer/src/ast/module_index.rs | 107 ++++++ .../celox-sv-analyzer/src/ast/procedural.rs | 6 +- .../celox-sv-analyzer/src/ast/scoped_map.rs | 141 ++++++++ .../celox-sv-analyzer/src/ast/statements.rs | 6 +- .../celox-sv-analyzer/src/ast/validation.rs | 2 +- crates/celox-sv-analyzer/src/lib.rs | 17 +- crates/celox-sv-analyzer/src/parsed.rs | 307 ++++++++++++++++++ docs/internals/sv-analysis-performance.md | 131 ++++++++ 15 files changed, 978 insertions(+), 116 deletions(-) create mode 100644 crates/celox-sv-analyzer/examples/scaling.rs create mode 100644 crates/celox-sv-analyzer/src/ast/module_index.rs create mode 100644 crates/celox-sv-analyzer/src/ast/scoped_map.rs create mode 100644 crates/celox-sv-analyzer/src/parsed.rs create mode 100644 docs/internals/sv-analysis-performance.md diff --git a/crates/celox-frontend-sv/src/lowering.rs b/crates/celox-frontend-sv/src/lowering.rs index cb413e8d3b..7052a5c101 100644 --- a/crates/celox-frontend-sv/src/lowering.rs +++ b/crates/celox-frontend-sv/src/lowering.rs @@ -7,7 +7,7 @@ //! dependency. use std::collections::BTreeSet; -use std::path::{Path, PathBuf}; +use std::path::Path; use celox_design::{ BinaryOp, BitAccess, DomainKind, ExternFunction, ExternSignature, ExternType, InitialStateData, @@ -107,8 +107,7 @@ pub(crate) struct LoweredSvModule { #[derive(Clone)] struct AnalyzedSvModule { name: String, - source_code: String, - source_path: PathBuf, + source: std::rc::Rc, implicit_nets_allowed: bool, /// The positional interface of every module in all sources, used to bind /// positional port and parameter connections. @@ -173,21 +172,34 @@ fn analyze_sources( sources: &[(&str, &Path)], ) -> Result, sv::AnalyzerError> { let mut modules = HashMap::default(); + let sources: Vec<_> = sources + .iter() + .map(|(code, path)| { + Ok(( + std::rc::Rc::new(sv::ParsedSource::parse(code, path)?), + *code, + *path, + )) + }) + .collect::>()?; let mut interfaces = sv::ModuleInterfaces::default(); - for (code, path) in sources { - interfaces.extend(sv::source_module_interfaces(code, path)?); + for (source, _, _) in &sources { + interfaces.extend(source.module_interfaces().clone()); } let interfaces = std::sync::Arc::new(interfaces); let mut packages = HashMap::default(); - for (code, path) in sources { - for package in sv::source_packages(code, path)? { + for (source, _, _) in &sources { + for package in source.packages()? { packages.insert(package.name.clone(), package); } } let packages = std::sync::Arc::new(packages); - for (code, path) in sources { - let implicit_net_permissions = sv::source_module_implicit_net_permissions(code, path)?; - for module_name in sv::source_module_names(code, path)? { + for (source, code, path) in &sources { + let implicit_net_permissions: HashMap<_, _> = + sv::source_module_implicit_net_permissions(code, path)? + .into_iter() + .collect(); + for module_name in source.module_names().iter().cloned() { let name = module_name.clone(); if modules.contains_key(&name) { return Err(sv::AnalyzerError::DuplicateModule { name: module_name }); @@ -196,12 +208,11 @@ fn analyze_sources( name, AnalyzedSvModule { implicit_nets_allowed: implicit_net_permissions - .iter() - .find_map(|(name, allowed)| (name == &module_name).then_some(*allowed)) + .get(&module_name) + .copied() .unwrap_or(true), name: module_name, - source_code: (*code).to_string(), - source_path: (*path).to_path_buf(), + source: source.clone(), interfaces: interfaces.clone(), packages: packages.clone(), }, @@ -843,26 +854,12 @@ fn specialize_module( .iter() .filter_map(|parameter| Some((parameter.name.clone(), parameter.type_text.clone()?))) .collect(); - let typed = sv::apply_module_type_parameters( - &module.source_code, - &module.source_path, - &module.name, - &type_overrides, - )?; - let typed_code = typed.as_deref().unwrap_or(&module.source_code); - // A module that uses packages is analyzed with their items inlined. - let inlined = sv::inline_module_packages( - typed_code, - &module.source_path, - &module.name, - &module.packages, - )?; - let ir = sv::analyze_source_module_with_parameter_expr_overrides( - inlined.as_deref().unwrap_or(typed_code), - &module.source_path, + let ir = module.source.analyze_module( &module.name, &overrides, + &type_overrides, &module.interfaces, + &module.packages, )?; let specialized = ir .modules() diff --git a/crates/celox-sv-analyzer/examples/scaling.rs b/crates/celox-sv-analyzer/examples/scaling.rs new file mode 100644 index 0000000000..e4f41bf0b8 --- /dev/null +++ b/crates/celox-sv-analyzer/examples/scaling.rs @@ -0,0 +1,153 @@ +//! Reproducible analyzer scaling probe. Run with an optimized profile: +//! `cargo run --locked -p celox-sv-analyzer --profile heliodor-dev --example scaling` + +use std::{fmt::Write, path::Path, time::Instant}; + +use celox_sv_analyzer::{ModuleInterfaces, ParsedSource, analyze, ast, syntax}; +use fxhash::FxHashMap as HashMap; + +fn source(kind: &str, count: usize) -> String { + if kind == "generate" { + return format!( + "module Top(input logic [7:0] a);\n\ + for (genvar i = 0; i < {count}; i++) begin : g\n\ + logic [7:0] s; assign s = a + i; end\nendmodule\n" + ); + } + let mut code = String::from("module Top(input logic clk, input logic [7:0] a);\n"); + for i in 0..count { + writeln!(code, "logic [7:0] s{i};").unwrap(); + } + for i in 0..count { + match kind { + "assign" => writeln!(code, "assign s{i} = a + 8'd1;"), + "comb" => writeln!(code, "always_comb s{i} = a + 8'd1;"), + "locals" => writeln!( + code, + "always_comb begin logic [7:0] t{i}; t{i} = a + 8'd1; s{i} = t{i}; end" + ), + "ff" => writeln!(code, "always_ff @(posedge clk) s{i} <= a + 8'd1;"), + _ => unreachable!(), + } + .unwrap(); + } + code.push_str("endmodule\n"); + code +} + +fn main() { + let mut args: Vec<_> = std::env::args().skip(1).collect(); + let hierarchy = args.first().is_some_and(|arg| arg == "--hierarchy"); + if hierarchy { + args.remove(0); + } + let counts: Vec = args + .into_iter() + .map(|arg| arg.parse().expect("counts must be positive integers")) + .collect(); + let counts = if counts.is_empty() { + if hierarchy { + vec![8, 32, 128] + } else { + vec![128, 512, 2048] + } + } else { + counts + }; + if hierarchy { + hierarchy_scaling(&counts); + return; + } + println!("kind,count,bytes,parse_ms,ast_ms,ir_ms"); + for kind in ["assign", "comb", "locals", "ff", "generate"] { + for &count in &counts { + let code = source(kind, count); + // Use the median of three runs to limit scheduler noise. Each run + // parses and lowers fresh input; there is no global warm cache. + let mut samples = Vec::new(); + for _ in 0..3 { + let start = Instant::now(); + let tree = syntax::parse_source(&code, Path::new("scaling.sv")).unwrap(); + let parsed = Instant::now(); + let source = ast::Source::from_syntax(&tree).unwrap(); + let lowered = Instant::now(); + let ir = analyze::analyze_source(source).unwrap(); + let finished = Instant::now(); + assert_eq!( + ir.modules()[0].signals().len(), + if kind == "locals" { 2 * count } else { count } + ); + samples.push([ + (parsed - start).as_secs_f64() * 1000.0, + (lowered - parsed).as_secs_f64() * 1000.0, + (finished - lowered).as_secs_f64() * 1000.0, + ]); + } + let median: Vec<_> = (0..3) + .map(|phase| { + let mut times: Vec<_> = samples.iter().map(|sample| sample[phase]).collect(); + times.sort_by(f64::total_cmp); + times[1] + }) + .collect(); + println!( + "{kind},{count},{},{:.3},{:.3},{:.3}", + code.len(), + median[0], + median[1], + median[2] + ); + } + } +} + +// Compare reparsing a multi-module source per module with reusing it. Include +// the reusable source's initial parse, and lower every module in both modes. +fn hierarchy_scaling(counts: &[usize]) { + println!("mode,modules,bytes,total_ms"); + for &count in counts { + let mut code = String::new(); + for i in 0..count { + writeln!(code, "module M{i} #(parameter N = 8) (input logic [N-1:0] a, output logic [N-1:0] y); assign y = a + 1'b1; endmodule").unwrap(); + } + for reuse in [false, true] { + let mut samples = Vec::new(); + for _ in 0..3 { + let start = Instant::now(); + let path = Path::new("hierarchy.sv"); + let parsed = reuse.then(|| ParsedSource::parse(&code, path).unwrap()); + for i in 0..count { + let name = format!("M{i}"); + let overrides = HashMap::default(); + let interfaces = ModuleInterfaces::default(); + let ir = if let Some(parsed) = &parsed { + parsed.analyze_module_with_parameter_expr_overrides( + &name, + &overrides, + &interfaces, + ) + } else { + celox_sv_analyzer::analyze_source_module_with_parameter_expr_overrides( + &code, + path, + &name, + &overrides, + &interfaces, + ) + } + .unwrap(); + assert_eq!(ir.modules().len(), 1); + assert_eq!(ir.modules()[0].name(), name); + } + samples.push(start.elapsed().as_secs_f64() * 1000.0); + } + samples.sort_by(f64::total_cmp); + println!( + "{},{count},{},{:.3}", + if reuse { "reused" } else { "fresh" }, + code.len(), + samples[1] + ); + } + } +} diff --git a/crates/celox-sv-analyzer/src/ast.rs b/crates/celox-sv-analyzer/src/ast.rs index 7b6cc99a5f..b06e953f7e 100644 --- a/crates/celox-sv-analyzer/src/ast.rs +++ b/crates/celox-sv-analyzer/src/ast.rs @@ -37,11 +37,13 @@ mod functions; mod generate; mod inlining; mod instances; +pub(crate) mod module_index; pub mod packages; mod packed_structs; mod parameters; mod patterns; mod procedural; +mod scoped_map; mod selects; mod statements; mod types; @@ -105,7 +107,7 @@ use inlining::{ expand_expr_calls, expr_signedness, expr_signedness_with_return_types, substitute_expr_idents, }; use instances::{ - connection_references_net, expr_ident_name, identifier_text, instances_from_module_node, + collect_connected_nets, expr_ident_name, identifier_text, instances_from_module_node, node_source_text, }; use parameters::{ @@ -115,6 +117,7 @@ use parameters::{ infer_parameter_value_type, parameter_element_literal, parameter_value_env, parameters_from_ref_node, substitute_typed_parameter_literals, }; +use scoped_map::{ScopedMap, Signedness}; use selects::{ add_expr, expr_select_from_select, indexed_select_base, net_lvalue_from_node, part_select_bounds, product_expr, variable_lvalue_from_node, @@ -163,6 +166,18 @@ pub struct ModuleInterface { /// The interface of each module, by module name. pub type ModuleInterfaces = HashMap; +/// Local declarations take precedence over interfaces supplied by other files. +struct InterfaceLookup<'a> { + local: &'a ModuleInterfaces, + extra: &'a ModuleInterfaces, +} + +impl InterfaceLookup<'_> { + fn get(&self, name: &str) -> Option<&ModuleInterface> { + self.local.get(name).or_else(|| self.extra.get(name)) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct Source { modules: Vec, @@ -192,7 +207,10 @@ impl Source { syntax_tree, module_name, ¶meter_overrides, - &interfaces, + &InterfaceLookup { + local: &interfaces, + extra: &ModuleInterfaces::default(), + }, )?); } RefNode::ModuleDeclarationNonansi(_) => { @@ -232,16 +250,32 @@ impl Source { parameter_overrides: &HashMap, extra_interfaces: &ModuleInterfaces, ) -> Result { - let mut interfaces = extra_interfaces.clone(); - interfaces.extend(Self::module_interfaces_from_syntax(syntax_tree)?); + let index = module_index::ModuleIndex::new(syntax_tree)?; + Self::from_indexed_syntax_module( + syntax_tree, + &index, + module_name, + parameter_overrides, + extra_interfaces, + ) + } + + pub(crate) fn from_indexed_syntax_module( + syntax_tree: &SyntaxTree, + index: &module_index::ModuleIndex, + module_name: &str, + parameter_overrides: &HashMap, + extra_interfaces: &ModuleInterfaces, + ) -> Result { + let interfaces = InterfaceLookup { + local: &index.interfaces, + extra: extra_interfaces, + }; let mut modules = Vec::new(); - for node in syntax_tree { + for node in index.nodes(syntax_tree, module_name) { match node { RefNode::ModuleDeclarationAnsi(module) => { let node = RefNode::ModuleDeclarationAnsi(module); - if module_name_from_node(node.clone(), syntax_tree)? != module_name { - continue; - } modules.push(Module::from_module_node_with_parameter_overrides( node, syntax_tree, @@ -334,7 +368,7 @@ impl Module { syntax_tree: &SyntaxTree, override_module_name: &str, parameter_overrides: &HashMap, - interfaces: &ModuleInterfaces, + interfaces: &InterfaceLookup<'_>, ) -> Result { let node = node.into(); let name = module_name_from_node(node.clone(), syntax_tree)?; @@ -438,7 +472,7 @@ impl Module { syntax_tree, &const_env, &type_aliases, - ¶meter_packed_dimensions(¶meters), + ¶meter_packed_dimensions(¶meters).into(), ¶meter_value_env(¶meters, &const_env), ) { Ok(()) => {} @@ -491,7 +525,7 @@ impl Module { syntax_tree, &const_env, &type_aliases, - ¶meter_packed_dimensions(¶meters), + ¶meter_packed_dimensions(¶meters).into(), ¶meter_value_env(¶meters, &const_env), )?; } @@ -531,11 +565,9 @@ impl Module { { validate_unpacked_dimension_sizes(r#type.unpacked_ranges(), &const_env)?; } + let signal_names: HashSet<_> = signals.iter().map(Signal::name).collect(); if let Some(parameter) = parameters.iter().find(|parameter| { - ports.iter().any(|port| port.name() == parameter.name()) - || signals - .iter() - .any(|signal| signal.name() == parameter.name()) + port_names.contains(parameter.name()) || signal_names.contains(parameter.name()) }) { return Err(AnalyzerError::Unsupported(format!( "parameter name collides with port or signal `{}`", @@ -652,7 +684,7 @@ impl Module { )) })); packed_dimensions.functions = Arc::new(functions.clone()); - packed_dimensions.expression_signedness = Arc::new(expression_signedness.clone()); + packed_dimensions.expression_signedness = expression_signedness.clone().into(); for instance in &mut instances { for connection in &mut instance.port_connections { connection.actual_expr = connection.actual_expr.take().map(|expr| { @@ -692,8 +724,6 @@ impl Module { syntax_tree, &const_env, &packed_dimensions, - &functions, - &expression_signedness, ¶meter_values, &mut body_state, )? @@ -732,16 +762,19 @@ impl Module { .flat_map(|process| process.body.iter()) .chain(ff_processes.iter().flat_map(|process| process.body.iter())), ); + let mut connected_nets = HashSet::default(); + for connection in instances + .iter() + .flat_map(|instance| instance.port_connections()) + { + if let Some(expr) = connection.actual_expr() { + collect_connected_nets(expr, &mut connected_nets); + } + } if let Some(signal) = signals.iter().find(|signal| { signal.is_net() && !procedurally_written.contains(signal.name()) - && !instances.iter().any(|instance| { - instance.port_connections().iter().any(|connection| { - connection - .actual_expr() - .is_some_and(|expr| connection_references_net(expr, signal.name())) - }) - }) + && !connected_nets.contains(signal.name()) }) { return Err(AnalyzerError::Unsupported(format!( "undriven net declaration `{}`", @@ -1713,13 +1746,13 @@ struct FunctionReturnMetadata { #[derive(Debug, Clone, Default, PartialEq, Eq)] struct PackedDimensions { - variables: VariablePackedDimensions, + variables: ScopedMap, const_env: HashMap, type_aliases: HashMap, function_return_types: HashMap, functions: Arc>, parameter_values: HashMap, - expression_signedness: Arc>, + expression_signedness: ScopedMap, constant_indexed_base: bool, /// The declared shape of each argument of each subroutine, for /// assignment patterns passed as arguments. @@ -1728,18 +1761,18 @@ struct PackedDimensions { impl PackedDimensions { fn new( - variables: VariablePackedDimensions, + variables: impl Into>, const_env: &HashMap, type_aliases: &HashMap, ) -> Self { Self { - variables, + variables: variables.into(), const_env: const_env.clone(), type_aliases: type_aliases.clone(), function_return_types: HashMap::default(), functions: Arc::default(), parameter_values: HashMap::default(), - expression_signedness: Arc::default(), + expression_signedness: ScopedMap::default(), constant_indexed_base: false, subroutine_param_shapes: Arc::default(), } @@ -1747,7 +1780,7 @@ impl PackedDimensions { } impl Deref for PackedDimensions { - type Target = VariablePackedDimensions; + type Target = ScopedMap; fn deref(&self) -> &Self::Target { &self.variables @@ -1760,6 +1793,12 @@ impl DerefMut for PackedDimensions { } } +impl Signedness for PackedDimensions { + fn signedness(&self, name: &str) -> Option { + self.get(name).map(|dimensions| dimensions.signed) + } +} + /// The outermost generate scope of a scoped name such as `g.x` or `g[0].x`, /// or `None` for a name declared directly in the module. An escaped scope /// component ends at its terminating space. diff --git a/crates/celox-sv-analyzer/src/ast/comb_process.rs b/crates/celox-sv-analyzer/src/ast/comb_process.rs index 6bdc7a3ab3..2c192cf7cf 100644 --- a/crates/celox-sv-analyzer/src/ast/comb_process.rs +++ b/crates/celox-sv-analyzer/src/ast/comb_process.rs @@ -7,8 +7,6 @@ pub(super) fn comb_processes_from_module_node( syntax_tree: &SyntaxTree, const_env: &HashMap, packed_dimensions: &PackedDimensions, - functions: &HashMap, - expression_signedness: &HashMap, parameter_literals: &HashMap, state: &mut procedural::BodyState<'_>, ) -> Result, AnalyzerError> { @@ -20,10 +18,7 @@ pub(super) fn comb_processes_from_module_node( &packed_dimensions.type_aliases, )? { let start = processes.len(); - let mut base_dimensions = packed_dimensions.clone(); - base_dimensions.functions = Arc::new(functions.clone()); - base_dimensions.expression_signedness = Arc::new(expression_signedness.clone()); - let dimensions = item.dimensions(&base_dimensions); + let dimensions = item.dimensions(packed_dimensions); let literals = item.parameter_literals(parameter_literals); comb_processes_from_module_or_generate_item( item.node, @@ -31,8 +26,6 @@ pub(super) fn comb_processes_from_module_node( syntax_tree, &item.env, &dimensions, - &dimensions.functions, - &dimensions.expression_signedness, &literals, &mut processes, state, @@ -55,8 +48,6 @@ fn comb_processes_from_module_or_generate_item( syntax_tree: &SyntaxTree, const_env: &HashMap, packed_dimensions: &PackedDimensions, - functions: &HashMap, - expression_signedness: &HashMap, parameter_literals: &HashMap, processes: &mut Vec, state: &mut procedural::BodyState<'_>, @@ -68,8 +59,6 @@ fn comb_processes_from_module_or_generate_item( syntax_tree, const_env, packed_dimensions, - functions, - expression_signedness, parameter_literals, processes, state, @@ -84,8 +73,6 @@ fn comb_processes_from_module_common_item( syntax_tree: &SyntaxTree, const_env: &HashMap, packed_dimensions: &PackedDimensions, - functions: &HashMap, - expression_signedness: &HashMap, parameter_literals: &HashMap, processes: &mut Vec, state: &mut procedural::BodyState<'_>, @@ -117,7 +104,6 @@ fn comb_processes_from_module_common_item( let mut local_packed_dimensions = packed_dimensions.clone(); local_packed_dimensions.const_env = const_env.clone(); local_packed_dimensions.parameter_values = parameter_literals.clone(); - let _ = (functions, expression_signedness); if let Some(process) = comb_process_from_always_construct( always, condition, diff --git a/crates/celox-sv-analyzer/src/ast/generate.rs b/crates/celox-sv-analyzer/src/ast/generate.rs index 0556ebcff1..8bafb13e5e 100644 --- a/crates/celox-sv-analyzer/src/ast/generate.rs +++ b/crates/celox-sv-analyzer/src/ast/generate.rs @@ -68,10 +68,14 @@ impl Item<'_> { let mut local = dimensions.clone(); local.const_env = self.env.clone(); local.parameter_values = self.parameter_literals(&dimensions.parameter_values); - local.functions = Arc::new(self.functions(&dimensions.functions)); - local.function_return_types = - self.function_aliases(dimensions.function_return_types.clone()); - let mut signedness = (*dimensions.expression_signedness).clone(); + // Ordinary module items keep the module's function bindings. Rewriting + // every function body for every item costs O(items * function size). + if !self.shadowed.is_empty() || !self.names.is_empty() { + local.functions = Arc::new(self.functions(&dimensions.functions)); + local.function_return_types = + self.function_aliases(dimensions.function_return_types.clone()); + } + let mut signedness = dimensions.expression_signedness.clone(); for name in &self.shadowed { let protected = format!("{OUTER_BINDING}{name}"); if let Some(value) = dimensions.get(name) { @@ -101,7 +105,7 @@ impl Item<'_> { signedness.insert(name.clone(), value.signed); } } - local.expression_signedness = Arc::new(signedness); + local.expression_signedness = signedness; local } @@ -347,7 +351,7 @@ impl<'a> Elaborator<'a, '_> { if scope.shadowed.contains(name) { return None; } - let signedness = parameter_types_from_const_env(&scope.env) + let signedness: HashMap<_, _> = parameter_types_from_const_env(&scope.env) .into_iter() .map(|(name, ty)| (name, ty.signed)) .collect(); diff --git a/crates/celox-sv-analyzer/src/ast/inlining.rs b/crates/celox-sv-analyzer/src/ast/inlining.rs index 5f6243c738..0061b0cbdc 100644 --- a/crates/celox-sv-analyzer/src/ast/inlining.rs +++ b/crates/celox-sv-analyzer/src/ast/inlining.rs @@ -4,7 +4,7 @@ use super::*; pub(super) fn expr_signedness( expr: &Expr, - identifiers: &HashMap, + identifiers: &impl Signedness, functions: &HashMap, ) -> Option { expr_signedness_with_return_types(expr, identifiers, functions, &HashMap::default()) @@ -12,12 +12,12 @@ pub(super) fn expr_signedness( pub(super) fn expr_signedness_with_return_types( expr: &Expr, - identifiers: &HashMap, + identifiers: &impl Signedness, functions: &HashMap, function_return_types: &HashMap, ) -> Option { match expr { - Expr::Ident(name) => identifiers.get(name).copied(), + Expr::Ident(name) => identifiers.signedness(name), Expr::Literal(literal) => { typecheck::parse_integral_literal(literal).map(|literal| literal.signed) } @@ -111,7 +111,7 @@ pub(super) fn expr_signedness_with_return_types( pub(super) fn expand_expr_calls( expr: Expr, functions: &HashMap, - expression_signedness: &HashMap, + expression_signedness: &impl Signedness, depth: usize, apply_return_type: bool, ) -> Expr { diff --git a/crates/celox-sv-analyzer/src/ast/instances.rs b/crates/celox-sv-analyzer/src/ast/instances.rs index 0e2e49b13e..6852af553b 100644 --- a/crates/celox-sv-analyzer/src/ast/instances.rs +++ b/crates/celox-sv-analyzer/src/ast/instances.rs @@ -7,7 +7,7 @@ pub(super) fn instances_from_module_node( syntax_tree: &SyntaxTree, const_env: &HashMap, packed_dimensions: &PackedDimensions, - interfaces: &ModuleInterfaces, + interfaces: &InterfaceLookup<'_>, ) -> Result, AnalyzerError> { let type_aliases = type_aliases_from_module_node(node.clone(), syntax_tree)?; let active = generate::items(node, syntax_tree, const_env, &type_aliases)?; @@ -48,7 +48,7 @@ fn instances_from_module_or_generate_item( syntax_tree: &SyntaxTree, const_env: &HashMap, packed_dimensions: &PackedDimensions, - interfaces: &ModuleInterfaces, + interfaces: &InterfaceLookup<'_>, instances: &mut Vec, ) -> Result<(), AnalyzerError> { if let sv_parser::ModuleOrGenerateItem::Module(module) = item { @@ -71,7 +71,7 @@ fn instances_from_module_instantiation( syntax_tree: &SyntaxTree, const_env: &HashMap, packed_dimensions: &PackedDimensions, - interfaces: &ModuleInterfaces, + interfaces: &InterfaceLookup<'_>, instances: &mut Vec, ) -> Result<(), AnalyzerError> { let module_name = identifier_text( @@ -331,19 +331,23 @@ fn port_connections_from_hierarchical_instance( // Child port directions are resolved by the frontend adapter. Defer its net // driver check for selected/concatenated connections as well as whole nets. -pub(super) fn connection_references_net(expr: &Expr, name: &str) -> bool { +pub(super) fn collect_connected_nets<'a>(expr: &'a Expr, names: &mut HashSet<&'a str>) { match expr { - Expr::Ident(actual) => actual == name, + Expr::Ident(actual) => { + names.insert(actual); + } Expr::Select { expr, .. } | Expr::Resize { expr, .. } | Expr::Unary { op: UnaryOp::ToTwoState, expr, - } => connection_references_net(expr, name), - Expr::Concat(parts) => parts - .iter() - .any(|part| connection_references_net(part, name)), - _ => false, + } => collect_connected_nets(expr, names), + Expr::Concat(parts) => { + for part in parts { + collect_connected_nets(part, names); + } + } + _ => {} } } diff --git a/crates/celox-sv-analyzer/src/ast/module_index.rs b/crates/celox-sv-analyzer/src/ast/module_index.rs new file mode 100644 index 0000000000..6c7114859b --- /dev/null +++ b/crates/celox-sv-analyzer/src/ast/module_index.rs @@ -0,0 +1,107 @@ +//! Locations of module nodes, without borrowing from the owned syntax tree. + +use super::*; + +#[derive(Clone, Copy)] +enum Location { + Description(usize), + // Preserve support for nested declarations and unusual source forms. + Nested(usize), +} + +#[derive(Default)] +pub(crate) struct ModuleIndex { + names: Vec, + locations: HashMap>, + pub interfaces: ModuleInterfaces, +} + +fn source_text(tree: &SyntaxTree) -> Option<&sv_parser::SourceText> { + tree.into_iter().find_map(|node| match node { + RefNode::SourceText(source) => Some(source), + _ => None, + }) +} + +fn module_node(declaration: &sv_parser::ModuleDeclaration) -> Option> { + match declaration { + sv_parser::ModuleDeclaration::Ansi(module) => Some(RefNode::ModuleDeclarationAnsi(module)), + sv_parser::ModuleDeclaration::Nonansi(module) => { + Some(RefNode::ModuleDeclarationNonansi(module)) + } + _ => None, + } +} + +fn identifier_offset(node: &RefNode<'_>) -> Option { + let identifier = match node { + RefNode::ModuleDeclarationAnsi(module) => &module.nodes.0.nodes.3, + RefNode::ModuleDeclarationNonansi(module) => &module.nodes.0.nodes.3, + _ => return None, + }; + identifier_locate(RefNode::ModuleIdentifier(identifier)).map(|locate| locate.offset) +} + +impl ModuleIndex { + pub fn new(tree: &SyntaxTree) -> Result { + let mut index = Self::default(); + let mut descriptions = HashMap::default(); + if let Some(source) = source_text(tree) { + for (i, description) in source.nodes.2.iter().enumerate() { + if let sv_parser::Description::ModuleDeclaration(declaration) = description + && let Some(node) = module_node(declaration) + && let Some(offset) = identifier_offset(&node) + { + descriptions.insert(offset, i); + } + } + } + for node in tree { + let Some(offset) = identifier_offset(&node) else { + continue; + }; + let name = module_name_from_node(node.clone(), tree)?; + index.names.push(name.clone()); + let location = descriptions + .get(&offset) + .map_or(Location::Nested(offset), |&i| Location::Description(i)); + index + .locations + .entry(name.clone()) + .or_default() + .push(location); + if matches!(node, RefNode::ModuleDeclarationAnsi(_)) { + index + .interfaces + .insert(name, module_interface_from_node(node, tree)?); + } + } + Ok(index) + } + + pub fn names(&self) -> &[String] { + &self.names + } + + pub fn nodes<'a>(&self, tree: &'a SyntaxTree, name: &str) -> Vec> { + let Some(locations) = self.locations.get(name) else { + return Vec::new(); + }; + locations + .iter() + .filter_map(|location| match location { + Location::Description(i) => { + let sv_parser::Description::ModuleDeclaration(declaration) = + &source_text(tree)?.nodes.2[*i] + else { + return None; + }; + module_node(declaration) + } + Location::Nested(offset) => tree + .into_iter() + .find(|node| identifier_offset(node) == Some(*offset)), + }) + .collect() + } +} diff --git a/crates/celox-sv-analyzer/src/ast/procedural.rs b/crates/celox-sv-analyzer/src/ast/procedural.rs index c3b79a7376..40c733a7e1 100644 --- a/crates/celox-sv-analyzer/src/ast/procedural.rs +++ b/crates/celox-sv-analyzer/src/ast/procedural.rs @@ -133,7 +133,7 @@ impl<'s, 't, 'a> BodyBuilder<'s, 't, 'a> { if let Some(value) = entry.shadowed_constant { self.dims.const_env.insert(entry.source.clone(), value); } - let signedness = Arc::make_mut(&mut self.dims.expression_signedness); + let signedness = &mut self.dims.expression_signedness; match entry.shadowed_signedness { Some(signed) => { signedness.insert(entry.source, signed); @@ -153,7 +153,9 @@ impl<'s, 't, 'a> BodyBuilder<'s, 't, 'a> { .dims .insert(source.to_string(), dimensions_from_type(&r#type)); let shadowed_constant = self.dims.const_env.remove(source); - let shadowed_signedness = Arc::make_mut(&mut self.dims.expression_signedness) + let shadowed_signedness = self + .dims + .expression_signedness .insert(source.to_string(), r#type.is_signed()); self.state.locals.push(LocalVariable { name: unique.clone(), diff --git a/crates/celox-sv-analyzer/src/ast/scoped_map.rs b/crates/celox-sv-analyzer/src/ast/scoped_map.rs new file mode 100644 index 0000000000..e39d271d8e --- /dev/null +++ b/crates/celox-sv-analyzer/src/ast/scoped_map.rs @@ -0,0 +1,141 @@ +//! Shared module symbols with small, independently mutable scope overlays. + +use std::sync::Arc; + +use fxhash::FxHashMap as HashMap; + +/// Cloning a scope shares both tables. Mutation copies only local changes when +/// another scope still shares the overlay, never the complete module table. +/// `None` in the overlay hides a symbol in the shared table. +#[derive(Debug, Clone)] +pub(super) struct ScopedMap { + base: Arc>, + overlay: Arc>>, +} + +impl Default for ScopedMap { + fn default() -> Self { + Self::from(HashMap::default()) + } +} + +impl From> for ScopedMap { + fn from(base: HashMap) -> Self { + Self { + base: Arc::new(base), + overlay: Arc::default(), + } + } +} + +impl ScopedMap { + pub fn get(&self, name: &str) -> Option<&V> { + match self.overlay.get(name) { + Some(value) => value.as_ref(), + None => self.base.get(name), + } + } + + pub fn iter(&self) -> impl Iterator { + self.base + .iter() + .filter(|(name, _)| !self.overlay.contains_key(*name)) + .chain( + self.overlay + .iter() + .filter_map(|(name, value)| value.as_ref().map(|value| (name, value))), + ) + } +} + +impl ScopedMap { + pub fn insert(&mut self, name: String, value: V) -> Option { + // While constructing the module, populate the base without an overlay. + if self.overlay.is_empty() + && let Some(base) = Arc::get_mut(&mut self.base) + { + return base.insert(name, value); + } + let old = self.get(&name).cloned(); + Arc::make_mut(&mut self.overlay).insert(name, Some(value)); + old + } + + pub fn remove(&mut self, name: &str) -> Option { + if self.overlay.is_empty() + && let Some(base) = Arc::get_mut(&mut self.base) + { + return base.remove(name); + } + let old = self.get(name).cloned(); + if self.base.contains_key(name) { + Arc::make_mut(&mut self.overlay).insert(name.to_string(), None); + } else { + Arc::make_mut(&mut self.overlay).remove(name); + } + old + } +} + +impl Extend<(String, V)> for ScopedMap { + fn extend>(&mut self, iter: T) { + for (name, value) in iter { + self.insert(name, value); + } + } +} + +impl PartialEq for ScopedMap { + fn eq(&self, other: &Self) -> bool { + self.iter() + .all(|(name, value)| other.get(name) == Some(value)) + && self.iter().count() == other.iter().count() + } +} + +impl Eq for ScopedMap {} + +/// Signedness inference accepts both module tables and scoped overlays. +pub(super) trait Signedness { + fn signedness(&self, name: &str) -> Option; +} + +impl Signedness for HashMap { + fn signedness(&self, name: &str) -> Option { + self.get(name).copied() + } +} + +impl Signedness for ScopedMap { + fn signedness(&self, name: &str) -> Option { + self.get(name).copied() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn scopes_shadow_remove_and_restore_without_changing_siblings() { + let base = ScopedMap::from(HashMap::from_iter([("x".into(), 1), ("y".into(), 2)])); + let mut scope = base.clone(); + assert!(Arc::ptr_eq(&scope.base, &base.base)); + assert_eq!(scope.insert("x".into(), 3), Some(1)); + assert_eq!(scope.remove("y"), Some(2)); + scope.insert("z".into(), 4); + let mut nested = scope.clone(); + assert!(Arc::ptr_eq(&nested.overlay, &scope.overlay)); + assert_eq!(nested.remove("x"), Some(3)); + assert_eq!(nested.remove("z"), Some(4)); + assert_eq!(scope.get("x"), Some(&3)); + assert_eq!(scope.get("z"), Some(&4)); + assert_eq!(base.get("x"), Some(&1)); + assert_eq!(base.get("y"), Some(&2)); + scope.insert("x".into(), 1); + scope.insert("y".into(), 2); + scope.remove("z"); + assert_eq!(scope, base); + assert_eq!(scope.iter().count(), 2); + } +} diff --git a/crates/celox-sv-analyzer/src/ast/statements.rs b/crates/celox-sv-analyzer/src/ast/statements.rs index 44a8cc61bd..280386cfdc 100644 --- a/crates/celox-sv-analyzer/src/ast/statements.rs +++ b/crates/celox-sv-analyzer/src/ast/statements.rs @@ -10,13 +10,9 @@ pub(super) fn coerce_procedural_assignment_rhs( let Some(target_type) = lvalue_expr_type(lhs, packed_dimensions) else { return rhs; }; - let identifier_signedness = packed_dimensions - .iter() - .map(|(name, dimensions)| (name.clone(), dimensions.signed)) - .collect(); let Some(source_signed) = expr_signedness_with_return_types( &rhs, - &identifier_signedness, + packed_dimensions, &HashMap::default(), &packed_dimensions.function_return_types, ) else { diff --git a/crates/celox-sv-analyzer/src/ast/validation.rs b/crates/celox-sv-analyzer/src/ast/validation.rs index db056ab031..572aa48ecf 100644 --- a/crates/celox-sv-analyzer/src/ast/validation.rs +++ b/crates/celox-sv-analyzer/src/ast/validation.rs @@ -100,7 +100,7 @@ pub(super) fn reject_silently_ignored_constructs( syntax_tree: &SyntaxTree, const_env: &HashMap, type_aliases: &HashMap, - parameter_dimensions: &VariablePackedDimensions, + parameter_dimensions: &ScopedMap, parameter_values: &HashMap, ) -> Result<(), AnalyzerError> { let mut indexed_dimensions = diff --git a/crates/celox-sv-analyzer/src/lib.rs b/crates/celox-sv-analyzer/src/lib.rs index 277ff32530..e47de6bcd2 100644 --- a/crates/celox-sv-analyzer/src/lib.rs +++ b/crates/celox-sv-analyzer/src/lib.rs @@ -15,6 +15,7 @@ use thiserror::Error; pub mod analyze; pub mod ast; pub mod ir; +mod parsed; pub mod procedural; pub mod symbol; pub mod syntax; @@ -24,6 +25,7 @@ pub mod typecheck; pub use ast::packages::PackageSource; pub use ast::{ModuleInterface, ModuleInterfaces}; pub use ir::Ir; +pub use parsed::ParsedSource; /// Internal marker used to defer division-by-zero state handling until the /// simulator's two-state/four-state mode is known. @@ -179,18 +181,11 @@ pub fn analyze_source_module_with_parameter_expr_overrides( parameter_overrides: &HashMap, interfaces: &ModuleInterfaces, ) -> Result { - let syntax_tree = syntax::parse_source(code, path)?; - let parameter_overrides = parameter_overrides - .iter() - .map(|(name, value)| (name.clone(), value.clone().into())) - .collect(); - let source = ast::Source::from_syntax_module_with_parameter_expr_overrides( - &syntax_tree, + ParsedSource::parse(code, path)?.analyze_module_with_parameter_expr_overrides( module_name, - ¶meter_overrides, - &interfaces.clone().into_iter().collect(), - )?; - analyze::analyze_source(source) + parameter_overrides, + interfaces, + ) } /// The positional interface (ports and overridable parameters) of every diff --git a/crates/celox-sv-analyzer/src/parsed.rs b/crates/celox-sv-analyzer/src/parsed.rs new file mode 100644 index 0000000000..d9956607c6 --- /dev/null +++ b/crates/celox-sv-analyzer/src/parsed.rs @@ -0,0 +1,307 @@ +//! Reusable syntax and module metadata for hierarchy specialization. + +use std::path::{Path, PathBuf}; + +use fxhash::FxHashMap as HashMap; + +use crate::{AnalyzerError, Ir, ModuleInterfaces, PackageSource, analyze, ast, ir, syntax}; + +/// A parsed source that can analyze many modules or parameter specializations. +/// +/// The syntax tree and module interfaces are built once. Module lookup follows +/// cached top-level declaration indices, without scanning unrelated bodies. +/// No elaborated AST is cached: overrides and constant-function environments +/// remain local to each analysis. Like `sv-parser`'s tree, this value is confined +/// to the parsing thread; consumers can share it there with [`std::rc::Rc`]. +pub struct ParsedSource { + code: String, + path: PathBuf, + tree: sv_parser::SyntaxTree, + index: ast::module_index::ModuleIndex, +} + +impl ParsedSource { + pub fn parse(code: &str, path: &Path) -> Result { + let tree = syntax::parse_source(code, path)?; + let index = ast::module_index::ModuleIndex::new(&tree)?; + Ok(Self { + code: code.to_string(), + path: path.to_path_buf(), + tree, + index, + }) + } + + pub fn module_names(&self) -> &[String] { + self.index.names() + } + + pub fn module_interfaces(&self) -> &ModuleInterfaces { + &self.index.interfaces + } + + pub fn packages(&self) -> Result, AnalyzerError> { + ast::packages::source_packages(&self.code, &self.tree) + } + + /// Analyze a module, rewriting its type parameters and package imports + /// when needed. Numeric overrides reuse the original syntax tree. + pub fn analyze_module( + &self, + module_name: &str, + overrides: &HashMap, + type_overrides: &[(String, String)], + interfaces: &ModuleInterfaces, + packages: &HashMap, + ) -> Result { + let typed = if type_overrides.is_empty() { + None + } else { + ast::packages::apply_type_parameter_overrides( + &self.code, + &self.tree, + module_name, + type_overrides, + )? + }; + let typed_source = typed + .as_deref() + .map(|code| Self::parse(code, &self.path)) + .transpose()?; + let source = typed_source.as_ref().unwrap_or(self); + let inlined = if packages.is_empty() { + None + } else { + ast::packages::inline_packages(&source.code, &source.tree, module_name, packages)? + }; + let inlined_source = inlined + .as_deref() + .map(|code| Self::parse(code, &self.path)) + .transpose()?; + let source = inlined_source.as_ref().unwrap_or(source); + source.analyze_module_with_parameter_expr_overrides(module_name, overrides, interfaces) + } + + /// Analyze without rewriting the source, retaining literal override types. + pub fn analyze_module_with_parameter_expr_overrides( + &self, + module_name: &str, + overrides: &HashMap, + interfaces: &ModuleInterfaces, + ) -> Result { + let overrides = overrides + .iter() + .map(|(name, value)| (name.clone(), value.clone().into())) + .collect(); + let source = ast::Source::from_indexed_syntax_module( + &self.tree, + &self.index, + module_name, + &overrides, + interfaces, + )?; + analyze::analyze_source(source) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn specializations_keep_constants_and_function_bindings_independent() { + let code = r#" + module A #(parameter N = 2) (output logic [N-1:0] y); + function automatic int f(); return N; endfunction + for (genvar i = 0; i < f(); i++) begin : g + logic [N-1:0] s; + assign s = N; + end + assign y = N; + endmodule + module B(output logic [7:0] y); + function automatic int f(); return 7; endfunction + localparam V = f(); + assign y = V; + endmodule + "#; + let path = Path::new("specializations.sv"); + let source = ParsedSource::parse(code, path).unwrap(); + for n in [2, 5, 1, 2] { + let overrides = + HashMap::from_iter([("N".into(), ir::ConstExpr::Literal(n.to_string()))]); + let actual = source + .analyze_module_with_parameter_expr_overrides( + "A", + &overrides, + &ModuleInterfaces::default(), + ) + .unwrap(); + let fresh = crate::analyze_source_with_module_parameter_overrides( + code, + path, + "A", + &HashMap::from_iter([("N".into(), n)]), + ) + .unwrap(); + let expected = &fresh.modules()[0]; + let actual = &actual.modules()[0]; + assert_eq!(actual.parameters(), expected.parameters()); + assert_eq!(actual.ports(), expected.ports()); + assert_eq!(actual.signals(), expected.signals()); + assert_eq!(actual.comb_processes(), expected.comb_processes()); + assert_eq!(actual.signals().len(), n as usize); + let b = source + .analyze_module_with_parameter_expr_overrides( + "B", + &HashMap::default(), + &ModuleInterfaces::default(), + ) + .unwrap(); + assert_eq!(b.modules()[0].parameters()[0].resolved_value(), Some(7)); + } + } + + #[test] + fn local_interfaces_override_external_ones_and_other_sources_bind_positionally() { + let source = ParsedSource::parse( + r#" + module Child(input logic a); endmodule + module Top(input logic x); + Child c(x); + External #(3) e(x); + endmodule + "#, + Path::new("interfaces.sv"), + ) + .unwrap(); + let mut interfaces = HashMap::default(); + interfaces.insert( + "Child".into(), + crate::ModuleInterface { + ports: vec!["wrong".into()], + parameters: Vec::new(), + }, + ); + interfaces.insert( + "External".into(), + crate::ModuleInterface { + ports: vec!["ext".into()], + parameters: vec!["N".into()], + }, + ); + let ir = source + .analyze_module_with_parameter_expr_overrides("Top", &HashMap::default(), &interfaces) + .unwrap(); + let instances = ir.modules()[0].instances(); + assert_eq!(instances[0].port_connections()[0].formal(), "a"); + assert_eq!(instances[1].port_connections()[0].formal(), "ext"); + assert_eq!(instances[1].parameter_overrides()[0].name(), "N"); + } + + #[test] + fn rewrites_type_parameters_and_packages_before_analysis() { + let code = r#" + package p; localparam V = 3; endpackage + module Top #(parameter type T = logic [3:0]) (output T y); + import p::*; + assign y = V; + endmodule + "#; + let path = Path::new("rewritten.sv"); + let source = ParsedSource::parse(code, path).unwrap(); + let packages = source + .packages() + .unwrap() + .into_iter() + .map(|p| (p.name.clone(), p)) + .collect(); + let types = vec![("T".into(), "logic [7:0]".into())]; + let actual = source + .analyze_module( + "Top", + &HashMap::default(), + &types, + &ModuleInterfaces::default(), + &packages, + ) + .unwrap(); + let typed = crate::apply_module_type_parameters(code, path, "Top", &types) + .unwrap() + .unwrap(); + let inlined = crate::inline_module_packages(&typed, path, "Top", &packages) + .unwrap() + .unwrap(); + let expected = crate::analyze_source(&inlined, path).unwrap(); + assert_eq!(actual, expected); + assert_eq!( + actual.modules()[0].ports()[0].r#type().resolved_width(), + Some(8) + ); + } + + #[test] + fn indexed_lookup_preserves_nested_escaped_and_duplicate_modules() { + let source = ParsedSource::parse( + r#" + module Outer; + module Inner(output logic y); assign y = 1'b1; endmodule + endmodule + module \escaped.name (output logic y); assign y = 1'b0; endmodule + module Duplicate; endmodule + module Duplicate; endmodule + module Legacy(y); output y; endmodule + "#, + Path::new("index.sv"), + ) + .unwrap(); + assert_eq!( + source.module_names(), + [ + "Outer", + "Inner", + "\\escaped.name", + "Duplicate", + "Duplicate", + "Legacy" + ] + ); + for name in ["Inner", "\\escaped.name"] { + let ir = source + .analyze_module_with_parameter_expr_overrides( + name, + &HashMap::default(), + &ModuleInterfaces::default(), + ) + .unwrap(); + assert_eq!(ir.modules()[0].name(), name); + } + assert!(matches!( + source.analyze_module_with_parameter_expr_overrides( + "Duplicate", + &HashMap::default(), + &ModuleInterfaces::default(), + ), + Err(AnalyzerError::DuplicateModule { .. }) + )); + assert!(matches!( + source.analyze_module_with_parameter_expr_overrides( + "Legacy", + &HashMap::default(), + &ModuleInterfaces::default(), + ), + Err(AnalyzerError::Unsupported(_)) + )); + assert!( + source + .analyze_module_with_parameter_expr_overrides( + "Missing", + &HashMap::default(), + &ModuleInterfaces::default(), + ) + .unwrap() + .modules() + .is_empty() + ); + } +} diff --git a/docs/internals/sv-analysis-performance.md b/docs/internals/sv-analysis-performance.md new file mode 100644 index 0000000000..02747f9415 --- /dev/null +++ b/docs/internals/sv-analysis-performance.md @@ -0,0 +1,131 @@ +# SystemVerilog analyzer scaling + +The scaling probe separates `sv-parser`, analyzer AST construction, and AST-to-IR +conversion. It also compares parsing a multi-module file for each selected +module with reusing one `ParsedSource`. This measures frontend work, without +backend compilation or simulation. + +```sh +cargo run --locked -p celox-sv-analyzer --profile heliodor-dev --example scaling +cargo run --locked -p celox-sv-analyzer --profile heliodor-dev --example scaling -- --hierarchy +``` + +Optional integer arguments replace the default counts. Flat designs default to +128, 512, and 2,048 signals/processes; hierarchy measurements default to 8, 32, +and 128 modules. Each reported phase is the median of three fresh runs. Inputs +and output counts are checked; wall-clock thresholds are deliberately absent. +The [probe source](../../crates/celox-sv-analyzer/examples/scaling.rs) contains +the generators and measurement boundaries. + +## Changed algorithms + +Previously, each declaration or process copied the module's complete variable +dimension map. With S signals and B bodies/items, this contributed O(S * B) +work even when a body referenced one signal. Procedural local declarations also +copied the entire signedness map through `Arc::make_mut`. + +Variable dimensions and signedness now use a shared module table plus a small +scope overlay. A scope clone shares both tables; mutation copies local changes +only when an overlay is still shared. Insertion, shadowing, and removal affect +the overlay. Removed base names have +tombstones, and iteration yields each visible name once. Module construction +fills an exclusively owned base directly. Ordinary module items also reuse the +function table, while generated scopes still rewrite definition-site bindings +when they introduce aliases or shadowed names. + +Assignment coercion now queries signedness for identifiers used in the +expression, rather than collecting all signal names for every assignment. +Parameter-name collision checks use name sets. Connected nets are collected +once from instance actuals, replacing a scan of every connection for every net. +The set preserves the previous connection forms for which driver validation +is deferred to the frontend adapter. + +The SV frontend previously parsed whole files for interfaces, packages, module +names, type-parameter rewrites, package inlining, and each specialization. It +also copied the entire source text into every module record. It now shares one +`ParsedSource` per file through `Rc`. Names, interfaces, and top-level module +indices are collected once. Interfaces from the current file and other files +are queried through borrowed tables, with the existing local precedence. +Numeric specializations lower fresh ASTs from the shared syntax tree. + +The syntax cache lives for one frontend build, rather than globally. It retains +the original CST until that build finishes, trading that linear storage for +repeated parsing and per-module source copies. `sv-parser`'s tree is confined +to its parsing thread. Generated simulator execution remains independent of +this syntax cache. + +## Measurements + +Measurements on an AMD Ryzen 7 9800X3D under WSL, Rust 1.99.0, using the +`heliodor-dev` profile (optimized, no LTO). The baseline is revision +`c802d5b64`, with the same assign/comb input generators and profile. +Times are milliseconds and are illustrative measurements on a shared machine, +not acceptance thresholds or a claim about every SV workload. + +| Workload | Signals | AST before | AST after | +| --- | ---: | ---: | ---: | +| continuous assign | 128 | 160.782 | 18.659 | +| continuous assign | 512 | 2,380.909 | 62.772 | +| continuous assign | 2,048 | 39,361.071 | 280.932 | +| always_comb | 128 | 203.982 | 16.932 | +| always_comb | 512 | 2,884.785 | 75.250 | +| always_comb | 2,048 | 42,455.450 | 298.759 | + +At 2,048 signals, AST construction improves by about 140x for continuous +assignments and 142x for `always_comb`. Parsing still takes about 497–524 ms +for these inputs, and IR conversion takes about 1–3 ms. The AST measurements +exclude those phases and should not be interpreted as end-to-end speedups. + +Additional post-change AST measurements: + +| Workload | 128 items | 512 items | 2,048 items | +| --- | ---: | ---: | ---: | +| always_comb with a local variable | 37.736 | 150.626 | 665.813 | +| always_ff | 24.611 | 88.814 | 369.507 | +| loop generate | 62.650 | 241.653 | 1,018.967 | + +The hierarchy probe analyzes every module of one file. Both modes use the +post-change analyzer, so this comparison isolates syntax reuse and includes the +initial parse in the reused mode. It does not measure a complete simulator +build or the old frontend's additional metadata/rewrite parses. + +| Modules | Parse separately per module | Reuse parsed source | +| --- | ---: | ---: | +| 8 | 37.387 | 5.759 | +| 32 | 531.142 | 26.507 | +| 128 | 8,819.291 | 109.228 | + +## Remaining boundaries + +Type-parameter substitutions and package inlining rewrite source text and still +require parsing the rewritten source. Nested module declarations retain the +syntax-walk fallback; ordinary top-level module lookup avoids unrelated bodies. +Resolving the parser's source root can still visit its description list. + +Constant environments, type-alias tables, and generated parameter metadata are +still copied in some scope paths. Many dependent parameters, many functions +inside generated scopes, and repeated `$bits`/`$size` type discovery can have +different scaling from the flat probes: the latter still discovers enclosing +declarations by walking syntax. This change does not establish linear scaling +for those workloads. The probes provide reproducible baselines for further +optimization without changing their language semantics. + +## Validation + +Scope-overlay and reusable-source regressions belong to the analyzer because +they check internal representation, cache isolation, and its API. They cover +shadow/remove/restore, repeated numeric specializations and constant functions, +local/external positional interfaces, type-parameter and package rewrites, +nested and escaped names, duplicate modules, and non-ANSI rejection. + +The existing SystemVerilog integration suite exercises native, Cranelift, and +Wasm consumers, including parallel and four-state cases. DPI and unpacked-array +regressions also run: + +```sh +cargo fmt --all -- --check +cargo clippy --locked -p celox-sv-analyzer -p celox-frontend-sv --all-targets --all-features -- -D warnings +cargo test --locked -p celox-sv-analyzer -p celox-frontend-sv --all-features +cargo test --locked -p celox --features sv-dpi --test systemverilog --test sv_unpacked_array --test sv_dpi +cargo doc --locked -p celox-sv-analyzer -p celox-frontend-sv --no-deps +``` From 3665f501e2e9d27762ca3789ef5a4c74538836f6 Mon Sep 17 00:00:00 2001 From: tignear Date: Thu, 8 Oct 2026 11:22:03 +0900 Subject: [PATCH 5/5] fix(docs): link SV scaling probe to repository source --- docs/internals/sv-analysis-performance.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/internals/sv-analysis-performance.md b/docs/internals/sv-analysis-performance.md index 02747f9415..1f11f34073 100644 --- a/docs/internals/sv-analysis-performance.md +++ b/docs/internals/sv-analysis-performance.md @@ -14,7 +14,7 @@ Optional integer arguments replace the default counts. Flat designs default to 128, 512, and 2,048 signals/processes; hierarchy measurements default to 8, 32, and 128 modules. Each reported phase is the median of three fresh runs. Inputs and output counts are checked; wall-clock thresholds are deliberately absent. -The [probe source](../../crates/celox-sv-analyzer/examples/scaling.rs) contains +The [probe source](https://github.com/celox-sim/celox/blob/740f969402818f7d2552d0991d5c96247c456533/crates/celox-sv-analyzer/examples/scaling.rs) contains the generators and measurement boundaries. ## Changed algorithms @@ -128,4 +128,5 @@ cargo clippy --locked -p celox-sv-analyzer -p celox-frontend-sv --all-targets -- cargo test --locked -p celox-sv-analyzer -p celox-frontend-sv --all-features cargo test --locked -p celox --features sv-dpi --test systemverilog --test sv_unpacked_array --test sv_dpi cargo doc --locked -p celox-sv-analyzer -p celox-frontend-sv --no-deps +pnpm docs:build ```