Drop insertion: resolve the release of a loop-carried pair of phis

crates/compiler/src/drop_insert.rs

@@ -748,7 +748,32 @@ fn release_owned_phis(func: &mut HirFunction, facts: &ModuleFacts) -> usize {
         }
 
         // What releases each owning phi's value: what releases what reaches
-        // it, agreed on by every incoming or the phi is not owned.
+        // it, agreed on by every incoming or the phi is not owned. A
+        // loop-carried pair of phis names each other, so an incoming that
+        // is a candidate still undecided does not hold a phi back; every
+        // decision is then checked against all incomings and withdrawn
+        // until the decisions agree.
+        let incoming_release = |phi: &crate::hir::HirPhi,
+                                val: HirId,
+                                releases: &std::collections::HashMap<HirId, Release>|
+         -> IncomingRelease {
+            if val == phi.result || is_null_value(func, val) {
+                IncomingRelease::Nothing
+            } else if let Some(r) = sites.get(&val) {
+                IncomingRelease::Known(*r)
+            } else if let Some(r) = releases.get(&val) {
+                IncomingRelease::Known(*r)
+            } else if copies
+                .iter()
+                .any(|(_, p, _, v)| *p == phi.result && *v == val)
+            {
+                IncomingRelease::Known(Release::Symbol(STRING_FREE))
+            } else if candidates.contains(&val) {
+                IncomingRelease::Undecided
+            } else {
+                IncomingRelease::Unknown
+            }
+        };
         let mut releases: std::collections::HashMap<HirId, Release> =
             std::collections::HashMap::new();
         for _ in 0..candidates.len() {
@@ -760,20 +785,13 @@ fn release_owned_phis(func: &mut HirFunction, facts: &ModuleFacts) -> usize {
                     let mut agreed: Option<Release> = None;
                     let mut known = true;
                     for (val, _) in &phi.incoming {
-                        let r = if *val == phi.result || is_null_value(func, *val) {
-                            continue;
-                        } else if let Some(r) = sites.get(val) {
-                            *r
-                        } else if let Some(r) = releases.get(val) {
-                            *r
-                        } else if copies
-                            .iter()
-                            .any(|(_, p, _, v)| *p == phi.result && *v == *val)
-                        {
-                            Release::Symbol(STRING_FREE)
-                        } else {
-                            known = false;
-                            break;
+                        let r = match incoming_release(phi, *val, &releases) {
+                            IncomingRelease::Nothing | IncomingRelease::Undecided => continue,
+                            IncomingRelease::Known(r) => r,
+                            IncomingRelease::Unknown => {
+                                known = false;
+                                break;
+                            }
                         };
                         if agreed.is_none_or(|have| have == r) {
                             agreed = Some(r);
@@ -788,6 +806,32 @@ fn release_owned_phis(func: &mut HirFunction, facts: &ModuleFacts) -> usize {
                 }
             }
         }
+        loop {
+            let mut withdrawn: Vec<HirId> = Vec::new();
+            for (_, block) in &func.blocks {
+                for phi in &block.phis {
+                    let Some(own) = releases.get(&phi.result) else {
+                        continue;
+                    };
+                    let agrees = phi.incoming.iter().all(|(val, _)| {
+                        match incoming_release(phi, *val, &releases) {
+                            IncomingRelease::Nothing => true,
+                            IncomingRelease::Known(r) => r == *own,
+                            IncomingRelease::Undecided | IncomingRelease::Unknown => false,
+                        }
+                    });
+                    if !agrees {
+                        withdrawn.push(phi.result);
+                    }
+                }
+            }
+            if withdrawn.is_empty() {
+                break;
+            }
+            for p in withdrawn {
+                releases.remove(&p);
+            }
+        }
         let before = candidates.len();
         candidates.retain(|p| releases.contains_key(p));
         if candidates.len() == before {
@@ -871,6 +915,18 @@ fn release_owned_phis(func: &mut HirFunction, facts: &ModuleFacts) -> usize {
     inserted
 }
 
+/// What releases one incoming of a phi, while the phis' releases are
+/// being decided.
+enum IncomingRelease {
+    /// The phi itself or a null: nothing arrives to release.
+    Nothing,
+    Known(Release),
+    /// Another candidate phi whose release is not decided yet.
+    Undecided,
+    /// Storage no release is known for.
+    Unknown,
+}
+
 /// Whether every incoming of `phi` is storage the phi may own, given the
 /// sites and the phis still candidates. Returns the string seeds to copy
 /// on their entry edges, or `None` where an incoming fails.

crates/compiler/tests/owned_phi_release.rs

@@ -125,6 +125,12 @@ fn store(value: HirId, ptr: HirId) -> HirInstruction {
 /// exit:   return *t
 /// ```
 fn build_module() -> (HirModule, HirId) {
+    let (module, id, _) = build_module_with_values();
+    (module, id)
+}
+
+/// The module, the function and the values `a`, `t` and `tn`.
+fn build_module_with_values() -> (HirModule, HirId, [HirId; 3]) {
     let mut f = HirFunction::new(InternedString::new_global("keep_largest"), sig());
     let entry = f.entry_block;
     let header = f.create_block();
@@ -267,7 +273,38 @@ fn build_module() -> (HirModule, HirId) {
     module.automatic_release = true;
     module.functions.insert(id, f);
     zyntax_compiler::drop_insert::run_module(&mut module);
-    (module, id)
+    (module, id, [a, t, tn])
+}
+
+/// The values the function frees.
+fn freed(module: &HirModule, id: HirId) -> HashSet<HirId> {
+    module.functions[&id]
+        .blocks
+        .values()
+        .flat_map(|b| b.instructions.iter())
+        .filter_map(|inst| match inst {
+            HirInstruction::Call {
+                callee: HirCallable::Intrinsic(Intrinsic::Free),
+                args,
+                ..
+            } => args.first().copied(),
+            _ => None,
+        })
+        .collect()
+}
+
+#[test]
+fn the_running_value_and_the_candidate_are_released() {
+    let (module, id, [a, t, tn]) = build_module_with_values();
+    let freed = freed(&module, id);
+    assert!(
+        freed.contains(&a),
+        "the candidate is never freed: {freed:?}"
+    );
+    assert!(
+        freed.contains(&t) || freed.contains(&tn),
+        "the running value is never freed: {freed:?}"
+    );
 }
 
 #[test]