Skip to content

Commit

Permalink
Additional RIR block remapping, Error string updates (#1526)
Browse files Browse the repository at this point in the history
This change fixes another issue in block id remapping in RIR that can
lead to degenerate performance cases. The corresponding test case is
updated to exercise deeper nesting and verify the fix.

This also updates the error string used for dynamic values encountered
by partial evaluation based on feedback.
  • Loading branch information
swernli authored May 15, 2024
1 parent 765ad49 commit c0811d2
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 9 deletions.
6 changes: 2 additions & 4 deletions compiler/qsc_partial_eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,9 @@ pub enum Error {
#[diagnostic(transparent)]
CapabilityError(CapabilityError),

#[error("use of unanalyzed dynamic value")]
#[error("cannot use a dynamic value returned from a runtime-resolved callable")]
#[diagnostic(code("Qsc.PartialEval.UnexpectedDynamicValue"))]
#[diagnostic(help(
"analysis is limited for callables that cannot be uniquely resolved at compile time, try invoking the desired callable directly"
))]
#[diagnostic(help("try invoking the desired callable directly"))]
UnexpectedDynamicValue(#[label] Span),

#[error("partial evaluation failed with error: {0}")]
Expand Down
10 changes: 6 additions & 4 deletions compiler/qsc_partial_eval/tests/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,10 @@ fn large_loop_with_inner_if_completes_eval_and_transform() {
use q = Qubit();
mutable i = 0;
for idx in 0..99 {
if MResetZ(q) == One {
set i += 1;
if i == 0 {
if MResetZ(q) == One {
set i += 1;
}
}
}
return i;
Expand All @@ -362,10 +364,10 @@ fn large_loop_with_inner_if_completes_eval_and_transform() {

// Program is expected to be too large to reasonably print out here, so instead verify the last block
// and the total number of blocks.
assert_eq!(program.blocks.iter().count(), 201);
assert_eq!(program.blocks.iter().count(), 399);
assert_block_instructions(
&program,
BlockId(199),
BlockId(395),
&expect![[r#"
Block:
Variable(1, Integer) = Store Integer(100)
Expand Down
6 changes: 5 additions & 1 deletion compiler/qsc_rir/src/passes/remap_block_ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn remap_block_ids(program: &mut Program) {
// This effectively remaps all the blocks in the list and updates the mapped id of the current block.
// This is only safe without cycles, so on a cyclic graph the node is skipped and not remapped.
if is_acyclic {
block_id_map.retain_mut(|id| *id != block_id);
block_id_map.retain(|id| *id != block_id);
} else if block_id_map.contains(&block_id) {
continue;
}
Expand All @@ -54,6 +54,10 @@ pub fn remap_block_ids(program: &mut Program) {
// the same blocks back-to-back.
continue;
}
// Since we are going to extend the blocks to visit using the successors of the current block, we can remove them from
// anywhere else in the list to visit so we avoid visiting them multiple times (only the last visit to a block is
// significant, so others can be skipped).
blocks_to_visit.retain(|id| !successors.contains(id));
blocks_to_visit.extend(successors);
}

Expand Down

0 comments on commit c0811d2

Please sign in to comment.