Skip to content

Commit

Permalink
Merge 'Simplify working with labels' from Jussi Saurio
Browse files Browse the repository at this point in the history 
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()

Closes #625
  • Loading branch information
@penberg
penberg committed Jan 7, 2025
2 parents a369329 + 731ff14 commit fbb5ddd
Show file tree
Hide file tree
Showing 13 changed files with 630 additions and 826 deletions.
40 changes: 13 additions & 27 deletions core/translate/emitter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -151,8 +148,6 @@ fn epilogue(
target_pc: start_offset,
});

program.resolve_deferred_labels();

Ok(())
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(())
Expand Down
405 changes: 153 additions & 252 deletions core/translate/expr.rs
View file

Large diffs are not rendered by default.

136 changes: 50 additions & 86 deletions core/translate/group_by.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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());
Expand All @@ -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() {
Expand All @@ -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,
Expand All @@ -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,
});
Expand Down Expand Up @@ -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,
)?;
Expand Down
Loading

0 comments on commit fbb5ddd

Please sign in to comment.