diff --git a/crates/compiler/src/drop_insert.rs b/crates/compiler/src/drop_insert.rs index 12174862..90bd398d 100644 --- a/crates/compiler/src/drop_insert.rs +++ b/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| + -> 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 = 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 = 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 = 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. diff --git a/crates/compiler/tests/owned_phi_release.rs b/crates/compiler/tests/owned_phi_release.rs index b068d00d..53fe3e5d 100644 --- a/crates/compiler/tests/owned_phi_release.rs +++ b/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 { + 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]