From c0811d26de55a7d11f21c5db1127b75e337044a6 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Wed, 15 May 2024 10:45:21 -0700 Subject: [PATCH] Additional RIR block remapping, Error string updates (#1526) 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. --- compiler/qsc_partial_eval/src/lib.rs | 6 ++---- compiler/qsc_partial_eval/tests/misc.rs | 10 ++++++---- compiler/qsc_rir/src/passes/remap_block_ids.rs | 6 +++++- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/compiler/qsc_partial_eval/src/lib.rs b/compiler/qsc_partial_eval/src/lib.rs index 61bb2c0f65..4a3c2ffc89 100644 --- a/compiler/qsc_partial_eval/src/lib.rs +++ b/compiler/qsc_partial_eval/src/lib.rs @@ -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}")] diff --git a/compiler/qsc_partial_eval/tests/misc.rs b/compiler/qsc_partial_eval/tests/misc.rs index dce7ffda74..a85b6fe436 100644 --- a/compiler/qsc_partial_eval/tests/misc.rs +++ b/compiler/qsc_partial_eval/tests/misc.rs @@ -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; @@ -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) diff --git a/compiler/qsc_rir/src/passes/remap_block_ids.rs b/compiler/qsc_rir/src/passes/remap_block_ids.rs index d8bd95ffec..d4a4304f8f 100644 --- a/compiler/qsc_rir/src/passes/remap_block_ids.rs +++ b/compiler/qsc_rir/src/passes/remap_block_ids.rs @@ -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; } @@ -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); }