perf(analyzer): analyze instantiated modules once

Why: the comb-loop pass visited every source template before recursively rebuilding concrete children for parent summaries. On the aggregate analyze benchmark, that repeated graph construction dominated the new post-pass and triggered CodSpeed's 5.87% regression. Start from source roots, use each concrete child graph for both its diagnostics and parent summary, and retain fallback analysis for uninstantiated modules and every distinct generic specialization.

Author: tignear

crates/analyzer/src/comb_loop_detect.rs

@@ -45,10 +45,13 @@
 //!    spans and regular repeated copies have symbolic nodes, so graph size is
 //!    normally a function of accesses and boundaries rather than declared bit
 //!    width or unpacked-array length.
-//! 3. Child modules are analyzed before parents. A module summary contains only
-//!    input-to-output feedthrough proven in the child graph. Instance actuals
-//!    project the child's regions back into the parent, preserving bit
-//!    positions, aggregate layout, and Cartesian repetition axes when that
+//! 3. Traversal starts at source modules which are not instantiated by another
+//!    source template. Concrete child modules are analyzed before their parent;
+//!    the same child graph emits definition-local diagnostics and produces its
+//!    input-to-output summary, so the standalone template is not rebuilt.
+//!    Uninstantiated modules are roots and remain independently checked.
+//!    Instance actuals project child regions back into the parent, preserving
+//!    bit positions, aggregate layout, and Cartesian repetition axes when that
 //!    mapping is representable.
 //! 4. Iterative SCC discovery finds cyclic components without using the process
 //!    stack. For each SCC, diagnostics choose a stable source-backed edge and
@@ -456,6 +459,7 @@ enum ComponentSummaryKey {
 struct CombSummaryCache {
     modules: HashMap<ComponentSummaryKey, ModuleCombSummary>,
     procedures: crate::comb_memory_ssa::ProcedureSummaryCache,
+    analyzed_templates: HashSet<StrId>,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
@@ -527,12 +531,21 @@ fn analyze(ir: &Ir, collect_coverage: bool) -> (CombAnalysisResult, Vec<Analyzer
     let order = module_analysis_order(ir);
     for &idx in &order {
         if let Component::Module(module) = &ir.components[idx] {
+            // A concrete instance analysis already checked this source
+            // template and produced the summary its parent consumes. Generic
+            // specializations are still all visited by
+            // `ensure_instance_summaries`; this skips only the redundant
+            // standalone template pass.
+            if summaries.analyzed_templates.contains(&module.name) {
+                continue;
+            }
             // Unevaluable generic params do not have a concrete module shape.
             if module.suppress_unassigned {
                 incomplete.push(IncompleteCombAnalysis {
                     module: module.name.to_string(),
                     reasons: [IncompleteReason::UnevaluatedGeneric].into(),
                 });
+                summaries.analyzed_templates.insert(module.name);
                 continue;
             }
             ensure_instance_summaries(
@@ -552,6 +565,7 @@ fn analyze(ir: &Ir, collect_coverage: bool) -> (CombAnalysisResult, Vec<Analyzer
             );
             extend_unique_errors(&mut coverage, module_coverage);
             check_graph(module, &graph, &mut loops);
+            summaries.analyzed_templates.insert(module.name);
             if !reasons.is_empty() {
                 incomplete.push(IncompleteCombAnalysis {
                     module: module.name.to_string(),
@@ -648,6 +662,7 @@ fn ensure_instance_summaries(
         );
         extend_unique_errors(coverage, child_coverage);
         check_graph(child, &graph, loops);
+        summaries.analyzed_templates.insert(child.name);
         let summary = compute_module_summary(child, &graph, &bit_part);
         summaries.modules.insert(key.clone(), summary);
         if !reasons.is_empty() {
@@ -695,7 +710,24 @@ fn module_analysis_order(ir: &Ir) -> Vec<usize> {
         .iter()
         .map(|component| matches!(component, Component::Module(_)))
         .collect::<Vec<_>>();
-    order_from_dependencies(&is_module, &deps, &rev_deps)
+    let order = order_from_dependencies(&is_module, &deps, &rev_deps);
+    prioritize_module_roots(order, &rev_deps)
+}
+
+fn prioritize_module_roots(
+    order: Vec<usize>,
+    reverse_dependencies: &[HashSet<usize>],
+) -> Vec<usize> {
+    // Start from modules which are not instantiated by another source
+    // template. Their recursive summary walk checks concrete children before
+    // those children's redundant standalone entries are encountered. Keep
+    // the previous deterministic order within both groups; a name-cycle has
+    // no root and remains in the fallback group.
+    let (mut roots, remaining): (Vec<_>, Vec<_>) = order
+        .into_iter()
+        .partition(|index| reverse_dependencies[*index].is_empty());
+    roots.extend(remaining);
+    roots
 }
 
 fn order_from_dependencies(
@@ -3501,6 +3533,12 @@ mod memory_ssa_tests {
 
         let order = order_from_dependencies(&is_module, &deps, &rev_deps);
         assert_eq!(order, vec![1, 3, 0, 2]);
+
+        // Why this case exists: concrete instance summaries are discovered
+        // only while visiting a parent. Roots must therefore run before the
+        // child-first fallback order, while a source-name cycle remains in a
+        // deterministic fallback position instead of disappearing.
+        assert_eq!(prioritize_module_roots(order, &rev_deps), vec![3, 2, 1, 0]);
     }
 
     #[test]

crates/analyzer/src/tests.rs

@@ -10992,6 +10992,77 @@ fn comb_loop_distinguishes_generic_module_specializations() {
     );
 }
 
+#[test]
+fn comb_loop_analyzes_each_instantiated_generic_specialization() {
+    // Why this case exists: seeing one concrete child may suppress the
+    // redundant source-template pass, but it must not suppress a different
+    // concrete specialization. The disabled instance is intentionally first;
+    // the enabled instance still closes a real parent feedback path.
+    assert_comb_loop(
+        "each concrete generic-module specialization receives its own summary",
+        r#"
+        module Child #(
+            param ENABLE: u32 = 0,
+        )(
+            i: input  logic,
+            o: output logic,
+        ) {
+            if ENABLE :g_enabled {
+                assign o = i;
+            } else {
+                assign o = 0;
+            }
+        }
+        module Top (
+            o: output logic<2>,
+        ) {
+            var feedback: logic<2>;
+            var passed  : logic<2>;
+            inst disabled: Child #(
+                ENABLE: 0,
+            )(
+                i: feedback[0],
+                o: passed[0],
+            );
+            inst enabled: Child #(
+                ENABLE: 1,
+            )(
+                i: feedback[1],
+                o: passed[1],
+            );
+            assign feedback = passed;
+            assign o = feedback;
+        }
+        "#,
+        true,
+    );
+}
+
+#[test]
+fn comb_loop_analyzes_uninstantiated_modules() {
+    // Why this case exists: roots are analyzed first to reuse child summaries,
+    // but an uninstantiated source module is itself a root. Its internal loop
+    // remains an error even when another unrelated root is also present.
+    assert_comb_loop(
+        "an uninstantiated module retains its internal loop diagnostic",
+        r#"
+        module Uninstantiated (
+            x: output logic,
+            y: output logic,
+        ) {
+            assign x = y;
+            assign y = x;
+        }
+        module Top (
+            o: output logic,
+        ) {
+            assign o = 0;
+        }
+        "#,
+        true,
+    );
+}
+
 #[test]
 fn comb_loop_short_circuits_instance_actual_side_effects() {
     // Why this case exists: IEEE 1800-2023 11.4.11 evaluates only the selected
@@ -13413,10 +13484,11 @@ fn comb_loop_incomplete_effect_does_not_erase_proven_edges() {
 
 #[test]
 fn comb_loop_child_definition_has_one_diagnostic_across_instances() {
-    // Why this case exists: a child graph is checked once as a declaration and
-    // again while each parent specialization requests its causal summary. An
-    // internal loop belongs to the child source definition, so repeated
-    // instantiation must not repeat the same source diagnostic.
+    // Why this case exists: parent-first analysis checks a concrete child and
+    // derives its causal summary in one graph build. Repeated instances must
+    // reuse that result, while the child's port-independent internal loop is
+    // still diagnosed exactly once rather than being lost with the standalone
+    // source-template pass.
     let detailed = analyze_comb_detailed(
         r#"
         module Child (