From f5b4cb5e7c37c940841fda6c8e62a59ac195c2a2 Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Thu, 4 Apr 2024 11:06:51 -0700 Subject: [PATCH] RIR SSA check pass (#1358) This pass checks that the RIR is in Single Static Assignment (SSA) form. The check verifies that each variable is assigned exactly once before it is used, and that phi nodes are used correctly. The change also updates how successors are calculated for branch instructions that have constant conditions, and updates the tests that used constant conditions to use dynamic bool values instead. --- compiler/qsc_rir/src/builder.rs | 20 + compiler/qsc_rir/src/passes.rs | 2 + .../src/passes/build_dominator_graph.rs | 13 +- .../src/passes/build_dominator_graph/tests.rs | 216 +++-- compiler/qsc_rir/src/passes/ssa_check.rs | 230 +++++ .../qsc_rir/src/passes/ssa_check/tests.rs | 870 ++++++++++++++++++ .../passes/unreachable_code_check/tests.rs | 37 +- compiler/qsc_rir/src/rir.rs | 14 +- compiler/qsc_rir/src/utils.rs | 12 +- 9 files changed, 1338 insertions(+), 76 deletions(-) create mode 100644 compiler/qsc_rir/src/passes/ssa_check.rs create mode 100644 compiler/qsc_rir/src/passes/ssa_check/tests.rs diff --git a/compiler/qsc_rir/src/builder.rs b/compiler/qsc_rir/src/builder.rs index 62d3fcc51d..71f9d18932 100644 --- a/compiler/qsc_rir/src/builder.rs +++ b/compiler/qsc_rir/src/builder.rs @@ -127,6 +127,24 @@ pub fn array_record_decl() -> Callable { } } +/// Creates a new program with a single, entry callable that has block 0 as its body. +#[must_use] +pub fn new_program() -> Program { + let mut program = Program::new(); + program.entry = CallableId(0); + program.callables.insert( + CallableId(0), + Callable { + name: "main".to_string(), + input_type: Vec::new(), + output_type: None, + body: Some(BlockId(0)), + call_type: CallableType::Regular, + }, + ); + program +} + #[must_use] pub fn bell_program() -> Program { let mut program = Program::default(); @@ -147,6 +165,7 @@ pub fn bell_program() -> Program { call_type: CallableType::Regular, }, ); + program.entry = CallableId(5); program.blocks.insert( BlockId(0), Block(vec![ @@ -236,6 +255,7 @@ pub fn teleport_program() -> Program { call_type: CallableType::Regular, }, ); + program.entry = CallableId(7); program.blocks.insert( BlockId(0), Block(vec![ diff --git a/compiler/qsc_rir/src/passes.rs b/compiler/qsc_rir/src/passes.rs index e19810a786..8ece8379bc 100644 --- a/compiler/qsc_rir/src/passes.rs +++ b/compiler/qsc_rir/src/passes.rs @@ -5,10 +5,12 @@ mod build_dominator_graph; mod defer_meas; mod reindex_qubits; mod remap_block_ids; +mod ssa_check; mod unreachable_code_check; pub use build_dominator_graph::build_dominator_graph; pub use defer_meas::defer_measurements; pub use reindex_qubits::reindex_qubits; pub use remap_block_ids::remap_block_ids; +pub use ssa_check::check_ssa_form; pub use unreachable_code_check::check_unreachable_code; diff --git a/compiler/qsc_rir/src/passes/build_dominator_graph.rs b/compiler/qsc_rir/src/passes/build_dominator_graph.rs index cdcb27e2ef..b2b211a0e6 100644 --- a/compiler/qsc_rir/src/passes/build_dominator_graph.rs +++ b/compiler/qsc_rir/src/passes/build_dominator_graph.rs @@ -3,10 +3,7 @@ use qsc_data_structures::index_map::IndexMap; -use crate::{ - rir::{BlockId, Program}, - utils::build_predecessors_map, -}; +use crate::rir::{BlockId, Program}; #[cfg(test)] mod tests; @@ -19,16 +16,16 @@ mod tests; /// - Blocks are assumed to be sequentially numbered starting from 0 in reverse postorder rather than depth first order. /// - Given that reversal, intersection between nodes uses the lesser of the two nodes rather than the greater. #[must_use] -pub fn build_dominator_graph(program: &Program) -> IndexMap { +pub fn build_dominator_graph( + program: &Program, + preds: &IndexMap>, +) -> IndexMap { let mut doms = IndexMap::default(); let entry_block_id = program .get_callable(program.entry) .body .expect("entry point should have a body"); - // Predecessors are needed to compute the dominator tree. - let preds = build_predecessors_map(program); - // The entry block dominates itself. doms.insert(entry_block_id, entry_block_id); diff --git a/compiler/qsc_rir/src/passes/build_dominator_graph/tests.rs b/compiler/qsc_rir/src/passes/build_dominator_graph/tests.rs index 8ebd616b1e..254562d89d 100644 --- a/compiler/qsc_rir/src/passes/build_dominator_graph/tests.rs +++ b/compiler/qsc_rir/src/passes/build_dominator_graph/tests.rs @@ -7,31 +7,17 @@ use expect_test::expect; use qsc_data_structures::index_map::IndexMap; use crate::{ + builder::new_program, passes::remap_block_ids, rir::{ - Block, BlockId, Callable, CallableId, CallableType, Instruction, Literal, Operand, Program, + Block, BlockId, Callable, CallableId, CallableType, Instruction, Operand, Program, Ty, + Variable, VariableId, }, + utils::build_predecessors_map, }; use super::build_dominator_graph; -/// Creates a new program with a single, entry callable that has block 0 as its body. -fn new_program() -> Program { - let mut program = Program::new(); - program.entry = CallableId(0); - program.callables.insert( - CallableId(0), - Callable { - name: "main".to_string(), - input_type: Vec::new(), - output_type: None, - body: Some(BlockId(0)), - call_type: CallableType::Regular, - }, - ); - program -} - fn display_dominator_graph(doms: &IndexMap) -> String { let mut result = String::new(); for (block_id, dom) in doms.iter() { @@ -43,15 +29,20 @@ fn display_dominator_graph(doms: &IndexMap) -> String { result } +fn build_doms(program: &mut Program) -> IndexMap { + remap_block_ids(program); + let preds = build_predecessors_map(program); + build_dominator_graph(program, &preds) +} + #[test] -fn test_dominator_graph_single_block_dominates_itself() { +fn dominator_graph_single_block_dominates_itself() { let mut program = new_program(); program .blocks .insert(BlockId(0), Block(vec![Instruction::Return])); - remap_block_ids(&mut program); - let doms = build_dominator_graph(&program); + let doms = build_doms(&mut program); expect![[r#" Block 0 dominated by block 0, @@ -60,7 +51,7 @@ fn test_dominator_graph_single_block_dominates_itself() { } #[test] -fn test_dominator_graph_sequential_blocks_dominated_by_predecessor() { +fn dominator_graph_sequential_blocks_dominated_by_predecessor() { let mut program = new_program(); program .blocks @@ -72,8 +63,7 @@ fn test_dominator_graph_sequential_blocks_dominated_by_predecessor() { .blocks .insert(BlockId(2), Block(vec![Instruction::Return])); - remap_block_ids(&mut program); - let doms = build_dominator_graph(&program); + let doms = build_doms(&mut program); expect![[r#" Block 0 dominated by block 0, @@ -84,15 +74,40 @@ fn test_dominator_graph_sequential_blocks_dominated_by_predecessor() { } #[test] -fn test_dominator_graph_branching_blocks_dominated_by_common_predecessor() { +fn dominator_graph_branching_blocks_dominated_by_common_predecessor() { let mut program = new_program(); - program - .blocks - .insert(BlockId(0), Block(vec![Instruction::Jump(BlockId(1))])); + program.callables.insert( + CallableId(1), + Callable { + name: "dynamic_bool".to_string(), + input_type: Vec::new(), + output_type: Some(Ty::Boolean), + body: None, + call_type: CallableType::Regular, + }, + ); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::Call( + CallableId(1), + Vec::new(), + Some(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + ), + Instruction::Jump(BlockId(1)), + ]), + ); program.blocks.insert( BlockId(1), Block(vec![Instruction::Branch( - Operand::Literal(Literal::Bool(true)), + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), BlockId(2), BlockId(3), )]), @@ -104,8 +119,7 @@ fn test_dominator_graph_branching_blocks_dominated_by_common_predecessor() { .blocks .insert(BlockId(3), Block(vec![Instruction::Return])); - remap_block_ids(&mut program); - let doms = build_dominator_graph(&program); + let doms = build_doms(&mut program); expect![[r#" Block 0 dominated by block 0, @@ -117,7 +131,7 @@ fn test_dominator_graph_branching_blocks_dominated_by_common_predecessor() { } #[test] -fn test_dominator_graph_infinite_loop() { +fn dominator_graph_infinite_loop() { let mut program = new_program(); program .blocks @@ -126,8 +140,7 @@ fn test_dominator_graph_infinite_loop() { .blocks .insert(BlockId(1), Block(vec![Instruction::Jump(BlockId(1))])); - remap_block_ids(&mut program); - let doms = build_dominator_graph(&program); + let doms = build_doms(&mut program); expect![[r#" Block 0 dominated by block 0, @@ -137,15 +150,39 @@ fn test_dominator_graph_infinite_loop() { } #[test] -fn test_dominator_graph_branch_and_loop() { +fn dominator_graph_branch_and_loop() { let mut program = new_program(); - program - .blocks - .insert(BlockId(0), Block(vec![Instruction::Jump(BlockId(1))])); + program.callables.insert( + CallableId(1), + Callable { + name: "dynamic_bool".to_string(), + input_type: Vec::new(), + output_type: Some(Ty::Boolean), + body: None, + call_type: CallableType::Regular, + }, + ); + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::Call( + CallableId(1), + Vec::new(), + Some(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + ), + Instruction::Jump(BlockId(1)), + ]), + ); program.blocks.insert( BlockId(1), Block(vec![Instruction::Branch( - Operand::Literal(Literal::Bool(true)), + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), BlockId(2), BlockId(3), )]), @@ -160,8 +197,7 @@ fn test_dominator_graph_branch_and_loop() { .blocks .insert(BlockId(4), Block(vec![Instruction::Return])); - remap_block_ids(&mut program); - let doms = build_dominator_graph(&program); + let doms = build_doms(&mut program); expect![[r#" Block 0 dominated by block 0, @@ -174,11 +210,22 @@ fn test_dominator_graph_branch_and_loop() { } #[test] -fn test_dominator_graph_complex_structure_only_dominated_by_entry() { +fn dominator_graph_complex_structure_only_dominated_by_entry() { // This example comes from the paper from [A Simple, Fast Dominance Algorithm](http://www.hipersoft.rice.edu/grads/publications/dom14.pdf) // by Cooper, Harvey, and Kennedy and uses the node numbering from the paper. However, the resulting dominator graph // is different due to the numbering of the blocks, such that each block is numbered in reverse postorder. let mut program = new_program(); + program.callables.insert( + CallableId(1), + Callable { + name: "dynamic_bool".to_string(), + input_type: Vec::new(), + output_type: Some(Ty::Boolean), + body: None, + call_type: CallableType::Regular, + }, + ); + program .callables .get_mut(CallableId(0)) @@ -186,11 +233,24 @@ fn test_dominator_graph_complex_structure_only_dominated_by_entry() { .body = Some(BlockId(6)); program.blocks.insert( BlockId(6), - Block(vec![Instruction::Branch( - Operand::Literal(Literal::Bool(true)), - BlockId(5), - BlockId(4), - )]), + Block(vec![ + Instruction::Call( + CallableId(1), + Vec::new(), + Some(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + ), + Instruction::Branch( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + BlockId(5), + BlockId(4), + ), + ]), ); program .blocks @@ -198,7 +258,10 @@ fn test_dominator_graph_complex_structure_only_dominated_by_entry() { program.blocks.insert( BlockId(4), Block(vec![Instruction::Branch( - Operand::Literal(Literal::Bool(true)), + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), BlockId(2), BlockId(3), )]), @@ -209,7 +272,10 @@ fn test_dominator_graph_complex_structure_only_dominated_by_entry() { program.blocks.insert( BlockId(2), Block(vec![Instruction::Branch( - Operand::Literal(Literal::Bool(true)), + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), BlockId(3), BlockId(1), )]), @@ -218,8 +284,7 @@ fn test_dominator_graph_complex_structure_only_dominated_by_entry() { .blocks .insert(BlockId(3), Block(vec![Instruction::Jump(BlockId(2))])); - remap_block_ids(&mut program); - let doms = build_dominator_graph(&program); + let doms = build_doms(&mut program); expect![[r#" Block 0 dominated by block 0, @@ -233,20 +298,47 @@ fn test_dominator_graph_complex_structure_only_dominated_by_entry() { } #[test] -fn test_dominator_graph_with_node_having_many_predicates() { +fn dominator_graph_with_node_having_many_predicates() { let mut program = new_program(); + program.callables.insert( + CallableId(1), + Callable { + name: "dynamic_bool".to_string(), + input_type: Vec::new(), + output_type: Some(Ty::Boolean), + body: None, + call_type: CallableType::Regular, + }, + ); + program.blocks.insert( BlockId(0), - Block(vec![Instruction::Branch( - Operand::Literal(Literal::Bool(true)), - BlockId(1), - BlockId(2), - )]), + Block(vec![ + Instruction::Call( + CallableId(1), + Vec::new(), + Some(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + ), + Instruction::Branch( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + BlockId(1), + BlockId(2), + ), + ]), ); program.blocks.insert( BlockId(1), Block(vec![Instruction::Branch( - Operand::Literal(Literal::Bool(true)), + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), BlockId(3), BlockId(4), )]), @@ -254,7 +346,10 @@ fn test_dominator_graph_with_node_having_many_predicates() { program.blocks.insert( BlockId(2), Block(vec![Instruction::Branch( - Operand::Literal(Literal::Bool(true)), + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), BlockId(5), BlockId(6), )]), @@ -275,8 +370,7 @@ fn test_dominator_graph_with_node_having_many_predicates() { .blocks .insert(BlockId(7), Block(vec![Instruction::Return])); - remap_block_ids(&mut program); - let doms = build_dominator_graph(&program); + let doms = build_doms(&mut program); expect![[r#" Block 0 dominated by block 0, diff --git a/compiler/qsc_rir/src/passes/ssa_check.rs b/compiler/qsc_rir/src/passes/ssa_check.rs new file mode 100644 index 0000000000..5a4793d23e --- /dev/null +++ b/compiler/qsc_rir/src/passes/ssa_check.rs @@ -0,0 +1,230 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use qsc_data_structures::index_map::IndexMap; + +use crate::rir::{BlockId, Instruction, Operand, Program, VariableId}; + +#[cfg(test)] +mod tests; + +/// Verifies that the program is in Single Static Assignment (SSA) form. +/// This check ensures that: +/// - Each variable is assigned exactly once. +/// - Each variable is used after it is assigned. +/// - Each variable is used in a block that is dominated by the block in which it is assigned. +/// - Each phi node references only its predecessors. +pub fn check_ssa_form( + program: &Program, + preds: &IndexMap>, + doms: &IndexMap, +) { + check_phi_nodes(program, preds); + let variable_assignments = get_variable_assignments(program); + let variable_uses = get_variable_uses(program); + + for (var_id, uses) in variable_uses.iter() { + let Some((def_block_id, def_idx)) = variable_assignments.get(var_id) else { + panic!("{var_id:?} is used but not assigned"); + }; + for (use_block_id, use_idx) in uses { + if use_block_id == def_block_id { + assert!( + use_idx > def_idx, + "{var_id:?} is used before it is assigned in {use_block_id:?}, instruction {use_idx:?}" + ); + } else { + let mut dominator = doms + .get(*use_block_id) + .expect("all blocks should have dominator"); + while dominator != def_block_id { + let new_dom = doms + .get(*dominator) + .expect("all blocks should have dominator"); + assert!(new_dom != dominator || new_dom == def_block_id, + "Definition of {var_id:?} in {def_block_id:?} does not dominate use in {use_block_id:?}, instruction {use_idx:?}"); + dominator = new_dom; + } + } + } + } +} + +fn check_phi_nodes(program: &Program, preds: &IndexMap>) { + for (block_id, block) in program.blocks.iter() { + for instr in &block.0 { + if let Instruction::Phi(args, res) = instr { + for (val, pred_block_id) in args { + assert!( + preds + .get(block_id) + .expect("block with phi should have predecessors") + .contains(pred_block_id), + "Phi node in {block_id:?} references a non-predecessor {pred_block_id:?}" + ); + if let Operand::Variable(var) = val { + assert!( + var.variable_id.0 != res.variable_id.0, + "Phi node in {block_id:?} assigns to {:?} to itself", + res.variable_id + ); + } + } + } + } + } +} + +fn get_variable_assignments(program: &Program) -> IndexMap { + let mut assignments = IndexMap::default(); + for (block_id, block) in program.blocks.iter() { + for (idx, instr) in block.0.iter().enumerate() { + match instr { + Instruction::Call(_, _, Some(var)) + | Instruction::Add(_, _, var) + | Instruction::Sub(_, _, var) + | Instruction::Mul(_, _, var) + | Instruction::Sdiv(_, _, var) + | Instruction::Srem(_, _, var) + | Instruction::Shl(_, _, var) + | Instruction::Ashr(_, _, var) + | Instruction::Icmp(_, _, _, var) + | Instruction::LogicalNot(_, var) + | Instruction::LogicalAnd(_, _, var) + | Instruction::LogicalOr(_, _, var) + | Instruction::BitwiseNot(_, var) + | Instruction::BitwiseAnd(_, _, var) + | Instruction::BitwiseOr(_, _, var) + | Instruction::BitwiseXor(_, _, var) + | Instruction::Phi(_, var) => { + assert!( + !assignments.contains_key(var.variable_id), + "Duplicate assignment to {:?} in {block_id:?}, instruction {idx}", + var.variable_id + ); + assignments.insert(var.variable_id, (block_id, idx)); + } + Instruction::Call(_, _, None) + | Instruction::Jump(..) + | Instruction::Branch(..) + | Instruction::Return => {} + + Instruction::Store(..) => { + panic!("Unexpected Store at {block_id:?}, instruction {idx}") + } + } + } + } + assignments +} + +fn get_variable_uses(program: &Program) -> IndexMap> { + let mut uses: IndexMap> = IndexMap::default(); + let mut add_use = |var_id, block_id, idx| { + if let Some(entry) = uses.get_mut(var_id) { + entry.push((block_id, idx)); + } else { + uses.insert(var_id, vec![(block_id, idx)]); + } + }; + for (block_id, block) in program.blocks.iter() { + for (idx, instr) in block.0.iter().enumerate() { + match instr { + // Single variable + Instruction::Add(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::Add(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::Sub(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::Sub(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::Mul(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::Mul(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::Sdiv(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::Sdiv(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::Srem(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::Srem(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::Shl(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::Shl(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::Ashr(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::Ashr(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::Icmp(_, Operand::Variable(var), Operand::Literal(_), _) + | Instruction::Icmp(_, Operand::Literal(_), Operand::Variable(var), _) + | Instruction::LogicalNot(Operand::Variable(var), _) + | Instruction::LogicalAnd(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::LogicalAnd(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::LogicalOr(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::LogicalOr(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::BitwiseNot(Operand::Variable(var), _) + | Instruction::BitwiseAnd(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::BitwiseAnd(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::BitwiseOr(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::BitwiseOr(Operand::Literal(_), Operand::Variable(var), _) + | Instruction::BitwiseXor(Operand::Variable(var), Operand::Literal(_), _) + | Instruction::BitwiseXor(Operand::Literal(_), Operand::Variable(var), _) => { + add_use(var.variable_id, block_id, idx); + } + + // Double variable + Instruction::Add(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::Sub(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::Mul(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::Sdiv(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::Srem(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::Shl(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::Ashr(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::Icmp(_, Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::LogicalAnd(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::LogicalOr(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::BitwiseAnd(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::BitwiseOr(Operand::Variable(var1), Operand::Variable(var2), _) + | Instruction::BitwiseXor(Operand::Variable(var1), Operand::Variable(var2), _) => { + add_use(var1.variable_id, block_id, idx); + add_use(var2.variable_id, block_id, idx); + } + + // Multiple variables + Instruction::Call(_, vals, _) => { + for val in vals { + if let Operand::Variable(var) = val { + add_use(var.variable_id, block_id, idx); + } + } + } + Instruction::Phi(args, _) => { + for (val, pred_block_id) in args { + if let Operand::Variable(var) = val { + // As a special case for phi, treat the variable as used in the predecessor block + // rather than the phi block, at the max instruction index to avoid failing the + // dominance check within that block. + add_use(var.variable_id, *pred_block_id, usize::MAX); + } + } + } + + // No variables + Instruction::Add(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::Sub(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::Mul(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::Sdiv(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::Srem(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::Shl(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::Ashr(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::Icmp(_, Operand::Literal(_), Operand::Literal(_), _) + | Instruction::LogicalNot(Operand::Literal(_), _) + | Instruction::LogicalAnd(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::LogicalOr(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::BitwiseNot(Operand::Literal(_), _) + | Instruction::BitwiseAnd(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::BitwiseOr(Operand::Literal(_), Operand::Literal(_), _) + | Instruction::BitwiseXor(Operand::Literal(_), Operand::Literal(_), _) => { + panic!("{block_id:?}, instruction {idx} has no variables: {instr}") + } + + Instruction::Jump(..) | Instruction::Branch(..) | Instruction::Return => {} + + Instruction::Store(..) => { + panic!("Unexpected Store at {block_id:?}, instruction {idx}") + } + } + } + } + uses +} diff --git a/compiler/qsc_rir/src/passes/ssa_check/tests.rs b/compiler/qsc_rir/src/passes/ssa_check/tests.rs new file mode 100644 index 0000000000..f9cefb362c --- /dev/null +++ b/compiler/qsc_rir/src/passes/ssa_check/tests.rs @@ -0,0 +1,870 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#![allow(clippy::too_many_lines, clippy::needless_raw_string_hashes)] + +use crate::{ + builder::{bell_program, new_program, teleport_program}, + passes::{build_dominator_graph, remap_block_ids}, + rir::{ + Block, BlockId, Callable, CallableId, CallableType, Instruction, Literal, Operand, Program, + Ty, Variable, VariableId, + }, + utils::build_predecessors_map, +}; + +use super::check_ssa_form; + +fn perform_ssa_check(program: &mut Program) { + remap_block_ids(program); + let preds = build_predecessors_map(program); + let doms = build_dominator_graph(program, &preds); + check_ssa_form(program, &preds, &doms); +} + +#[test] +fn test_ssa_check_passes_for_base_profile_program() { + let mut program = bell_program(); + + perform_ssa_check(&mut program); +} + +#[test] +fn test_ssa_check_passes_for_adaptive_program_with_all_literals() { + let mut program = teleport_program(); + + perform_ssa_check(&mut program); +} + +#[test] +#[should_panic( + expected = "BlockId(0), instruction 0 has no variables: Variable(0, Boolean) = LogicalNot Bool(true)" +)] +fn test_ssa_check_fails_for_instruction_on_literal_values() { + let mut program = new_program(); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::LogicalNot( + Operand::Literal(Literal::Bool(true)), + Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }, + ), + Instruction::Return, + ]), + ); + + perform_ssa_check(&mut program); +} + +#[test] +#[should_panic( + expected = "VariableId(1) is used before it is assigned in BlockId(0), instruction 0" +)] +fn test_ssa_check_fails_for_use_before_assignment_in_single_block() { + let mut program = new_program(); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }, + ), + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }, + ), + Instruction::Return, + ]), + ); + + perform_ssa_check(&mut program); +} + +#[test] +#[should_panic(expected = "VariableId(4) is used but not assigned")] +fn test_ssa_check_fails_for_use_without_assignment_in_single_block() { + let mut program = new_program(); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(4), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }, + ), + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }, + ), + Instruction::Return, + ]), + ); + + perform_ssa_check(&mut program); +} + +#[test] +#[should_panic( + expected = "Definition of VariableId(1) in BlockId(1) does not dominate use in BlockId(0), instruction 0" +)] +fn test_ssa_check_fails_for_use_before_assignment_across_sequential_blocks() { + let mut program = new_program(); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(1)), + ]), + ); + + program.blocks.insert( + BlockId(1), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }, + ), + Instruction::Return, + ]), + ); + + perform_ssa_check(&mut program); +} + +#[test] +#[should_panic(expected = "Duplicate assignment to VariableId(0) in BlockId(0), instruction 1")] +fn test_ssa_check_fails_for_multiple_assignment_in_single_block() { + let mut program = new_program(); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }, + ), + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }, + ), + Instruction::Return, + ]), + ); + + perform_ssa_check(&mut program); +} + +#[test] +fn test_ssa_check_passes_for_variable_that_dominates_usage() { + let mut program = new_program(); + program.callables.insert( + CallableId(1), + Callable { + name: "dynamic_bool".to_string(), + input_type: Vec::new(), + output_type: Some(Ty::Boolean), + body: None, + call_type: CallableType::Regular, + }, + ); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::Call( + CallableId(1), + Vec::new(), + Some(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + ), + Instruction::Branch( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + BlockId(1), + BlockId(2), + ), + ]), + ); + + program.blocks.insert( + BlockId(1), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(3)), + ]), + ); + + program.blocks.insert( + BlockId(2), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(3)), + ]), + ); + + program + .blocks + .insert(BlockId(3), Block(vec![Instruction::Return])); + + perform_ssa_check(&mut program); +} + +#[test] +#[should_panic( + expected = "Definition of VariableId(2) in BlockId(2) does not dominate use in BlockId(3), instruction 0" +)] +fn test_ssa_check_fails_when_definition_does_not_dominates_usage() { + let mut program = new_program(); + program.callables.insert( + CallableId(1), + Callable { + name: "dynamic_bool".to_string(), + input_type: Vec::new(), + output_type: Some(Ty::Boolean), + body: None, + call_type: CallableType::Regular, + }, + ); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::Call( + CallableId(1), + Vec::new(), + Some(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + ), + Instruction::Branch( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + BlockId(1), + BlockId(2), + ), + ]), + ); + + program.blocks.insert( + BlockId(1), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(3)), + ]), + ); + + program.blocks.insert( + BlockId(2), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(3)), + ]), + ); + + program.blocks.insert( + BlockId(3), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(3), + ty: Ty::Boolean, + }, + ), + Instruction::Return, + ]), + ); + + perform_ssa_check(&mut program); +} + +#[test] +fn test_ssa_check_succeeds_when_phi_handles_multiple_values_from_branches() { + let mut program = new_program(); + program.callables.insert( + CallableId(1), + Callable { + name: "dynamic_bool".to_string(), + input_type: Vec::new(), + output_type: Some(Ty::Boolean), + body: None, + call_type: CallableType::Regular, + }, + ); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::Call( + CallableId(1), + Vec::new(), + Some(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + ), + Instruction::Branch( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + BlockId(1), + BlockId(2), + ), + ]), + ); + + program.blocks.insert( + BlockId(1), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(3)), + ]), + ); + + program.blocks.insert( + BlockId(2), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(3)), + ]), + ); + + program.blocks.insert( + BlockId(3), + Block(vec![ + Instruction::Phi( + vec![ + ( + Operand::Variable(Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }), + BlockId(1), + ), + ( + Operand::Variable(Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }), + BlockId(2), + ), + ], + Variable { + variable_id: VariableId(3), + ty: Ty::Boolean, + }, + ), + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(3), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(4), + ty: Ty::Boolean, + }, + ), + Instruction::Return, + ]), + ); + + perform_ssa_check(&mut program); +} + +#[test] +fn test_ssa_check_succeeds_when_phi_handles_value_from_dominator_of_predecessor() { + let mut program = new_program(); + program.callables.insert( + CallableId(1), + Callable { + name: "dynamic_bool".to_string(), + input_type: Vec::new(), + output_type: Some(Ty::Boolean), + body: None, + call_type: CallableType::Regular, + }, + ); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::Call( + CallableId(1), + Vec::new(), + Some(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + ), + Instruction::Branch( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + BlockId(1), + BlockId(2), + ), + ]), + ); + + program.blocks.insert( + BlockId(1), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(3)), + ]), + ); + + program.blocks.insert( + BlockId(2), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(4)), + ]), + ); + + program + .blocks + .insert(BlockId(4), Block(vec![Instruction::Jump(BlockId(3))])); + + program.blocks.insert( + BlockId(3), + Block(vec![ + Instruction::Phi( + vec![ + ( + Operand::Variable(Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }), + BlockId(1), + ), + ( + Operand::Variable(Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }), + BlockId(4), + ), + ], + Variable { + variable_id: VariableId(3), + ty: Ty::Boolean, + }, + ), + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(3), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(4), + ty: Ty::Boolean, + }, + ), + Instruction::Return, + ]), + ); + + perform_ssa_check(&mut program); +} + +#[test] +#[should_panic( + expected = "Definition of VariableId(3) in BlockId(5) does not dominate use in BlockId(6), instruction 18446744073709551615" +)] +fn test_ssa_check_fails_when_phi_handles_value_from_non_dominator_of_predecessor() { + let mut program = new_program(); + program.callables.insert( + CallableId(1), + Callable { + name: "dynamic_bool".to_string(), + input_type: Vec::new(), + output_type: Some(Ty::Boolean), + body: None, + call_type: CallableType::Regular, + }, + ); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::Call( + CallableId(1), + Vec::new(), + Some(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + ), + Instruction::Branch( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + BlockId(1), + BlockId(2), + ), + ]), + ); + + program.blocks.insert( + BlockId(1), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(3)), + ]), + ); + + program.blocks.insert( + BlockId(2), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }, + ), + Instruction::Branch( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + BlockId(4), + BlockId(5), + ), + ]), + ); + + program + .blocks + .insert(BlockId(4), Block(vec![Instruction::Jump(BlockId(6))])); + + program.blocks.insert( + BlockId(5), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(3), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(6)), + ]), + ); + + program + .blocks + .insert(BlockId(6), Block(vec![Instruction::Jump(BlockId(3))])); + + program.blocks.insert( + BlockId(3), + Block(vec![ + Instruction::Phi( + vec![ + ( + Operand::Variable(Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }), + BlockId(1), + ), + ( + Operand::Variable(Variable { + variable_id: VariableId(3), + ty: Ty::Boolean, + }), + BlockId(6), + ), + ], + Variable { + variable_id: VariableId(4), + ty: Ty::Boolean, + }, + ), + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(4), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(5), + ty: Ty::Boolean, + }, + ), + Instruction::Return, + ]), + ); + + perform_ssa_check(&mut program); +} + +#[test] +#[should_panic(expected = "Phi node in BlockId(3) references a non-predecessor BlockId(0)")] +fn test_ssa_check_fails_when_phi_lists_non_predecessor_block() { + let mut program = new_program(); + program.callables.insert( + CallableId(1), + Callable { + name: "dynamic_bool".to_string(), + input_type: Vec::new(), + output_type: Some(Ty::Boolean), + body: None, + call_type: CallableType::Regular, + }, + ); + + program.blocks.insert( + BlockId(0), + Block(vec![ + Instruction::Call( + CallableId(1), + Vec::new(), + Some(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + ), + Instruction::Branch( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + BlockId(1), + BlockId(2), + ), + ]), + ); + + program.blocks.insert( + BlockId(1), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(3)), + ]), + ); + + program.blocks.insert( + BlockId(2), + Block(vec![ + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }, + ), + Instruction::Jump(BlockId(3)), + ]), + ); + + program.blocks.insert( + BlockId(3), + Block(vec![ + Instruction::Phi( + vec![ + ( + Operand::Variable(Variable { + variable_id: VariableId(1), + ty: Ty::Boolean, + }), + BlockId(0), + ), + ( + Operand::Variable(Variable { + variable_id: VariableId(2), + ty: Ty::Boolean, + }), + BlockId(4), + ), + ], + Variable { + variable_id: VariableId(3), + ty: Ty::Boolean, + }, + ), + Instruction::LogicalNot( + Operand::Variable(Variable { + variable_id: VariableId(3), + ty: Ty::Boolean, + }), + Variable { + variable_id: VariableId(4), + ty: Ty::Boolean, + }, + ), + Instruction::Return, + ]), + ); + + perform_ssa_check(&mut program); +} diff --git a/compiler/qsc_rir/src/passes/unreachable_code_check/tests.rs b/compiler/qsc_rir/src/passes/unreachable_code_check/tests.rs index 8508e4229e..6db59c57ea 100644 --- a/compiler/qsc_rir/src/passes/unreachable_code_check/tests.rs +++ b/compiler/qsc_rir/src/passes/unreachable_code_check/tests.rs @@ -131,7 +131,10 @@ fn test_check_unreachable_blocks_succeeds_on_no_unreachable_blocks_with_branch() program.blocks.insert( BlockId(0), Block(vec![Instruction::Branch( - Operand::Literal(Literal::Bool(true)), + Operand::Variable(Variable { + variable_id: VariableId(0), + ty: Ty::Boolean, + }), BlockId(1), BlockId(2), )]), @@ -410,3 +413,35 @@ fn test_check_unreachable_callable_succeeds_on_no_unreachable_callables_with_cal ); check_unreachable_callable(&program); } + +#[test] +#[should_panic(expected = "Unreachable blocks found: [BlockId(2)]")] +fn constant_branches_lead_to_unreachable_blocks() { + let mut program = Program::new(); + program.entry = CallableId(0); + program.callables.insert( + CallableId(0), + Callable { + name: "test".to_string(), + input_type: vec![], + output_type: None, + body: Some(BlockId(0)), + call_type: CallableType::Regular, + }, + ); + program.blocks.insert( + BlockId(0), + Block(vec![Instruction::Branch( + Operand::Literal(Literal::Bool(true)), + BlockId(1), + BlockId(2), + )]), + ); + program + .blocks + .insert(BlockId(1), Block(vec![Instruction::Return])); + program + .blocks + .insert(BlockId(2), Block(vec![Instruction::Return])); + check_unreachable_blocks(&program); +} diff --git a/compiler/qsc_rir/src/rir.rs b/compiler/qsc_rir/src/rir.rs index 6ba079062a..2af2d7b78e 100644 --- a/compiler/qsc_rir/src/rir.rs +++ b/compiler/qsc_rir/src/rir.rs @@ -416,9 +416,21 @@ impl Display for Instruction { } } -#[derive(Clone, Copy, Default)] +#[derive(Debug, Clone, Copy, Default)] pub struct VariableId(pub u32); +impl From for usize { + fn from(id: VariableId) -> usize { + id.0 as usize + } +} + +impl From for VariableId { + fn from(id: usize) -> Self { + Self(id.try_into().expect("variable id should fit into u32")) + } +} + #[derive(Clone, Copy)] pub struct Variable { pub variable_id: VariableId, diff --git a/compiler/qsc_rir/src/utils.rs b/compiler/qsc_rir/src/utils.rs index bf6b887fd4..d15e3dbc13 100644 --- a/compiler/qsc_rir/src/utils.rs +++ b/compiler/qsc_rir/src/utils.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use crate::rir::{Block, BlockId, Instruction, Program}; +use crate::rir::{Block, BlockId, Instruction, Literal, Operand, Program}; use qsc_data_structures::index_map::IndexMap; use rustc_hash::FxHashSet; @@ -15,13 +15,15 @@ pub fn get_block_successors(block: &Block) -> Vec { .last() .expect("block should have at least one instruction") { - Instruction::Jump(target) => { - successors.push(*target); - } - Instruction::Branch(_, target1, target2) => { + Instruction::Branch(Operand::Variable(_), target1, target2) => { successors.push(*target1); successors.push(*target2); } + Instruction::Jump(target) + | Instruction::Branch(Operand::Literal(Literal::Bool(true)), target, _) + | Instruction::Branch(Operand::Literal(Literal::Bool(false)), _, target) => { + successors.push(*target); + } _ => {} } successors