Skip to content

Commit d9692dd

Browse files
authored
fix: reuse generated operation functions (#1995)
## Summary - make cached `OpFunctionMap` definitions public while HUGR linking runs - reuse `NodeTemplate::call_to_function` so `OnMultiDefn::UseTarget` coalesces repeated definitions - rely on the existing qsystem hiding phase to restore generated helpers to private visibility - add generic and wrapped-barrier regressions asserting repeated operations share one definition ## Root cause `OpFunctionMap` cached one generated HUGR per operation shape, but its replacement template embedded a private function HUGR at every call site. Name linking only coalesces public definitions, so `OnMultiDefn::UseTarget` had no effect and repeated barriers produced thousands of equivalent wrapper functions. Wide wrappers then amplified LLVM IPSCCP memory use. The fix belongs in `func_as_node_template`: cached helpers are now linker-visible when passed through the existing `call_to_function` path. `QSystemRebasePass` already records pre-existing public functions and hides newly introduced helpers after lowering, so the final HUGR retains private linkage without adding a parallel call-template mechanism. ## Validation - `cargo check -p tket -p tket-qsystem` - `cargo test -p tket --lib` (604 passed, 2 ignored) - `cargo test -p tket-qsystem --lib` (167 passed, 1 ignored) - `just check`: formatting, Ruff, mypy, Cargo check/docs, and pg-libs tests passed; the full local gate remains blocked by unrelated existing nightly Clippy findings and macOS hugrenv `@rpath` resolution in aggregate nextest/Python discovery - `git diff --check origin/main...HEAD`
1 parent be362a3 commit d9692dd

3 files changed

Lines changed: 99 additions & 35 deletions

File tree

‎tket-qsystem/src/extension/qsystem/barrier.rs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,29 @@ mod test {
139139
}
140140
}
141141
}
142+
143+
#[test]
144+
fn repeated_barriers_share_wrapped_function() {
145+
let mut b =
146+
DFGBuilder::new(hugr::types::Signature::new_endo(vec![qb_t(), qb_t()])).unwrap();
147+
let first = b.add_barrier(b.input_wires()).unwrap();
148+
let second = b.add_barrier(first.outputs()).unwrap();
149+
let mut h = b.finish_hugr_with_outputs(second.outputs()).unwrap();
150+
151+
lower_tk2_ops(&mut h, Preserve::Public, QSystemPlatform::Helios).unwrap();
152+
h.validate().unwrap();
153+
154+
let wrapped_function = h
155+
.children(h.module_root())
156+
.filter(|&node| {
157+
h.get_optype(node).as_func_defn().is_some_and(|op| {
158+
op.func_name()
159+
.contains(wrapped_barrier::WRAPPED_BARRIER_NAME.as_str())
160+
})
161+
})
162+
.exactly_one()
163+
.ok()
164+
.expect("identical barriers should share one wrapped function");
165+
assert_eq!(h.output_neighbours(wrapped_function).count(), 2);
166+
}
142167
}

‎tket/src/passes/utils/unpack_container.rs‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,13 +541,15 @@ mod tests {
541541
use super::*;
542542
use hugr::{
543543
HugrView,
544-
builder::{DFGBuilder, DataflowHugr as _},
544+
builder::{DFGBuilder, DataflowHugr as _, FunctionBuilder},
545545
extension::prelude::{bool_t, option_type, qb_t, usize_t},
546546
std_extensions::collections::array::array_type,
547547
types::Signature,
548548
};
549549
use rstest::rstest;
550550

551+
use crate::passes::{ComposablePass, ReplaceTypes};
552+
551553
#[test]
552554
fn test_container_factory_creation() {
553555
let analyzer = TypeUnpacker::for_qubits();
@@ -571,6 +573,52 @@ mod tests {
571573
Ok(())
572574
}
573575

576+
#[test]
577+
fn repeated_ops_share_lowering_functions() -> Result<(), BuildError> {
578+
let factory = UnpackContainerBuilder::new(TypeUnpacker::for_qubits());
579+
let option_qb_type = Type::from(option_type([qb_t()]));
580+
let option_usize_type = Type::from(option_type([usize_t()]));
581+
let mut builder = FunctionBuilder::new(
582+
"main",
583+
Signature::new_endo([option_qb_type, option_usize_type]),
584+
)?;
585+
586+
let qb_input = builder.input().out_wire(0);
587+
let unwrapped = factory.unpack_option(&mut builder, qb_input, &qb_t())?;
588+
let wrapped = factory.repack_option(&mut builder, unwrapped, &qb_t())?;
589+
let unwrapped = factory.unpack_option(&mut builder, wrapped, &qb_t())?;
590+
let qb_output = factory.repack_option(&mut builder, unwrapped, &qb_t())?;
591+
592+
let usize_input = builder.input().out_wire(1);
593+
let unwrapped = factory.unpack_option(&mut builder, usize_input, &usize_t())?;
594+
let usize_output = factory.repack_option(&mut builder, unwrapped, &usize_t())?;
595+
let mut hugr = builder.finish_hugr_with_outputs([qb_output, usize_output])?;
596+
597+
let mut lowerer = ReplaceTypes::new_empty();
598+
factory
599+
.into_function_map()
600+
.register_operation_replacements(&mut hugr, &mut lowerer);
601+
lowerer.run(&mut hugr).unwrap();
602+
hugr.validate().unwrap();
603+
604+
let lowering_functions = hugr
605+
.children(hugr.module_root())
606+
.filter(|&node| {
607+
hugr.get_optype(node)
608+
.as_func_defn()
609+
.is_some_and(|op| op.func_name() != "main")
610+
})
611+
.collect::<Vec<_>>();
612+
assert_eq!(lowering_functions.len(), 4);
613+
let mut use_counts = lowering_functions
614+
.iter()
615+
.map(|&function| hugr.output_neighbours(function).count())
616+
.collect::<Vec<_>>();
617+
use_counts.sort_unstable();
618+
assert_eq!(use_counts, [1, 1, 2, 2]);
619+
Ok(())
620+
}
621+
574622
#[rstest]
575623
#[case::array(Array, ARRAY_UNPACK, ARRAY_REPACK)]
576624
#[case::borrow_array(BorrowArray, BARRAY_UNPACK, BARRAY_REPACK)]

‎tket/src/passes/utils/unpack_container/op_function_map.rs‎

Lines changed: 25 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,15 @@
33
use crate::passes::monomorphize::mangle_name;
44
use crate::passes::{ReplaceTypes, replace_types::NodeTemplate};
55
use hugr::HugrView;
6-
use hugr::builder::{Container, Dataflow, HugrBuilder};
7-
use hugr::hugr::linking::OnMultiDefn;
8-
use hugr::ops::handle::{FuncID, NodeHandle};
96
use hugr::{
107
Hugr, Node, Wire,
118
builder::{BuildError, DataflowHugr, FunctionBuilder},
129
hugr::hugrmut::HugrMut,
13-
ops::{DataflowOpTrait, ExtensionOp},
10+
ops::{DataflowOpTrait, ExtensionOp, OpType},
1411
types::TypeArg,
1512
};
1613
use hugr_core::Visibility;
17-
use hugr_core::hugr::linking::NameLinkingPolicy;
14+
use hugr_core::hugr::internal::HugrMutInternals;
1815
use indexmap::IndexMap;
1916
use std::{cell::RefCell, ops::Deref};
2017

@@ -79,7 +76,16 @@ impl OpFunctionMap {
7976
if self.map.borrow().contains_key(&key) {
8077
return Ok(());
8178
}
82-
let name = mangle_name(op.def().name(), mangle_args);
79+
// Definitions are made public while name linking deduplicates them, so
80+
// the symbol must distinguish every cached operation instance. Keep
81+
// caller-selected arguments for readability, and append the complete
82+
// operation arguments to guarantee uniqueness.
83+
let name_args = mangle_args
84+
.iter()
85+
.chain(op.args())
86+
.cloned()
87+
.collect::<Vec<_>>();
88+
let name = mangle_name(op.def().name(), name_args);
8389
let sig = op.signature().deref().clone();
8490
let mut func_b = FunctionBuilder::new(name, sig)?;
8591
// insert None as a placeholder to avoid cyclic recursion in func_builder call
@@ -122,42 +128,27 @@ impl OpFunctionMap {
122128
_hugr: &mut impl HugrMut<Node = Node>,
123129
lowerer: &mut ReplaceTypes,
124130
) {
125-
// Use the centralized cache for all operation replacements
126131
for (op, func_def) in self.into_function_iter() {
127132
lowerer.set_replace_op(&op, func_as_node_template(func_def));
128133
}
129134
}
130135
}
131136

137+
/// Given a HUGR with a function definition as entrypoint, constructs a
138+
/// [`NodeTemplate::LinkedHugr`] that produces a call to the function.
139+
fn func_as_node_template(mut func_def: Hugr) -> NodeTemplate {
140+
let entrypoint = func_def.entrypoint();
141+
let OpType::FuncDefn(func) = func_def.optype_mut(entrypoint) else {
142+
panic!("OpFunctionMap entries must be function definitions");
143+
};
144+
*func.visibility_mut() = Visibility::Public;
145+
146+
NodeTemplate::call_to_function(func_def, &[])
147+
.expect("OpFunctionMap entries must be monomorphic function definitions")
148+
}
149+
132150
impl Default for OpFunctionMap {
133151
fn default() -> Self {
134152
Self::new()
135153
}
136154
}
137-
138-
/// Given a hugr with a function definition as entrypoint, constructs a
139-
/// [`NodeTemplate::LinkedHugr`] that produces a call to the function.
140-
//
141-
// TODO: Use [`NodeTemplate::call_to_function`] once it gets released in `hugr 0.25.6`.
142-
fn func_as_node_template(func_def: Hugr) -> NodeTemplate {
143-
// Create a replacement hugr for the op nodes: Add a `call` node in the `func_def` hugr and set it as entrypoint.
144-
let func_signature = func_def.inner_function_type().unwrap().into_owned();
145-
146-
// Build a new hugr and insert the function definition into it
147-
let mut b = FunctionBuilder::new_vis("", func_signature, Visibility::Private).unwrap();
148-
let func_id = FuncID::<true>::from(
149-
b.module_root_builder()
150-
.add_hugr(func_def)
151-
.inserted_entrypoint,
152-
);
153-
154-
// Build a call to the function in the new separate function.
155-
let call = b.call(&func_id, &[], b.input_wires()).unwrap();
156-
let mut call_hugr = b.finish_hugr_with_outputs(call.outputs()).unwrap();
157-
call_hugr.set_entrypoint(call.node());
158-
159-
NodeTemplate::LinkedHugr(
160-
Box::new(call_hugr),
161-
NameLinkingPolicy::default().on_multiple_defn(OnMultiDefn::UseTarget),
162-
)
163-
}

0 commit comments

Comments
 (0)