Skip to content

Commit 8e6217b

Browse files
committed
fix(analyzer): preserve sparse whole-port elements
1 parent 82ebf94 commit 8e6217b

2 files changed

Lines changed: 113 additions & 2 deletions

File tree

crates/analyzer/src/comb_loop_detect.rs

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,8 @@ fn add_inst_feedthrough_edges(
450450
parent_vars: &HashMap<VarId, Variable>,
451451
ctx: &mut Context,
452452
) {
453+
add_sparse_whole_port_copy_edges(inst, child, bit_part, graph, node_map, parent_vars);
454+
453455
let mut input_reads: HashMap<VarId, Vec<NodeKey>> = HashMap::default();
454456
for inp in &inst.inputs {
455457
if !is_pure_input_or_output(inp.id, &child.variables, Direction::Input) {
@@ -498,6 +500,111 @@ fn add_inst_feedthrough_edges(
498500
}
499501
}
500502

503+
fn add_sparse_whole_port_copy_edges(
504+
inst: &InstDeclaration,
505+
child: &Module,
506+
bit_part: &BitPartition,
507+
graph: &mut Graph<NodeKey, ()>,
508+
node_map: &mut HashMap<NodeKey, NodeIndex>,
509+
parent_vars: &HashMap<VarId, Variable>,
510+
) {
511+
for declaration in &child.declarations {
512+
let Declaration::Comb(comb) = declaration else {
513+
continue;
514+
};
515+
let [Statement::Assign(assign)] = comb.statements.as_slice() else {
516+
continue;
517+
};
518+
let [destination] = assign.dst.as_slice() else {
519+
continue;
520+
};
521+
if !destination.index.0.is_empty()
522+
|| !destination.select.is_empty()
523+
|| !is_pure_input_or_output(destination.id, &child.variables, Direction::Output)
524+
{
525+
continue;
526+
}
527+
let Expression::Term(factor) = &assign.expr else {
528+
continue;
529+
};
530+
let Factor::Variable(input_id, input_index, input_select, _) = factor.as_ref() else {
531+
continue;
532+
};
533+
if !input_index.0.is_empty()
534+
|| !input_select.is_empty()
535+
|| !is_pure_input_or_output(*input_id, &child.variables, Direction::Input)
536+
{
537+
continue;
538+
}
539+
540+
let Some(input) = inst.inputs.iter().find(|input| input.id == *input_id) else {
541+
continue;
542+
};
543+
let Expression::Term(input_factor) = &input.expr else {
544+
continue;
545+
};
546+
let Factor::Variable(parent_input, parent_input_index, parent_input_select, _) =
547+
input_factor.as_ref()
548+
else {
549+
continue;
550+
};
551+
if !parent_input_index.0.is_empty() || !parent_input_select.is_empty() {
552+
continue;
553+
}
554+
555+
let Some(output) = inst
556+
.outputs
557+
.iter()
558+
.find(|output| output.id == destination.id)
559+
else {
560+
continue;
561+
};
562+
let [parent_destination] = output.dst.as_slice() else {
563+
continue;
564+
};
565+
if !parent_destination.index.0.is_empty() || !parent_destination.select.is_empty() {
566+
continue;
567+
}
568+
let parent_output = parent_destination.id;
569+
570+
let Some(child_input) = child.variables.get(input_id) else {
571+
continue;
572+
};
573+
let Some(child_output) = child.variables.get(&destination.id) else {
574+
continue;
575+
};
576+
let Some(parent_input_variable) = parent_vars.get(parent_input) else {
577+
continue;
578+
};
579+
let Some(parent_output_variable) = parent_vars.get(&parent_output) else {
580+
continue;
581+
};
582+
if child_input.total_width() != child_output.total_width()
583+
|| child_input.r#type.total_array() != child_output.r#type.total_array()
584+
|| parent_input_variable.total_width() != parent_output_variable.total_width()
585+
|| parent_input_variable.r#type.total_array()
586+
!= parent_output_variable.r#type.total_array()
587+
{
588+
continue;
589+
}
590+
591+
for ((object, index), ranges) in &bit_part.ranges {
592+
if *object != parent_output {
593+
continue;
594+
}
595+
for (destination_range, mask) in ranges.iter().enumerate() {
596+
let destination_key = (parent_output, *index, destination_range);
597+
for source_range in bit_part.overlapping((*parent_input, *index), mask) {
598+
let source_key = (*parent_input, *index, source_range);
599+
let source = ensure_node(graph, node_map, source_key);
600+
let destination = ensure_node(graph, node_map, destination_key);
601+
graph.add_edge(source, destination, ());
602+
}
603+
}
604+
}
605+
}
606+
}
607+
501608
fn is_pure_input_or_output(id: VarId, vars: &HashMap<VarId, Variable>, want: Direction) -> bool {
502609
let Some(v) = vars.get(&id) else { return false };
503610
use crate::ir::VarKind;
@@ -628,6 +735,11 @@ fn build_error(module: &Module, keys: &[NodeKey]) -> Option<AnalyzerError> {
628735
}
629736
if let Some(toks) = module.assign_tokens.get(id) {
630737
tokens.extend(toks.iter().copied());
738+
} else if let Some(variable) = module.variables.get(id) {
739+
// Assignment coverage intentionally omits oversized arrays. Keep
740+
// a usable diagnostic site when the sparse graph still proves a
741+
// cycle through one of those variables.
742+
tokens.push(variable.token);
631743
}
632744
}
633745
{

crates/analyzer/src/tests/comb_loop_module_tests.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,8 @@ fn assert_unaligned_unpacked_instance_input(target: usize, expected: bool) {
6767
);
6868
}
6969

70-
comb_loop_case_ignored!(
70+
comb_loop_case!(
7171
comb_loop_whole_unpacked_matching_element_retains_feedback,
72-
"comb-loop migration: false negative; module feedthrough and instance mapping",
7372
"a distant matching element retains feedback",
7473
whole_unpacked_instance_code(123_456),
7574
true

0 commit comments

Comments
 (0)