Skip to content

Commit

Permalink
Use dense FIR IDs across packages (#1296)
Browse files Browse the repository at this point in the history
This change fixes three things:
- It reduces the memory footprint by using dense FIR IDs across
packages. This is accomplished by using a new lowerer instance for every
package such that IDs for blocks, statements, expressions and pats are
reset and start from zero.
- It fixes a bug where the debugger would not honor a breakpoint if the
statement that mapped to the breakpoint had the ID 0.
- As a consequence of changing the way the FIR lowerer is used, the
debugger would now randomly and inadvertently break into a core or
standard library statement if the statement ID happened to be the same
than the statement ID where a breakpoint is set in the user's code. This
problem is also addressed by this change.

This was done in collaboration with @swernli and @idavis.

---------

Co-authored-by: Stefan J. Wernli <[email protected]>
  • Loading branch information
cesarzc and swernli authored Mar 28, 2024
1 parent 15c26c4 commit 38a0d44
Show file tree
Hide file tree
Showing 8 changed files with 317 additions and 330 deletions.
12 changes: 4 additions & 8 deletions compiler/qsc/benches/rca.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,26 +134,22 @@ impl Default for CompilationContext {
LanguageFeatures::default(),
)
.expect("should be able to create a new compiler");
let mut lowerer = Lowerer::new();
let fir_store = lower_hir_package_store(&mut lowerer, compiler.package_store());
let fir_store = lower_hir_package_store(compiler.package_store());
Self {
compiler,
lowerer,
lowerer: Lowerer::new(),
fir_store,
compute_properties: None,
}
}
}

fn lower_hir_package_store(
lowerer: &mut Lowerer,
hir_package_store: &HirPackageStore,
) -> PackageStore {
fn lower_hir_package_store(hir_package_store: &HirPackageStore) -> PackageStore {
let mut fir_store = PackageStore::new();
for (id, unit) in hir_package_store {
fir_store.insert(
map_hir_package_to_fir(id),
lowerer.lower_package(&unit.package),
Lowerer::new().lower_package(&unit.package),
);
}
fir_store
Expand Down
9 changes: 3 additions & 6 deletions compiler/qsc/src/interpret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,28 +145,25 @@ impl Interpreter {
capabilities: RuntimeCapabilityFlags,
language_features: LanguageFeatures,
) -> std::result::Result<Self, Vec<Error>> {
let mut lowerer = qsc_eval::lower::Lowerer::new();
let mut fir_store = fir::PackageStore::new();

let compiler = Compiler::new(std, sources, package_type, capabilities, language_features)
.map_err(into_errors)?;

let mut fir_store = fir::PackageStore::new();
for (id, unit) in compiler.package_store() {
fir_store.insert(
map_hir_package_to_fir(id),
lowerer.lower_package(&unit.package),
qsc_eval::lower::Lowerer::new().lower_package(&unit.package),
);
}

let source_package_id = compiler.source_package_id();
let package_id = compiler.package_id();

Ok(Self {
compiler,
lines: 0,
capabilities,
fir_store,
lowerer,
lowerer: qsc_eval::lower::Lowerer::new(),
env: Env::default(),
sim: BackendChain::new(
SparseSim::new(),
Expand Down
3 changes: 1 addition & 2 deletions compiler/qsc_codegen/src/qir_base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,11 @@ pub fn generate_qir(
store: &PackageStore,
package: hir::PackageId,
) -> std::result::Result<String, (Error, Vec<Frame>)> {
let mut fir_lowerer = qsc_eval::lower::Lowerer::new();
let mut fir_store = fir::PackageStore::new();
for (id, unit) in store {
fir_store.insert(
map_hir_package_to_fir(id),
fir_lowerer.lower_package(&unit.package),
qsc_eval::lower::Lowerer::new().lower_package(&unit.package),
);
}

Expand Down
7 changes: 3 additions & 4 deletions compiler/qsc_eval/src/intrinsic/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,9 @@ impl Backend for CustomSim {
}

fn check_intrinsic(file: &str, expr: &str, out: &mut impl Receiver) -> Result<Value, Error> {
let mut fir_lowerer = crate::lower::Lowerer::new();
let mut core = compile::core();
run_core_passes(&mut core);
let core_fir = fir_lowerer.lower_package(&core.package);
let core_fir = crate::lower::Lowerer::new().lower_package(&core.package);
let mut store = PackageStore::new(core);

let mut std = compile::std(&store, RuntimeCapabilityFlags::all());
Expand All @@ -161,7 +160,7 @@ fn check_intrinsic(file: &str, expr: &str, out: &mut impl Receiver) -> Result<Va
RuntimeCapabilityFlags::all()
)
.is_empty());
let std_fir = fir_lowerer.lower_package(&std.package);
let std_fir = crate::lower::Lowerer::new().lower_package(&std.package);
let std_id = store.insert(std);

let sources = SourceMap::new([("test".into(), file.into())], Some(expr.into()));
Expand All @@ -180,7 +179,7 @@ fn check_intrinsic(file: &str, expr: &str, out: &mut impl Receiver) -> Result<Va
RuntimeCapabilityFlags::all()
)
.is_empty());
let unit_fir = fir_lowerer.lower_package(&unit.package);
let unit_fir = crate::lower::Lowerer::new().lower_package(&unit.package);
let entry = unit_fir.entry_exec_graph.clone();

let id = store.insert(unit);
Expand Down
7 changes: 6 additions & 1 deletion compiler/qsc_eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ pub struct State {
idx_stack: Vec<u32>,
val_register: Option<Value>,
val_stack: Vec<Vec<Value>>,
source_package: PackageId,
package: PackageId,
call_stack: CallStack,
current_span: Span,
Expand All @@ -429,6 +430,7 @@ impl State {
idx_stack: Vec::new(),
val_register: None,
val_stack: vec![Vec::new()],
source_package: package,
package,
call_stack: CallStack::default(),
current_span: Span::default(),
Expand Down Expand Up @@ -547,7 +549,10 @@ impl State {
self.idx += 1;
self.current_span = globals.get_stmt((self.package, *stmt).into()).span;

if let Some(bp) = breakpoints.iter().find(|&bp| *bp == *stmt) {
if let Some(bp) = breakpoints
.iter()
.find(|&bp| *bp == *stmt && self.package == self.source_package)
{
StepResult::BreakpointHit(*bp)
} else {
if self.current_span == Span::default() {
Expand Down
Loading

0 comments on commit 38a0d44

Please sign in to comment.