From 731ff1480f31cab91ff2bb8bb3d1664f50b2a34a Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Tue, 7 Jan 2025 12:46:01 +0200 Subject: [PATCH] Simplify working with labels TLDR: no need to call either of: program.emit_insn_with_label_dependency() -> just call program.emit_insn() program.defer_label_resolution() -> just call program.resolve_label() Changes: - make BranchOffset an explicit enum (Label, Offset, Placeholder) - remove program.emit_insn_with_label_dependency() - label dependency is automatically detected - for label to offset mapping, use a hashmap from label(negative i32) to offset (positive u32) - resolve all labels in program.build() - remove program.defer_label_resolution() - all labels are resolved in build() --- core/translate/emitter.rs | 40 ++-- core/translate/expr.rs | 405 +++++++++++++---------------------- core/translate/group_by.rs | 136 +++++------- core/translate/insert.rs | 63 +++--- core/translate/main_loop.rs | 180 ++++++---------- core/translate/mod.rs | 28 +-- core/translate/order_by.rs | 24 +-- core/translate/planner.rs | 2 +- core/translate/result_row.rs | 13 +- core/translate/subquery.rs | 15 +- core/vdbe/builder.rs | 209 ++++++------------ core/vdbe/explain.rs | 113 ++++++---- core/vdbe/mod.rs | 228 +++++++++++++------- 13 files changed, 630 insertions(+), 826 deletions(-) diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 7b07f1a7a..8fdff8cd1 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -104,12 +104,9 @@ fn prologue<'a>( let mut program = ProgramBuilder::new(); let init_label = program.allocate_label(); - program.emit_insn_with_label_dependency( - Insn::Init { - target_pc: init_label, - }, - init_label, - ); + program.emit_insn(Insn::Init { + target_pc: init_label, + }); let start_offset = program.offset(); @@ -151,8 +148,6 @@ fn epilogue( target_pc: start_offset, }); - program.resolve_deferred_labels(); - Ok(()) } @@ -218,12 +213,9 @@ pub fn emit_query<'a>( let after_main_loop_label = program.allocate_label(); t_ctx.label_main_loop_end = Some(after_main_loop_label); if plan.contains_constant_false_condition { - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: after_main_loop_label, - }, - after_main_loop_label, - ); + program.emit_insn(Insn::Goto { + target_pc: after_main_loop_label, + }); } // Allocate registers for result columns @@ -281,12 +273,9 @@ fn emit_program_for_delete( // No rows will be read from source table loops if there is a constant false condition eg. WHERE 0 let after_main_loop_label = program.allocate_label(); if plan.contains_constant_false_condition { - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: after_main_loop_label, - }, - after_main_loop_label, - ); + program.emit_insn(Insn::Goto { + target_pc: after_main_loop_label, + }); } // Initialize cursors and other resources needed for query execution @@ -356,13 +345,10 @@ fn emit_delete_insns<'a>( dest: limit_reg, }); program.mark_last_insn_constant(); - program.emit_insn_with_label_dependency( - Insn::DecrJumpZero { - reg: limit_reg, - target_pc: t_ctx.label_main_loop_end.unwrap(), - }, - t_ctx.label_main_loop_end.unwrap(), - ) + program.emit_insn(Insn::DecrJumpZero { + reg: limit_reg, + target_pc: t_ctx.label_main_loop_end.unwrap(), + }) } Ok(()) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index b4fb5e87e..4c4ee72b2 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -13,7 +13,7 @@ use crate::Result; use super::emitter::Resolver; use super::plan::{TableReference, TableReferenceType}; -#[derive(Default, Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy)] pub struct ConditionMetadata { pub jump_if_condition_is_true: bool, pub jump_target_when_true: BranchOffset, @@ -87,128 +87,92 @@ pub fn translate_condition_expr( match op { ast::Operator::Greater => { if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::Gt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }, - condition_metadata.jump_target_when_true, - ) + program.emit_insn(Insn::Gt { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_true, + }) } else { - program.emit_insn_with_label_dependency( - Insn::Le { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }, - condition_metadata.jump_target_when_false, - ) + program.emit_insn(Insn::Le { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_false, + }) } } ast::Operator::GreaterEquals => { if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::Ge { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }, - condition_metadata.jump_target_when_true, - ) + program.emit_insn(Insn::Ge { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_true, + }) } else { - program.emit_insn_with_label_dependency( - Insn::Lt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }, - condition_metadata.jump_target_when_false, - ) + program.emit_insn(Insn::Lt { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_false, + }) } } ast::Operator::Less => { if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::Lt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }, - condition_metadata.jump_target_when_true, - ) + program.emit_insn(Insn::Lt { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_true, + }) } else { - program.emit_insn_with_label_dependency( - Insn::Ge { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }, - condition_metadata.jump_target_when_false, - ) + program.emit_insn(Insn::Ge { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_false, + }) } } ast::Operator::LessEquals => { if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::Le { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }, - condition_metadata.jump_target_when_true, - ) + program.emit_insn(Insn::Le { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_true, + }) } else { - program.emit_insn_with_label_dependency( - Insn::Gt { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }, - condition_metadata.jump_target_when_false, - ) + program.emit_insn(Insn::Gt { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_false, + }) } } ast::Operator::Equals => { if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::Eq { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }, - condition_metadata.jump_target_when_true, - ) + program.emit_insn(Insn::Eq { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_true, + }) } else { - program.emit_insn_with_label_dependency( - Insn::Ne { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }, - condition_metadata.jump_target_when_false, - ) + program.emit_insn(Insn::Ne { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_false, + }) } } ast::Operator::NotEquals => { if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::Ne { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_true, - }, - condition_metadata.jump_target_when_true, - ) + program.emit_insn(Insn::Ne { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_true, + }) } else { - program.emit_insn_with_label_dependency( - Insn::Eq { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }, - condition_metadata.jump_target_when_false, - ) + program.emit_insn(Insn::Eq { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_false, + }) } } ast::Operator::Is => todo!(), @@ -228,23 +192,17 @@ pub fn translate_condition_expr( dest: reg, }); if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::If { - reg, - target_pc: condition_metadata.jump_target_when_true, - null_reg: reg, - }, - condition_metadata.jump_target_when_true, - ) + program.emit_insn(Insn::If { + reg, + target_pc: condition_metadata.jump_target_when_true, + null_reg: reg, + }) } else { - program.emit_insn_with_label_dependency( - Insn::IfNot { - reg, - target_pc: condition_metadata.jump_target_when_false, - null_reg: reg, - }, - condition_metadata.jump_target_when_false, - ) + program.emit_insn(Insn::IfNot { + reg, + target_pc: condition_metadata.jump_target_when_false, + null_reg: reg, + }) } } else { crate::bail_parse_error!("unsupported literal type in condition"); @@ -257,23 +215,17 @@ pub fn translate_condition_expr( dest: reg, }); if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::If { - reg, - target_pc: condition_metadata.jump_target_when_true, - null_reg: reg, - }, - condition_metadata.jump_target_when_true, - ) + program.emit_insn(Insn::If { + reg, + target_pc: condition_metadata.jump_target_when_true, + null_reg: reg, + }) } else { - program.emit_insn_with_label_dependency( - Insn::IfNot { - reg, - target_pc: condition_metadata.jump_target_when_false, - null_reg: reg, - }, - condition_metadata.jump_target_when_false, - ) + program.emit_insn(Insn::IfNot { + reg, + target_pc: condition_metadata.jump_target_when_false, + null_reg: reg, + }) } } unimpl => todo!("literal {:?} not implemented", unimpl), @@ -302,20 +254,14 @@ pub fn translate_condition_expr( // Note that we are already breaking up our WHERE clauses into a series of terms at "AND" boundaries, so right now we won't be running into cases where jumping on true would be incorrect, // but once we have e.g. parenthesization and more complex conditions, not having this 'if' here would introduce a bug. if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: condition_metadata.jump_target_when_true, - }, - condition_metadata.jump_target_when_true, - ); + program.emit_insn(Insn::Goto { + target_pc: condition_metadata.jump_target_when_true, + }); } } else { - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: condition_metadata.jump_target_when_false, - }, - condition_metadata.jump_target_when_false, - ); + program.emit_insn(Insn::Goto { + target_pc: condition_metadata.jump_target_when_false, + }); } return Ok(()); } @@ -349,35 +295,26 @@ pub fn translate_condition_expr( translate_expr(program, Some(referenced_tables), expr, rhs_reg, resolver)?; // If this is not the last condition, we need to jump to the 'jump_target_when_true' label if the condition is true. if !last_condition { - program.emit_insn_with_label_dependency( - Insn::Eq { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: jump_target_when_true, - }, - jump_target_when_true, - ); + program.emit_insn(Insn::Eq { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: jump_target_when_true, + }); } else { // If this is the last condition, we need to jump to the 'jump_target_when_false' label if there is no match. - program.emit_insn_with_label_dependency( - Insn::Ne { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }, - condition_metadata.jump_target_when_false, - ); + program.emit_insn(Insn::Ne { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_false, + }); } } // If we got here, then the last condition was a match, so we jump to the 'jump_target_when_true' label if 'jump_if_condition_is_true'. // If not, we can just fall through without emitting an unnecessary instruction. if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: condition_metadata.jump_target_when_true, - }, - condition_metadata.jump_target_when_true, - ); + program.emit_insn(Insn::Goto { + target_pc: condition_metadata.jump_target_when_true, + }); } } else { // If it's a NOT IN expression, we need to jump to the 'jump_target_when_false' label if any of the conditions are true. @@ -385,24 +322,18 @@ pub fn translate_condition_expr( let rhs_reg = program.alloc_register(); let _ = translate_expr(program, Some(referenced_tables), expr, rhs_reg, resolver)?; - program.emit_insn_with_label_dependency( - Insn::Eq { - lhs: lhs_reg, - rhs: rhs_reg, - target_pc: condition_metadata.jump_target_when_false, - }, - condition_metadata.jump_target_when_false, - ); + program.emit_insn(Insn::Eq { + lhs: lhs_reg, + rhs: rhs_reg, + target_pc: condition_metadata.jump_target_when_false, + }); } // If we got here, then none of the conditions were a match, so we jump to the 'jump_target_when_true' label if 'jump_if_condition_is_true'. // If not, we can just fall through without emitting an unnecessary instruction. if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: condition_metadata.jump_target_when_true, - }, - condition_metadata.jump_target_when_true, - ); + program.emit_insn(Insn::Goto { + target_pc: condition_metadata.jump_target_when_true, + }); } } @@ -464,42 +395,30 @@ pub fn translate_condition_expr( } if !*not { if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::If { - reg: cur_reg, - target_pc: condition_metadata.jump_target_when_true, - null_reg: cur_reg, - }, - condition_metadata.jump_target_when_true, - ); - } else { - program.emit_insn_with_label_dependency( - Insn::IfNot { - reg: cur_reg, - target_pc: condition_metadata.jump_target_when_false, - null_reg: cur_reg, - }, - condition_metadata.jump_target_when_false, - ); - } - } else if condition_metadata.jump_if_condition_is_true { - program.emit_insn_with_label_dependency( - Insn::IfNot { + program.emit_insn(Insn::If { reg: cur_reg, target_pc: condition_metadata.jump_target_when_true, null_reg: cur_reg, - }, - condition_metadata.jump_target_when_true, - ); - } else { - program.emit_insn_with_label_dependency( - Insn::If { + }); + } else { + program.emit_insn(Insn::IfNot { reg: cur_reg, target_pc: condition_metadata.jump_target_when_false, null_reg: cur_reg, - }, - condition_metadata.jump_target_when_false, - ); + }); + } + } else if condition_metadata.jump_if_condition_is_true { + program.emit_insn(Insn::IfNot { + reg: cur_reg, + target_pc: condition_metadata.jump_target_when_true, + null_reg: cur_reg, + }); + } else { + program.emit_insn(Insn::If { + reg: cur_reg, + target_pc: condition_metadata.jump_target_when_false, + null_reg: cur_reg, + }); } } ast::Expr::Parenthesized(exprs) => { @@ -707,23 +626,17 @@ pub fn translate_expr( translate_expr(program, referenced_tables, when_expr, expr_reg, resolver)?; match base_reg { // CASE 1 WHEN 0 THEN 0 ELSE 1 becomes 1==0, Ne branch to next clause - Some(base_reg) => program.emit_insn_with_label_dependency( - Insn::Ne { - lhs: base_reg, - rhs: expr_reg, - target_pc: next_case_label, - }, - next_case_label, - ), + Some(base_reg) => program.emit_insn(Insn::Ne { + lhs: base_reg, + rhs: expr_reg, + target_pc: next_case_label, + }), // CASE WHEN 0 THEN 0 ELSE 1 becomes ifnot 0 branch to next clause - None => program.emit_insn_with_label_dependency( - Insn::IfNot { - reg: expr_reg, - target_pc: next_case_label, - null_reg: 1, - }, - next_case_label, - ), + None => program.emit_insn(Insn::IfNot { + reg: expr_reg, + target_pc: next_case_label, + null_reg: 1, + }), }; // THEN... translate_expr( @@ -733,12 +646,9 @@ pub fn translate_expr( target_register, resolver, )?; - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: return_label, - }, - return_label, - ); + program.emit_insn(Insn::Goto { + target_pc: return_label, + }); // This becomes either the next WHEN, or in the last WHEN/THEN, we're // assured to have at least one instruction corresponding to the ELSE immediately follow. program.preassign_label_to_next_insn(next_case_label); @@ -984,13 +894,10 @@ pub fn translate_expr( resolver, )?; if index < args.len() - 1 { - program.emit_insn_with_label_dependency( - Insn::NotNull { - reg, - target_pc: label_coalesce_end, - }, - label_coalesce_end, - ); + program.emit_insn(Insn::NotNull { + reg, + target_pc: label_coalesce_end, + }); } } program.preassign_label_to_next_insn(label_coalesce_end); @@ -1085,7 +992,7 @@ pub fn translate_expr( )?; program.emit_insn(Insn::NotNull { reg: temp_reg, - target_pc: program.offset() + 2, + target_pc: program.offset().add(2u32), }); translate_expr( @@ -1120,14 +1027,11 @@ pub fn translate_expr( resolver, )?; let jump_target_when_false = program.allocate_label(); - program.emit_insn_with_label_dependency( - Insn::IfNot { - reg: temp_reg, - target_pc: jump_target_when_false, - null_reg: 1, - }, - jump_target_when_false, - ); + program.emit_insn(Insn::IfNot { + reg: temp_reg, + target_pc: jump_target_when_false, + null_reg: 1, + }); translate_expr( program, referenced_tables, @@ -1136,12 +1040,9 @@ pub fn translate_expr( resolver, )?; let jump_target_result = program.allocate_label(); - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: jump_target_result, - }, - jump_target_result, - ); + program.emit_insn(Insn::Goto { + target_pc: jump_target_result, + }); program.resolve_label(jump_target_when_false, program.offset()); translate_expr( program, @@ -2033,7 +1934,7 @@ fn wrap_eval_jump_expr( value: 1, // emit True by default dest: target_register, }); - program.emit_insn_with_label_dependency(insn, if_true_label); + program.emit_insn(insn); program.emit_insn(Insn::Integer { value: 0, // emit False if we reach this point (no jump) dest: target_register, diff --git a/core/translate/group_by.rs b/core/translate/group_by.rs index a38a9d26c..0990bee0d 100644 --- a/core/translate/group_by.rs +++ b/core/translate/group_by.rs @@ -93,13 +93,10 @@ pub fn init_group_by( program.add_comment(program.offset(), "go to clear accumulator subroutine"); let reg_subrtn_acc_clear_return_offset = program.alloc_register(); - program.emit_insn_with_label_dependency( - Insn::Gosub { - target_pc: label_subrtn_acc_clear, - return_reg: reg_subrtn_acc_clear_return_offset, - }, - label_subrtn_acc_clear, - ); + program.emit_insn(Insn::Gosub { + target_pc: label_subrtn_acc_clear, + return_reg: reg_subrtn_acc_clear_return_offset, + }); t_ctx.reg_agg_start = Some(reg_agg_exprs_start); @@ -187,15 +184,12 @@ pub fn emit_group_by<'a>( }); // Sort the sorter based on the group by columns - program.emit_insn_with_label_dependency( - Insn::SorterSort { - cursor_id: sort_cursor, - pc_if_empty: label_grouping_loop_end, - }, - label_grouping_loop_end, - ); + program.emit_insn(Insn::SorterSort { + cursor_id: sort_cursor, + pc_if_empty: label_grouping_loop_end, + }); - program.defer_label_resolution(label_grouping_loop_start, program.offset() as usize); + program.resolve_label(label_grouping_loop_start, program.offset()); // Read a row from the sorted data in the sorter into the pseudo cursor program.emit_insn(Insn::SorterData { cursor_id: sort_cursor, @@ -229,14 +223,11 @@ pub fn emit_group_by<'a>( "start new group if comparison is not equal", ); // If we are at a new group, continue. If we are at the same group, jump to the aggregation step (i.e. accumulate more values into the aggregations) - program.emit_insn_with_label_dependency( - Insn::Jump { - target_pc_lt: program.offset() + 1, - target_pc_eq: agg_step_label, - target_pc_gt: program.offset() + 1, - }, - agg_step_label, - ); + program.emit_insn(Insn::Jump { + target_pc_lt: program.offset().add(1u32), + target_pc_eq: agg_step_label, + target_pc_gt: program.offset().add(1u32), + }); // New group, move current group by columns into the comparison register program.emit_insn(Insn::Move { @@ -249,32 +240,23 @@ pub fn emit_group_by<'a>( program.offset(), "check if ended group had data, and output if so", ); - program.emit_insn_with_label_dependency( - Insn::Gosub { - target_pc: label_subrtn_acc_output, - return_reg: reg_subrtn_acc_output_return_offset, - }, - label_subrtn_acc_output, - ); + program.emit_insn(Insn::Gosub { + target_pc: label_subrtn_acc_output, + return_reg: reg_subrtn_acc_output_return_offset, + }); program.add_comment(program.offset(), "check abort flag"); - program.emit_insn_with_label_dependency( - Insn::IfPos { - reg: reg_abort_flag, - target_pc: label_group_by_end, - decrement_by: 0, - }, - label_group_by_end, - ); + program.emit_insn(Insn::IfPos { + reg: reg_abort_flag, + target_pc: label_group_by_end, + decrement_by: 0, + }); program.add_comment(program.offset(), "goto clear accumulator subroutine"); - program.emit_insn_with_label_dependency( - Insn::Gosub { - target_pc: label_subrtn_acc_clear, - return_reg: reg_subrtn_acc_clear_return_offset, - }, - label_subrtn_acc_clear, - ); + program.emit_insn(Insn::Gosub { + target_pc: label_subrtn_acc_clear, + return_reg: reg_subrtn_acc_clear_return_offset, + }); // Accumulate the values into the aggregations program.resolve_label(agg_step_label, program.offset()); @@ -299,14 +281,11 @@ pub fn emit_group_by<'a>( program.offset(), "don't emit group columns if continuing existing group", ); - program.emit_insn_with_label_dependency( - Insn::If { - target_pc: label_acc_indicator_set_flag_true, - reg: reg_data_in_acc_flag, - null_reg: 0, // unused in this case - }, - label_acc_indicator_set_flag_true, - ); + program.emit_insn(Insn::If { + target_pc: label_acc_indicator_set_flag_true, + reg: reg_data_in_acc_flag, + null_reg: 0, // unused in this case + }); // Read the group by columns for a finished group for i in 0..group_by.exprs.len() { @@ -326,32 +305,23 @@ pub fn emit_group_by<'a>( dest: reg_data_in_acc_flag, }); - program.emit_insn_with_label_dependency( - Insn::SorterNext { - cursor_id: sort_cursor, - pc_if_next: label_grouping_loop_start, - }, - label_grouping_loop_start, - ); + program.emit_insn(Insn::SorterNext { + cursor_id: sort_cursor, + pc_if_next: label_grouping_loop_start, + }); program.resolve_label(label_grouping_loop_end, program.offset()); program.add_comment(program.offset(), "emit row for final group"); - program.emit_insn_with_label_dependency( - Insn::Gosub { - target_pc: label_subrtn_acc_output, - return_reg: reg_subrtn_acc_output_return_offset, - }, - label_subrtn_acc_output, - ); + program.emit_insn(Insn::Gosub { + target_pc: label_subrtn_acc_output, + return_reg: reg_subrtn_acc_output_return_offset, + }); program.add_comment(program.offset(), "group by finished"); - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: label_group_by_end, - }, - label_group_by_end, - ); + program.emit_insn(Insn::Goto { + target_pc: label_group_by_end, + }); program.emit_insn(Insn::Integer { value: 1, dest: reg_abort_flag, @@ -363,19 +333,13 @@ pub fn emit_group_by<'a>( program.resolve_label(label_subrtn_acc_output, program.offset()); program.add_comment(program.offset(), "output group by row subroutine start"); - program.emit_insn_with_label_dependency( - Insn::IfPos { - reg: reg_data_in_acc_flag, - target_pc: label_agg_final, - decrement_by: 0, - }, - label_agg_final, - ); + program.emit_insn(Insn::IfPos { + reg: reg_data_in_acc_flag, + target_pc: label_agg_final, + decrement_by: 0, + }); let group_by_end_without_emitting_row_label = program.allocate_label(); - program.defer_label_resolution( - group_by_end_without_emitting_row_label, - program.offset() as usize, - ); + program.resolve_label(group_by_end_without_emitting_row_label, program.offset()); program.emit_insn(Insn::Return { return_reg: reg_subrtn_acc_output_return_offset, }); @@ -417,7 +381,7 @@ pub fn emit_group_by<'a>( ConditionMetadata { jump_if_condition_is_true: false, jump_target_when_false: group_by_end_without_emitting_row_label, - jump_target_when_true: i64::MAX, // unused + jump_target_when_true: BranchOffset::Placeholder, // not used. FIXME: this is a bug. HAVING can have e.g. HAVING a OR b. }, &t_ctx.resolver, )?; diff --git a/core/translate/insert.rs b/core/translate/insert.rs index 3213e89cd..b9f73ba8c 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -7,6 +7,7 @@ use sqlite3_parser::ast::{ use crate::error::SQLITE_CONSTRAINT_PRIMARYKEY; use crate::util::normalize_ident; +use crate::vdbe::BranchOffset; use crate::{ schema::{Column, Schema, Table}, storage::sqlite3_ondisk::DatabaseHeader, @@ -40,12 +41,9 @@ pub fn translate_insert( let mut program = ProgramBuilder::new(); let resolver = Resolver::new(syms); let init_label = program.allocate_label(); - program.emit_insn_with_label_dependency( - Insn::Init { - target_pc: init_label, - }, - init_label, - ); + program.emit_insn(Insn::Init { + target_pc: init_label, + }); let start_offset = program.offset(); // open table @@ -104,7 +102,7 @@ pub fn translate_insert( let record_register = program.alloc_register(); let halt_label = program.allocate_label(); - let mut loop_start_offset = 0; + let mut loop_start_offset = BranchOffset::Offset(0); let inserting_multiple_rows = values.len() > 1; @@ -112,14 +110,11 @@ pub fn translate_insert( if inserting_multiple_rows { let yield_reg = program.alloc_register(); let jump_on_definition_label = program.allocate_label(); - program.emit_insn_with_label_dependency( - Insn::InitCoroutine { - yield_reg, - jump_on_definition: jump_on_definition_label, - start_offset: program.offset() + 1, - }, - jump_on_definition_label, - ); + program.emit_insn(Insn::InitCoroutine { + yield_reg, + jump_on_definition: jump_on_definition_label, + start_offset: program.offset().add(1u32), + }); for value in values { populate_column_registers( @@ -133,7 +128,7 @@ pub fn translate_insert( )?; program.emit_insn(Insn::Yield { yield_reg, - end_offset: 0, + end_offset: halt_label, }); } program.emit_insn(Insn::EndCoroutine { yield_reg }); @@ -149,13 +144,10 @@ pub fn translate_insert( // FIXME: rollback is not implemented. E.g. if you insert 2 rows and one fails to unique constraint violation, // the other row will still be inserted. loop_start_offset = program.offset(); - program.emit_insn_with_label_dependency( - Insn::Yield { - yield_reg, - end_offset: halt_label, - }, - halt_label, - ); + program.emit_insn(Insn::Yield { + yield_reg, + end_offset: halt_label, + }); } else { // Single row - populate registers directly program.emit_insn(Insn::OpenWriteAsync { @@ -194,13 +186,10 @@ pub fn translate_insert( program.emit_insn(Insn::SoftNull { reg }); } // the user provided rowid value might itself be NULL. If it is, we create a new rowid on the next instruction. - program.emit_insn_with_label_dependency( - Insn::NotNull { - reg: rowid_reg, - target_pc: check_rowid_is_integer_label.unwrap(), - }, - check_rowid_is_integer_label.unwrap(), - ); + program.emit_insn(Insn::NotNull { + reg: rowid_reg, + target_pc: check_rowid_is_integer_label.unwrap(), + }); } // Create new rowid if a) not provided by user or b) provided by user but is NULL @@ -220,14 +209,11 @@ pub fn translate_insert( // When the DB allocates it there are no need for separate uniqueness checks. if has_user_provided_rowid { let make_record_label = program.allocate_label(); - program.emit_insn_with_label_dependency( - Insn::NotExists { - cursor: cursor_id, - rowid_reg, - target_pc: make_record_label, - }, - make_record_label, - ); + program.emit_insn(Insn::NotExists { + cursor: cursor_id, + rowid_reg, + target_pc: make_record_label, + }); let rowid_column_name = if let Some(index) = rowid_alias_index { table.column_index_to_name(index).unwrap() } else { @@ -276,7 +262,6 @@ pub fn translate_insert( program.emit_insn(Insn::Goto { target_pc: start_offset, }); - program.resolve_deferred_labels(); Ok(program.build(database_header, connection)) } diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index c11cc17c4..05daf01e5 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -194,7 +194,7 @@ pub fn open_loop( // In case the subquery is an inner loop, it needs to be reinitialized on each iteration of the outer loop. program.emit_insn(Insn::InitCoroutine { yield_reg, - jump_on_definition: 0, + jump_on_definition: BranchOffset::Offset(0), start_offset: coroutine_implementation_start, }); let LoopLabels { @@ -205,18 +205,15 @@ pub fn open_loop( .labels_main_loop .get(id) .expect("subquery has no loop labels"); - program.defer_label_resolution(loop_start, program.offset() as usize); + program.resolve_label(loop_start, program.offset()); // A subquery within the main loop of a parent query has no cursor, so instead of advancing the cursor, // it emits a Yield which jumps back to the main loop of the subquery itself to retrieve the next row. // When the subquery coroutine completes, this instruction jumps to the label at the top of the termination_label_stack, // which in this case is the end of the Yield-Goto loop in the parent query. - program.emit_insn_with_label_dependency( - Insn::Yield { - yield_reg, - end_offset: loop_end, - }, - loop_end, - ); + program.emit_insn(Insn::Yield { + yield_reg, + end_offset: loop_end, + }); // These are predicates evaluated outside of the subquery, // so they are translated here. @@ -291,10 +288,7 @@ pub fn open_loop( if *outer { let lj_meta = t_ctx.meta_left_joins.get(id).unwrap(); - program.defer_label_resolution( - lj_meta.label_match_flag_set_true, - program.offset() as usize, - ); + program.resolve_label(lj_meta.label_match_flag_set_true, program.offset()); program.emit_insn(Insn::Integer { value: 1, dest: lj_meta.reg_match_flag, @@ -326,7 +320,7 @@ pub fn open_loop( .labels_main_loop .get(id) .expect("scan has no loop labels"); - program.emit_insn_with_label_dependency( + program.emit_insn( if iter_dir .as_ref() .is_some_and(|dir| *dir == IterationDirection::Backwards) @@ -341,9 +335,8 @@ pub fn open_loop( pc_if_empty: loop_end, } }, - loop_end, ); - program.defer_label_resolution(loop_start, program.offset() as usize); + program.resolve_label(loop_start, program.offset()); if let Some(preds) = predicates { for expr in preds { @@ -420,28 +413,25 @@ pub fn open_loop( _ => unreachable!(), } // If we try to seek to a key that is not present in the table/index, we exit the loop entirely. - program.emit_insn_with_label_dependency( - match cmp_op { - ast::Operator::Equals | ast::Operator::GreaterEquals => Insn::SeekGE { - is_index: index_cursor_id.is_some(), - cursor_id: index_cursor_id.unwrap_or(table_cursor_id), - start_reg: cmp_reg, - num_regs: 1, - target_pc: loop_end, - }, - ast::Operator::Greater - | ast::Operator::Less - | ast::Operator::LessEquals => Insn::SeekGT { + program.emit_insn(match cmp_op { + ast::Operator::Equals | ast::Operator::GreaterEquals => Insn::SeekGE { + is_index: index_cursor_id.is_some(), + cursor_id: index_cursor_id.unwrap_or(table_cursor_id), + start_reg: cmp_reg, + num_regs: 1, + target_pc: loop_end, + }, + ast::Operator::Greater | ast::Operator::Less | ast::Operator::LessEquals => { + Insn::SeekGT { is_index: index_cursor_id.is_some(), cursor_id: index_cursor_id.unwrap_or(table_cursor_id), start_reg: cmp_reg, num_regs: 1, target_pc: loop_end, - }, - _ => unreachable!(), - }, - loop_end, - ); + } + } + _ => unreachable!(), + }); if *cmp_op == ast::Operator::Less || *cmp_op == ast::Operator::LessEquals { translate_expr( program, @@ -452,7 +442,7 @@ pub fn open_loop( )?; } - program.defer_label_resolution(loop_start, program.offset() as usize); + program.resolve_label(loop_start, program.offset()); // TODO: We are currently only handling ascending indexes. // For conditions like index_key > 10, we have already seeked to the first key greater than 10, and can just scan forward. // For conditions like index_key < 10, we are at the beginning of the index, and will scan forward and emit IdxGE(10) with a conditional jump to the end. @@ -466,56 +456,44 @@ pub fn open_loop( match cmp_op { ast::Operator::Equals | ast::Operator::LessEquals => { if let Some(index_cursor_id) = index_cursor_id { - program.emit_insn_with_label_dependency( - Insn::IdxGT { - cursor_id: index_cursor_id, - start_reg: cmp_reg, - num_regs: 1, - target_pc: loop_end, - }, - loop_end, - ); + program.emit_insn(Insn::IdxGT { + cursor_id: index_cursor_id, + start_reg: cmp_reg, + num_regs: 1, + target_pc: loop_end, + }); } else { let rowid_reg = program.alloc_register(); program.emit_insn(Insn::RowId { cursor_id: table_cursor_id, dest: rowid_reg, }); - program.emit_insn_with_label_dependency( - Insn::Gt { - lhs: rowid_reg, - rhs: cmp_reg, - target_pc: loop_end, - }, - loop_end, - ); + program.emit_insn(Insn::Gt { + lhs: rowid_reg, + rhs: cmp_reg, + target_pc: loop_end, + }); } } ast::Operator::Less => { if let Some(index_cursor_id) = index_cursor_id { - program.emit_insn_with_label_dependency( - Insn::IdxGE { - cursor_id: index_cursor_id, - start_reg: cmp_reg, - num_regs: 1, - target_pc: loop_end, - }, - loop_end, - ); + program.emit_insn(Insn::IdxGE { + cursor_id: index_cursor_id, + start_reg: cmp_reg, + num_regs: 1, + target_pc: loop_end, + }); } else { let rowid_reg = program.alloc_register(); program.emit_insn(Insn::RowId { cursor_id: table_cursor_id, dest: rowid_reg, }); - program.emit_insn_with_label_dependency( - Insn::Ge { - lhs: rowid_reg, - rhs: cmp_reg, - target_pc: loop_end, - }, - loop_end, - ); + program.emit_insn(Insn::Ge { + lhs: rowid_reg, + rhs: cmp_reg, + target_pc: loop_end, + }); } } _ => {} @@ -538,14 +516,11 @@ pub fn open_loop( src_reg, &t_ctx.resolver, )?; - program.emit_insn_with_label_dependency( - Insn::SeekRowid { - cursor_id: table_cursor_id, - src_reg, - target_pc: next, - }, - next, - ); + program.emit_insn(Insn::SeekRowid { + cursor_id: table_cursor_id, + src_reg, + target_pc: next, + }); } if let Some(predicates) = predicates { for predicate in predicates.iter() { @@ -748,12 +723,9 @@ pub fn close_loop( // A subquery has no cursor to call NextAsync on, so it just emits a Goto // to the Yield instruction, which in turn jumps back to the main loop of the subquery, // so that the next row from the subquery can be read. - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: loop_labels.loop_start, - }, - loop_labels.loop_start, - ); + program.emit_insn(Insn::Goto { + target_pc: loop_labels.loop_start, + }); } SourceOperator::Join { id, @@ -771,7 +743,7 @@ pub fn close_loop( // If the left join match flag has been set to 1, we jump to the next row on the outer table, // i.e. continue to the next row of t1 in our example. program.resolve_label(lj_meta.label_match_flag_check_value, program.offset()); - let jump_offset = program.offset() + 3; + let jump_offset = program.offset().add(3u32); program.emit_insn(Insn::IfPos { reg: lj_meta.reg_match_flag, target_pc: jump_offset, @@ -799,12 +771,9 @@ pub fn close_loop( // and we will end up back in the IfPos instruction above, which will then // check the match flag again, and since it is now 1, we will jump to the // next row in the left table. - program.emit_insn_with_label_dependency( - Insn::Goto { - target_pc: lj_meta.label_match_flag_set_true, - }, - lj_meta.label_match_flag_set_true, - ); + program.emit_insn(Insn::Goto { + target_pc: lj_meta.label_match_flag_set_true, + }); assert!(program.offset() == jump_offset); } @@ -830,21 +799,15 @@ pub fn close_loop( .as_ref() .is_some_and(|dir| *dir == IterationDirection::Backwards) { - program.emit_insn_with_label_dependency( - Insn::PrevAwait { - cursor_id, - pc_if_next: loop_labels.loop_start, - }, - loop_labels.loop_start, - ); + program.emit_insn(Insn::PrevAwait { + cursor_id, + pc_if_next: loop_labels.loop_start, + }); } else { - program.emit_insn_with_label_dependency( - Insn::NextAwait { - cursor_id, - pc_if_next: loop_labels.loop_start, - }, - loop_labels.loop_start, - ); + program.emit_insn(Insn::NextAwait { + cursor_id, + pc_if_next: loop_labels.loop_start, + }); } } SourceOperator::Search { @@ -866,13 +829,10 @@ pub fn close_loop( }; program.emit_insn(Insn::NextAsync { cursor_id }); - program.emit_insn_with_label_dependency( - Insn::NextAwait { - cursor_id, - pc_if_next: loop_labels.loop_start, - }, - loop_labels.loop_start, - ); + program.emit_insn(Insn::NextAwait { + cursor_id, + pc_if_next: loop_labels.loop_start, + }); } SourceOperator::Nothing { .. } => {} }; diff --git a/core/translate/mod.rs b/core/translate/mod.rs index f8adb0e09..92b661ce1 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -388,12 +388,9 @@ fn translate_create_table( if schema.get_table(tbl_name.name.0.as_str()).is_some() { if if_not_exists { let init_label = program.allocate_label(); - program.emit_insn_with_label_dependency( - Insn::Init { - target_pc: init_label, - }, - init_label, - ); + program.emit_insn(Insn::Init { + target_pc: init_label, + }); let start_offset = program.offset(); program.emit_insn(Insn::Halt { err_code: 0, @@ -414,12 +411,9 @@ fn translate_create_table( let parse_schema_label = program.allocate_label(); let init_label = program.allocate_label(); - program.emit_insn_with_label_dependency( - Insn::Init { - target_pc: init_label, - }, - init_label, - ); + program.emit_insn(Insn::Init { + target_pc: init_label, + }); let start_offset = program.offset(); // TODO: ReadCookie // TODO: If @@ -544,12 +538,9 @@ fn translate_pragma( ) -> Result { let mut program = ProgramBuilder::new(); let init_label = program.allocate_label(); - program.emit_insn_with_label_dependency( - Insn::Init { - target_pc: init_label, - }, - init_label, - ); + program.emit_insn(Insn::Init { + target_pc: init_label, + }); let start_offset = program.offset(); let mut write = false; match body { @@ -581,7 +572,6 @@ fn translate_pragma( program.emit_insn(Insn::Goto { target_pc: start_offset, }); - program.resolve_deferred_labels(); Ok(program.build(database_header, connection)) } diff --git a/core/translate/order_by.rs b/core/translate/order_by.rs index d05fccd60..b58e7ec21 100644 --- a/core/translate/order_by.rs +++ b/core/translate/order_by.rs @@ -110,15 +110,12 @@ pub fn emit_order_by( num_fields: num_columns_in_sorter, }); - program.emit_insn_with_label_dependency( - Insn::SorterSort { - cursor_id: sort_cursor, - pc_if_empty: sort_loop_end_label, - }, - sort_loop_end_label, - ); + program.emit_insn(Insn::SorterSort { + cursor_id: sort_cursor, + pc_if_empty: sort_loop_end_label, + }); - program.defer_label_resolution(sort_loop_start_label, program.offset() as usize); + program.resolve_label(sort_loop_start_label, program.offset()); program.emit_insn(Insn::SorterData { cursor_id: sort_cursor, dest_reg: reg_sorter_data, @@ -140,13 +137,10 @@ pub fn emit_order_by( emit_result_row_and_limit(program, t_ctx, plan, start_reg, Some(sort_loop_end_label))?; - program.emit_insn_with_label_dependency( - Insn::SorterNext { - cursor_id: sort_cursor, - pc_if_next: sort_loop_start_label, - }, - sort_loop_start_label, - ); + program.emit_insn(Insn::SorterNext { + cursor_id: sort_cursor, + pc_if_next: sort_loop_start_label, + }); program.resolve_label(sort_loop_end_label, program.offset()); diff --git a/core/translate/planner.rs b/core/translate/planner.rs index 1f0bf687b..b12391579 100644 --- a/core/translate/planner.rs +++ b/core/translate/planner.rs @@ -294,7 +294,7 @@ fn parse_from_clause_table( }; subplan.query_type = SelectQueryType::Subquery { yield_reg: usize::MAX, // will be set later in bytecode emission - coroutine_implementation_start: BranchOffset::MAX, // will be set later in bytecode emission + coroutine_implementation_start: BranchOffset::Placeholder, // will be set later in bytecode emission }; let identifier = maybe_alias .map(|a| match a { diff --git a/core/translate/result_row.rs b/core/translate/result_row.rs index 4901cc9ec..5c76f3008 100644 --- a/core/translate/result_row.rs +++ b/core/translate/result_row.rs @@ -54,7 +54,7 @@ pub fn emit_result_row_and_limit( SelectQueryType::Subquery { yield_reg, .. } => { program.emit_insn(Insn::Yield { yield_reg: *yield_reg, - end_offset: 0, + end_offset: BranchOffset::Offset(0), }); } } @@ -71,13 +71,10 @@ pub fn emit_result_row_and_limit( dest: t_ctx.reg_limit.unwrap(), }); program.mark_last_insn_constant(); - program.emit_insn_with_label_dependency( - Insn::DecrJumpZero { - reg: t_ctx.reg_limit.unwrap(), - target_pc: label_on_limit_reached.unwrap(), - }, - label_on_limit_reached.unwrap(), - ); + program.emit_insn(Insn::DecrJumpZero { + reg: t_ctx.reg_limit.unwrap(), + target_pc: label_on_limit_reached.unwrap(), + }); } Ok(()) } diff --git a/core/translate/subquery.rs b/core/translate/subquery.rs index cb0527a6d..206da45ba 100644 --- a/core/translate/subquery.rs +++ b/core/translate/subquery.rs @@ -72,7 +72,7 @@ pub fn emit_subquery<'a>( t_ctx: &mut TranslateCtx<'a>, ) -> Result { let yield_reg = program.alloc_register(); - let coroutine_implementation_start_offset = program.offset() + 1; + let coroutine_implementation_start_offset = program.offset().add(1u32); match &mut plan.query_type { SelectQueryType::Subquery { yield_reg: y, @@ -100,14 +100,11 @@ pub fn emit_subquery<'a>( resolver: Resolver::new(t_ctx.resolver.symbol_table), }; let subquery_body_end_label = program.allocate_label(); - program.emit_insn_with_label_dependency( - Insn::InitCoroutine { - yield_reg, - jump_on_definition: subquery_body_end_label, - start_offset: coroutine_implementation_start_offset, - }, - subquery_body_end_label, - ); + program.emit_insn(Insn::InitCoroutine { + yield_reg, + jump_on_definition: subquery_body_end_label, + start_offset: coroutine_implementation_start_offset, + }); // Normally we mark each LIMIT value as a constant insn that is emitted only once, but in the case of a subquery, // we need to initialize it every time the subquery is run; otherwise subsequent runs of the subquery will already // have the LIMIT counter at 0, and will never return rows. diff --git a/core/vdbe/builder.rs b/core/vdbe/builder.rs index 561765738..40f936cd1 100644 --- a/core/vdbe/builder.rs +++ b/core/vdbe/builder.rs @@ -11,23 +11,20 @@ use super::{BranchOffset, CursorID, Insn, InsnReference, Program, Table}; #[allow(dead_code)] pub struct ProgramBuilder { next_free_register: usize, - next_free_label: BranchOffset, + next_free_label: i32, next_free_cursor_id: usize, insns: Vec, // for temporarily storing instructions that will be put after Transaction opcode constant_insns: Vec, - // Each label has a list of InsnReferences that must - // be resolved. Lists are indexed by: label.abs() - 1 - unresolved_labels: Vec>, next_insn_label: Option, // Cursors that are referenced by the program. Indexed by CursorID. pub cursor_ref: Vec<(Option, Option)>, - // List of deferred label resolutions. Each entry is a pair of (label, insn_reference). - deferred_label_resolutions: Vec<(BranchOffset, InsnReference)>, + // Hashmap of label to insn reference. Resolved in build(). + label_to_resolved_offset: HashMap, // Bitmask of cursors that have emitted a SeekRowid instruction. seekrowid_emitted_bitmask: u64, // map of instruction index to manual comment (used in EXPLAIN) - comments: HashMap, + comments: HashMap, } impl ProgramBuilder { @@ -37,11 +34,10 @@ impl ProgramBuilder { next_free_label: 0, next_free_cursor_id: 0, insns: Vec::new(), - unresolved_labels: Vec::new(), next_insn_label: None, cursor_ref: Vec::new(), constant_insns: Vec::new(), - deferred_label_resolutions: Vec::new(), + label_to_resolved_offset: HashMap::new(), seekrowid_emitted_bitmask: 0, comments: HashMap::new(), } @@ -71,20 +67,17 @@ impl ProgramBuilder { cursor } - fn _emit_insn(&mut self, insn: Insn) { - self.insns.push(insn); - } - pub fn emit_insn(&mut self, insn: Insn) { - self._emit_insn(insn); if let Some(label) = self.next_insn_label { + self.label_to_resolved_offset + .insert(label.to_label_value(), self.insns.len() as InsnReference); self.next_insn_label = None; - self.resolve_label(label, (self.insns.len() - 1) as BranchOffset); } + self.insns.push(insn); } pub fn add_comment(&mut self, insn_index: BranchOffset, comment: &'static str) { - self.comments.insert(insn_index, comment); + self.comments.insert(insn_index.to_offset_int(), comment); } // Emit an instruction that will be put at the end of the program (after Transaction statement). @@ -99,19 +92,13 @@ impl ProgramBuilder { self.insns.append(&mut self.constant_insns); } - pub fn emit_insn_with_label_dependency(&mut self, insn: Insn, label: BranchOffset) { - self._emit_insn(insn); - self.add_label_dependency(label, (self.insns.len() - 1) as BranchOffset); - } - pub fn offset(&self) -> BranchOffset { - self.insns.len() as BranchOffset + BranchOffset::Offset(self.insns.len() as InsnReference) } pub fn allocate_label(&mut self) -> BranchOffset { self.next_free_label -= 1; - self.unresolved_labels.push(Vec::new()); - self.next_free_label + BranchOffset::Label(self.next_free_label) } // Effectively a GOTO without the need to emit an explicit GOTO instruction. @@ -121,232 +108,187 @@ impl ProgramBuilder { self.next_insn_label = Some(label); } - fn label_to_index(&self, label: BranchOffset) -> usize { - (label.abs() - 1) as usize - } - - pub fn add_label_dependency(&mut self, label: BranchOffset, insn_reference: BranchOffset) { - assert!(insn_reference >= 0); - assert!(label < 0); - let label_index = self.label_to_index(label); - assert!(label_index < self.unresolved_labels.len()); - let insn_reference = insn_reference as InsnReference; - let label_references = &mut self.unresolved_labels[label_index]; - label_references.push(insn_reference); - } - - pub fn defer_label_resolution(&mut self, label: BranchOffset, insn_reference: InsnReference) { - self.deferred_label_resolutions - .push((label, insn_reference)); + pub fn resolve_label(&mut self, label: BranchOffset, to_offset: BranchOffset) { + assert!(matches!(label, BranchOffset::Label(_))); + assert!(matches!(to_offset, BranchOffset::Offset(_))); + self.label_to_resolved_offset + .insert(label.to_label_value(), to_offset.to_offset_int()); } /// Resolve unresolved labels to a specific offset in the instruction list. /// - /// This function updates all instructions that reference the given label - /// to point to the specified offset. It ensures that the label and offset - /// are valid and updates the target program counter (PC) of each instruction - /// that references the label. - /// - /// # Arguments - /// - /// * `label` - The label to resolve. - /// * `to_offset` - The offset to which the labeled instructions should be resolved to. - pub fn resolve_label(&mut self, label: BranchOffset, to_offset: BranchOffset) { - assert!(label < 0); - assert!(to_offset >= 0); - let label_index = self.label_to_index(label); - assert!( - label_index < self.unresolved_labels.len(), - "Forbidden resolve of an unexistent label!" - ); - - let label_references = &mut self.unresolved_labels[label_index]; - for insn_reference in label_references.iter() { - let insn = &mut self.insns[*insn_reference]; + /// This function scans all instructions and resolves any labels to their corresponding offsets. + /// It ensures that all labels are resolved correctly and updates the target program counter (PC) + /// of each instruction that references a label. + pub fn resolve_labels(&mut self) { + let resolve = |pc: &mut BranchOffset, insn_name: &str| { + if let BranchOffset::Label(label) = pc { + let to_offset = *self.label_to_resolved_offset.get(label).unwrap_or_else(|| { + panic!("Reference to undefined label in {}: {}", insn_name, label) + }); + *pc = BranchOffset::Offset(to_offset); + } + }; + for insn in self.insns.iter_mut() { match insn { Insn::Init { target_pc } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "Init"); } Insn::Eq { lhs: _lhs, rhs: _rhs, target_pc, } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "Eq"); } Insn::Ne { lhs: _lhs, rhs: _rhs, target_pc, } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "Ne"); } Insn::Lt { lhs: _lhs, rhs: _rhs, target_pc, } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "Lt"); } Insn::Le { lhs: _lhs, rhs: _rhs, target_pc, } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "Le"); } Insn::Gt { lhs: _lhs, rhs: _rhs, target_pc, } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "Gt"); } Insn::Ge { lhs: _lhs, rhs: _rhs, target_pc, } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "Ge"); } Insn::If { reg: _reg, target_pc, null_reg: _, } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "If"); } Insn::IfNot { reg: _reg, target_pc, null_reg: _, } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "IfNot"); } Insn::RewindAwait { cursor_id: _cursor_id, pc_if_empty, } => { - assert!(*pc_if_empty < 0); - *pc_if_empty = to_offset; + resolve(pc_if_empty, "RewindAwait"); } Insn::LastAwait { cursor_id: _cursor_id, pc_if_empty, } => { - assert!(*pc_if_empty < 0); - *pc_if_empty = to_offset; + resolve(pc_if_empty, "LastAwait"); } Insn::Goto { target_pc } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "Goto"); } Insn::DecrJumpZero { reg: _reg, target_pc, } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "DecrJumpZero"); } Insn::SorterNext { cursor_id: _cursor_id, pc_if_next, } => { - assert!(*pc_if_next < 0); - *pc_if_next = to_offset; + resolve(pc_if_next, "SorterNext"); } Insn::SorterSort { pc_if_empty, .. } => { - assert!(*pc_if_empty < 0); - *pc_if_empty = to_offset; + resolve(pc_if_empty, "SorterSort"); } Insn::NotNull { reg: _reg, target_pc, } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "NotNull"); } Insn::IfPos { target_pc, .. } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "IfPos"); } Insn::NextAwait { pc_if_next, .. } => { - assert!(*pc_if_next < 0); - *pc_if_next = to_offset; + resolve(pc_if_next, "NextAwait"); } Insn::PrevAwait { pc_if_next, .. } => { - assert!(*pc_if_next < 0); - *pc_if_next = to_offset; + resolve(pc_if_next, "PrevAwait"); } Insn::InitCoroutine { yield_reg: _, jump_on_definition, start_offset: _, } => { - *jump_on_definition = to_offset; + resolve(jump_on_definition, "InitCoroutine"); } Insn::NotExists { cursor: _, rowid_reg: _, target_pc, } => { - *target_pc = to_offset; + resolve(target_pc, "NotExists"); } Insn::Yield { yield_reg: _, end_offset, } => { - *end_offset = to_offset; + resolve(end_offset, "Yield"); } Insn::SeekRowid { target_pc, .. } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "SeekRowid"); } Insn::Gosub { target_pc, .. } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "Gosub"); } - Insn::Jump { target_pc_eq, .. } => { - // FIXME: this current implementation doesnt scale for insns that - // have potentially multiple label dependencies. - assert!(*target_pc_eq < 0); - *target_pc_eq = to_offset; + Insn::Jump { + target_pc_eq, + target_pc_lt, + target_pc_gt, + } => { + resolve(target_pc_eq, "Jump"); + resolve(target_pc_lt, "Jump"); + resolve(target_pc_gt, "Jump"); } Insn::SeekGE { target_pc, .. } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "SeekGE"); } Insn::SeekGT { target_pc, .. } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "SeekGT"); } Insn::IdxGE { target_pc, .. } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "IdxGE"); } Insn::IdxGT { target_pc, .. } => { - assert!(*target_pc < 0); - *target_pc = to_offset; + resolve(target_pc, "IdxGT"); } Insn::IsNull { src: _, target_pc } => { - assert!(*target_pc < 0); - *target_pc = to_offset; - } - _ => { - todo!("missing resolve_label for {:?}", insn); + resolve(target_pc, "IsNull"); } + _ => continue, } } - label_references.clear(); + self.label_to_resolved_offset.clear(); } // translate table to cursor id @@ -361,23 +303,12 @@ impl ProgramBuilder { .unwrap() } - pub fn resolve_deferred_labels(&mut self) { - for i in 0..self.deferred_label_resolutions.len() { - let (label, insn_reference) = self.deferred_label_resolutions[i]; - self.resolve_label(label, insn_reference as BranchOffset); - } - self.deferred_label_resolutions.clear(); - } - pub fn build( - self, + mut self, database_header: Rc>, connection: Weak, ) -> Program { - assert!( - self.deferred_label_resolutions.is_empty(), - "deferred_label_resolutions is not empty when build() is called, did you forget to call resolve_deferred_labels()?" - ); + self.resolve_labels(); assert!( self.constant_insns.is_empty(), "constant_insns is not empty when build() is called, did you forget to call emit_constant_insns()?" diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index a17ababcf..133172b78 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -13,11 +13,11 @@ pub fn insn_to_str( Insn::Init { target_pc } => ( "Init", 0, - *target_pc as i32, + target_pc.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("Start at {}", target_pc), + format!("Start at {}", target_pc.to_debug_int()), ), Insn::Add { lhs, rhs, dest } => ( "Add", @@ -114,11 +114,11 @@ pub fn insn_to_str( Insn::NotNull { reg, target_pc } => ( "NotNull", *reg as i32, - *target_pc as i32, + target_pc.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("r[{}]!=NULL -> goto {}", reg, target_pc), + format!("r[{}]!=NULL -> goto {}", reg, target_pc.to_debug_int()), ), Insn::Compare { start_reg_a, @@ -145,9 +145,9 @@ pub fn insn_to_str( target_pc_gt, } => ( "Jump", - *target_pc_lt as i32, - *target_pc_eq as i32, - *target_pc_gt as i32, + target_pc_lt.to_debug_int(), + target_pc_eq.to_debug_int(), + target_pc_gt.to_debug_int(), OwnedValue::build_text(Rc::new("".to_string())), 0, "".to_string(), @@ -178,13 +178,16 @@ pub fn insn_to_str( } => ( "IfPos", *reg as i32, - *target_pc as i32, + target_pc.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, format!( "r[{}]>0 -> r[{}]-={}, goto {}", - reg, reg, decrement_by, target_pc + reg, + reg, + decrement_by, + target_pc.to_debug_int() ), ), Insn::Eq { @@ -195,10 +198,15 @@ pub fn insn_to_str( "Eq", *lhs as i32, *rhs as i32, - *target_pc as i32, + target_pc.to_debug_int(), OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("if r[{}]==r[{}] goto {}", lhs, rhs, target_pc), + format!( + "if r[{}]==r[{}] goto {}", + lhs, + rhs, + target_pc.to_debug_int() + ), ), Insn::Ne { lhs, @@ -208,10 +216,15 @@ pub fn insn_to_str( "Ne", *lhs as i32, *rhs as i32, - *target_pc as i32, + target_pc.to_debug_int(), OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("if r[{}]!=r[{}] goto {}", lhs, rhs, target_pc), + format!( + "if r[{}]!=r[{}] goto {}", + lhs, + rhs, + target_pc.to_debug_int() + ), ), Insn::Lt { lhs, @@ -221,10 +234,10 @@ pub fn insn_to_str( "Lt", *lhs as i32, *rhs as i32, - *target_pc as i32, + target_pc.to_debug_int(), OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("if r[{}]r[{}] goto {}", lhs, rhs, target_pc), + format!("if r[{}]>r[{}] goto {}", lhs, rhs, target_pc.to_debug_int()), ), Insn::Ge { lhs, @@ -260,10 +278,15 @@ pub fn insn_to_str( "Ge", *lhs as i32, *rhs as i32, - *target_pc as i32, + target_pc.to_debug_int(), OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("if r[{}]>=r[{}] goto {}", lhs, rhs, target_pc), + format!( + "if r[{}]>=r[{}] goto {}", + lhs, + rhs, + target_pc.to_debug_int() + ), ), Insn::If { reg, @@ -272,11 +295,11 @@ pub fn insn_to_str( } => ( "If", *reg as i32, - *target_pc as i32, + target_pc.to_debug_int(), *null_reg as i32, OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("if r[{}] goto {}", reg, target_pc), + format!("if r[{}] goto {}", reg, target_pc.to_debug_int()), ), Insn::IfNot { reg, @@ -285,11 +308,11 @@ pub fn insn_to_str( } => ( "IfNot", *reg as i32, - *target_pc as i32, + target_pc.to_debug_int(), *null_reg as i32, OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("if !r[{}] goto {}", reg, target_pc), + format!("if !r[{}] goto {}", reg, target_pc.to_debug_int()), ), Insn::OpenReadAsync { cursor_id, @@ -347,7 +370,7 @@ pub fn insn_to_str( } => ( "RewindAwait", *cursor_id as i32, - *pc_if_empty as i32, + pc_if_empty.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -431,7 +454,7 @@ pub fn insn_to_str( } => ( "NextAwait", *cursor_id as i32, - *pc_if_next as i32, + pc_if_next.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -461,7 +484,7 @@ pub fn insn_to_str( Insn::Goto { target_pc } => ( "Goto", 0, - *target_pc as i32, + target_pc.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -473,7 +496,7 @@ pub fn insn_to_str( } => ( "Gosub", *return_reg as i32, - *target_pc as i32, + target_pc.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -562,7 +585,7 @@ pub fn insn_to_str( "SeekRowid", *cursor_id as i32, *src_reg as i32, - *target_pc as i32, + target_pc.to_debug_int(), OwnedValue::build_text(Rc::new("".to_string())), 0, format!( @@ -572,7 +595,7 @@ pub fn insn_to_str( .0 .as_ref() .unwrap_or(&format!("cursor {}", cursor_id)), - target_pc + target_pc.to_debug_int() ), ), Insn::DeferredSeek { @@ -596,7 +619,7 @@ pub fn insn_to_str( } => ( "SeekGT", *cursor_id as i32, - *target_pc as i32, + target_pc.to_debug_int(), *start_reg as i32, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -611,7 +634,7 @@ pub fn insn_to_str( } => ( "SeekGE", *cursor_id as i32, - *target_pc as i32, + target_pc.to_debug_int(), *start_reg as i32, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -625,7 +648,7 @@ pub fn insn_to_str( } => ( "IdxGT", *cursor_id as i32, - *target_pc as i32, + target_pc.to_debug_int(), *start_reg as i32, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -639,7 +662,7 @@ pub fn insn_to_str( } => ( "IdxGE", *cursor_id as i32, - *target_pc as i32, + target_pc.to_debug_int(), *start_reg as i32, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -648,11 +671,11 @@ pub fn insn_to_str( Insn::DecrJumpZero { reg, target_pc } => ( "DecrJumpZero", *reg as i32, - *target_pc as i32, + target_pc.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("if (--r[{}]==0) goto {}", reg, target_pc), + format!("if (--r[{}]==0) goto {}", reg, target_pc.to_debug_int()), ), Insn::AggStep { func, @@ -742,7 +765,7 @@ pub fn insn_to_str( } => ( "SorterSort", *cursor_id as i32, - *pc_if_empty as i32, + pc_if_empty.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -754,7 +777,7 @@ pub fn insn_to_str( } => ( "SorterNext", *cursor_id as i32, - *pc_if_next as i32, + pc_if_next.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -792,8 +815,8 @@ pub fn insn_to_str( } => ( "InitCoroutine", *yield_reg as i32, - *jump_on_definition as i32, - *start_offset as i32, + jump_on_definition.to_debug_int(), + start_offset.to_debug_int(), OwnedValue::build_text(Rc::new("".to_string())), 0, "".to_string(), @@ -813,7 +836,7 @@ pub fn insn_to_str( } => ( "Yield", *yield_reg as i32, - *end_offset as i32, + end_offset.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -898,7 +921,7 @@ pub fn insn_to_str( } => ( "NotExists", *cursor as i32, - *target_pc as i32, + target_pc.to_debug_int(), *rowid_reg as i32, OwnedValue::build_text(Rc::new("".to_string())), 0, @@ -968,11 +991,11 @@ pub fn insn_to_str( Insn::IsNull { src, target_pc } => ( "IsNull", *src as i32, - *target_pc as i32, + target_pc.to_debug_int(), 0, OwnedValue::build_text(Rc::new("".to_string())), 0, - format!("if (r[{}]==NULL) goto {}", src, target_pc), + format!("if (r[{}]==NULL) goto {}", src, target_pc.to_debug_int()), ), Insn::ParseSchema { db, where_clause } => ( "ParseSchema", diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index a0b42abb6..c9151e934 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -58,13 +58,75 @@ use std::cell::RefCell; use std::collections::{BTreeMap, HashMap}; use std::rc::{Rc, Weak}; -pub type BranchOffset = i64; +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +/// Represents a target for a jump instruction. +/// Stores 32-bit ints to keep the enum word-sized. +pub enum BranchOffset { + /// A label is a named location in the program. + /// If there are references to it, it must always be resolved to an Offset + /// via program.resolve_label(). + Label(i32), + /// An offset is a direct index into the instruction list. + Offset(InsnReference), + /// A placeholder is a temporary value to satisfy the compiler. + /// It must be set later. + Placeholder, +} + +impl BranchOffset { + /// Returns true if the branch offset is a label. + pub fn is_label(&self) -> bool { + matches!(self, BranchOffset::Label(_)) + } + + /// Returns true if the branch offset is an offset. + pub fn is_offset(&self) -> bool { + matches!(self, BranchOffset::Offset(_)) + } + + /// Returns the offset value. Panics if the branch offset is a label or placeholder. + pub fn to_offset_int(&self) -> InsnReference { + match self { + BranchOffset::Label(v) => unreachable!("Unresolved label: {}", v), + BranchOffset::Offset(v) => *v, + BranchOffset::Placeholder => unreachable!("Unresolved placeholder"), + } + } + + /// Returns the label value. Panics if the branch offset is an offset or placeholder. + pub fn to_label_value(&self) -> i32 { + match self { + BranchOffset::Label(v) => *v, + BranchOffset::Offset(_) => unreachable!("Offset cannot be converted to label value"), + BranchOffset::Placeholder => unreachable!("Unresolved placeholder"), + } + } + + /// Returns the branch offset as a signed integer. + /// Used in explain output, where we don't want to panic in case we have an unresolved + /// label or placeholder. + pub fn to_debug_int(&self) -> i32 { + match self { + BranchOffset::Label(v) => *v, + BranchOffset::Offset(v) => *v as i32, + BranchOffset::Placeholder => i32::MAX, + } + } + + /// Adds an integer value to the branch offset. + /// Returns a new branch offset. + /// Panics if the branch offset is a label or placeholder. + pub fn add>(self, n: N) -> BranchOffset { + BranchOffset::Offset(self.to_offset_int() + n.into()) + } +} + pub type CursorID = usize; pub type PageIdx = usize; // Index of insn in list of insns -type InsnReference = usize; +type InsnReference = u32; pub enum StepResult<'a> { Done, @@ -101,7 +163,7 @@ impl RegexCache { /// The program state describes the environment in which the program executes. pub struct ProgramState { - pub pc: BranchOffset, + pub pc: InsnReference, cursors: RefCell>>, registers: Vec, last_compare: Option, @@ -151,7 +213,7 @@ pub struct Program { pub insns: Vec, pub cursor_ref: Vec<(Option, Option
)>, pub database_header: Rc>, - pub comments: HashMap, + pub comments: HashMap, pub connection: Weak, pub auto_commit: bool, } @@ -189,8 +251,8 @@ impl Program { let mut cursors = state.cursors.borrow_mut(); match insn { Insn::Init { target_pc } => { - assert!(*target_pc >= 0); - state.pc = *target_pc; + assert!(target_pc.is_offset()); + state.pc = target_pc.to_offset_int(); } Insn::Add { lhs, rhs, dest } => { state.registers[*dest] = @@ -278,6 +340,9 @@ impl Program { target_pc_eq, target_pc_gt, } => { + assert!(target_pc_lt.is_offset()); + assert!(target_pc_eq.is_offset()); + assert!(target_pc_gt.is_offset()); let cmp = state.last_compare.take(); if cmp.is_none() { return Err(LimboError::InternalError( @@ -289,8 +354,7 @@ impl Program { std::cmp::Ordering::Equal => *target_pc_eq, std::cmp::Ordering::Greater => *target_pc_gt, }; - assert!(target_pc >= 0); - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } Insn::Move { source_reg, @@ -313,12 +377,12 @@ impl Program { target_pc, decrement_by, } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); let reg = *reg; let target_pc = *target_pc; match &state.registers[reg] { OwnedValue::Integer(n) if *n > 0 => { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); state.registers[reg] = OwnedValue::Integer(*n - *decrement_by as i64); } OwnedValue::Integer(_) => { @@ -332,7 +396,7 @@ impl Program { } } Insn::NotNull { reg, target_pc } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); let reg = *reg; let target_pc = *target_pc; match &state.registers[reg] { @@ -340,7 +404,7 @@ impl Program { state.pc += 1; } _ => { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } } } @@ -350,17 +414,17 @@ impl Program { rhs, target_pc, } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } _ => { if state.registers[lhs] == state.registers[rhs] { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -372,17 +436,17 @@ impl Program { rhs, target_pc, } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } _ => { if state.registers[lhs] != state.registers[rhs] { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -394,17 +458,17 @@ impl Program { rhs, target_pc, } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } _ => { if state.registers[lhs] < state.registers[rhs] { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -416,17 +480,17 @@ impl Program { rhs, target_pc, } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } _ => { if state.registers[lhs] <= state.registers[rhs] { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -438,17 +502,17 @@ impl Program { rhs, target_pc, } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } _ => { if state.registers[lhs] > state.registers[rhs] { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -460,17 +524,17 @@ impl Program { rhs, target_pc, } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); let lhs = *lhs; let rhs = *rhs; let target_pc = *target_pc; match (&state.registers[lhs], &state.registers[rhs]) { (_, OwnedValue::Null) | (OwnedValue::Null, _) => { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } _ => { if state.registers[lhs] >= state.registers[rhs] { - state.pc = target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -482,9 +546,9 @@ impl Program { target_pc, null_reg, } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); if exec_if(&state.registers[*reg], &state.registers[*null_reg], false) { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -494,9 +558,9 @@ impl Program { target_pc, null_reg, } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); if exec_if(&state.registers[*reg], &state.registers[*null_reg], true) { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -539,10 +603,11 @@ impl Program { cursor_id, pc_if_empty, } => { + assert!(pc_if_empty.is_offset()); let cursor = cursors.get_mut(cursor_id).unwrap(); cursor.wait_for_completion()?; if cursor.is_empty() { - state.pc = *pc_if_empty; + state.pc = pc_if_empty.to_offset_int(); } else { state.pc += 1; } @@ -551,10 +616,11 @@ impl Program { cursor_id, pc_if_empty, } => { + assert!(pc_if_empty.is_offset()); let cursor = cursors.get_mut(cursor_id).unwrap(); cursor.wait_for_completion()?; if cursor.is_empty() { - state.pc = *pc_if_empty; + state.pc = pc_if_empty.to_offset_int(); } else { state.pc += 1; } @@ -620,11 +686,11 @@ impl Program { cursor_id, pc_if_next, } => { - assert!(*pc_if_next >= 0); + assert!(pc_if_next.is_offset()); let cursor = cursors.get_mut(cursor_id).unwrap(); cursor.wait_for_completion()?; if !cursor.is_empty() { - state.pc = *pc_if_next; + state.pc = pc_if_next.to_offset_int(); } else { state.pc += 1; } @@ -633,11 +699,11 @@ impl Program { cursor_id, pc_if_next, } => { - assert!(*pc_if_next >= 0); + assert!(pc_if_next.is_offset()); let cursor = cursors.get_mut(cursor_id).unwrap(); cursor.wait_for_completion()?; if !cursor.is_empty() { - state.pc = *pc_if_next; + state.pc = pc_if_next.to_offset_int(); } else { state.pc += 1; } @@ -705,24 +771,22 @@ impl Program { state.pc += 1; } Insn::Goto { target_pc } => { - assert!(*target_pc >= 0); - state.pc = *target_pc; + assert!(target_pc.is_offset()); + state.pc = target_pc.to_offset_int(); } Insn::Gosub { target_pc, return_reg, } => { - assert!(*target_pc >= 0); - state.registers[*return_reg] = OwnedValue::Integer(state.pc + 1); - state.pc = *target_pc; + assert!(target_pc.is_offset()); + state.registers[*return_reg] = OwnedValue::Integer((state.pc + 1) as i64); + state.pc = target_pc.to_offset_int(); } Insn::Return { return_reg } => { if let OwnedValue::Integer(pc) = state.registers[*return_reg] { - if pc < 0 { - return Err(LimboError::InternalError( - "Return register is negative".to_string(), - )); - } + let pc: u32 = pc + .try_into() + .unwrap_or_else(|_| panic!("Return register is negative: {}", pc)); state.pc = pc; } else { return Err(LimboError::InternalError( @@ -779,11 +843,12 @@ impl Program { src_reg, target_pc, } => { + assert!(target_pc.is_offset()); let cursor = cursors.get_mut(cursor_id).unwrap(); let rowid = match &state.registers[*src_reg] { OwnedValue::Integer(rowid) => *rowid as u64, OwnedValue::Null => { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); continue; } other => { @@ -794,7 +859,7 @@ impl Program { }; let found = return_if_io!(cursor.seek(SeekKey::TableRowId(rowid), SeekOp::EQ)); if !found { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -813,6 +878,7 @@ impl Program { target_pc, is_index, } => { + assert!(target_pc.is_offset()); if *is_index { let cursor = cursors.get_mut(cursor_id).unwrap(); let record_from_regs: OwnedRecord = @@ -821,7 +887,7 @@ impl Program { cursor.seek(SeekKey::IndexKey(&record_from_regs), SeekOp::GE) ); if !found { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -844,7 +910,7 @@ impl Program { let found = return_if_io!(cursor.seek(SeekKey::TableRowId(rowid), SeekOp::GE)); if !found { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -857,6 +923,7 @@ impl Program { target_pc, is_index, } => { + assert!(target_pc.is_offset()); if *is_index { let cursor = cursors.get_mut(cursor_id).unwrap(); let record_from_regs: OwnedRecord = @@ -865,7 +932,7 @@ impl Program { cursor.seek(SeekKey::IndexKey(&record_from_regs), SeekOp::GT) ); if !found { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -888,7 +955,7 @@ impl Program { let found = return_if_io!(cursor.seek(SeekKey::TableRowId(rowid), SeekOp::GT)); if !found { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -900,7 +967,7 @@ impl Program { num_regs, target_pc, } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); let cursor = cursors.get_mut(cursor_id).unwrap(); let record_from_regs: OwnedRecord = make_owned_record(&state.registers, start_reg, num_regs); @@ -909,12 +976,12 @@ impl Program { if idx_record.values[..idx_record.values.len() - 1] >= *record_from_regs.values { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } } else { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } } Insn::IdxGT { @@ -923,6 +990,7 @@ impl Program { num_regs, target_pc, } => { + assert!(target_pc.is_offset()); let cursor = cursors.get_mut(cursor_id).unwrap(); let record_from_regs: OwnedRecord = make_owned_record(&state.registers, start_reg, num_regs); @@ -931,21 +999,21 @@ impl Program { if idx_record.values[..idx_record.values.len() - 1] > *record_from_regs.values { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } } else { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } } Insn::DecrJumpZero { reg, target_pc } => { - assert!(*target_pc >= 0); + assert!(target_pc.is_offset()); match state.registers[*reg] { OwnedValue::Integer(n) => { let n = n - 1; if n == 0 { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.registers[*reg] = OwnedValue::Integer(n); state.pc += 1; @@ -1250,18 +1318,18 @@ impl Program { cursor.rewind()?; state.pc += 1; } else { - state.pc = *pc_if_empty; + state.pc = pc_if_empty.to_offset_int(); } } Insn::SorterNext { cursor_id, pc_if_next, } => { - assert!(*pc_if_next >= 0); + assert!(pc_if_next.is_offset()); let cursor = cursors.get_mut(cursor_id).unwrap(); return_if_io!(cursor.next()); if !cursor.is_empty() { - state.pc = *pc_if_next; + state.pc = pc_if_next.to_offset_int(); } else { state.pc += 1; } @@ -1726,18 +1794,23 @@ impl Program { jump_on_definition, start_offset, } => { - assert!(*jump_on_definition >= 0); - state.registers[*yield_reg] = OwnedValue::Integer(*start_offset); + assert!(jump_on_definition.is_offset()); + let start_offset = start_offset.to_offset_int(); + state.registers[*yield_reg] = OwnedValue::Integer(start_offset as i64); state.ended_coroutine.insert(*yield_reg, false); - state.pc = if *jump_on_definition == 0 { + let jump_on_definition = jump_on_definition.to_offset_int(); + state.pc = if jump_on_definition == 0 { state.pc + 1 } else { - *jump_on_definition + jump_on_definition }; } Insn::EndCoroutine { yield_reg } => { if let OwnedValue::Integer(pc) = state.registers[*yield_reg] { state.ended_coroutine.insert(*yield_reg, true); + let pc: u32 = pc + .try_into() + .unwrap_or_else(|_| panic!("EndCoroutine: pc overflow: {}", pc)); state.pc = pc - 1; // yield jump is always next to yield. Here we substract 1 to go back to yield instruction } else { unreachable!(); @@ -1753,12 +1826,15 @@ impl Program { .get(yield_reg) .expect("coroutine not initialized") { - state.pc = *end_offset; + state.pc = end_offset.to_offset_int(); } else { + let pc: u32 = pc + .try_into() + .unwrap_or_else(|_| panic!("Yield: pc overflow: {}", pc)); // swap the program counter with the value in the yield register // this is the mechanism that allows jumping back and forth between the coroutine and the caller (state.pc, state.registers[*yield_reg]) = - (pc, OwnedValue::Integer(state.pc + 1)); + (pc, OwnedValue::Integer((state.pc + 1) as i64)); } } else { unreachable!( @@ -1842,7 +1918,7 @@ impl Program { if exists { state.pc += 1; } else { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } } // this cursor may be reused for next insert @@ -1894,7 +1970,7 @@ impl Program { } Insn::IsNull { src, target_pc } => { if matches!(state.registers[*src], OwnedValue::Null) { - state.pc = *target_pc; + state.pc = target_pc.to_offset_int(); } else { state.pc += 1; } @@ -1976,7 +2052,7 @@ fn trace_insn(program: &Program, addr: InsnReference, insn: &Insn) { addr, insn, String::new(), - program.comments.get(&(addr as BranchOffset)).copied() + program.comments.get(&(addr as u32)).copied() ) ); } @@ -1987,7 +2063,7 @@ fn print_insn(program: &Program, addr: InsnReference, insn: &Insn, indent: Strin addr, insn, indent, - program.comments.get(&(addr as BranchOffset)).copied(), + program.comments.get(&(addr as u32)).copied(), ); println!("{}", s); }