Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix block stack table #1557

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

- Fixed an issue with formatting of blocks in Miden Assembly syntax
- Fixed the construction of the block hash table (#1506)
- Fixed a bug in the block stack table (#1511) (#1512)
- Fixed a bug in the block stack table (#1511) (#1512) (#1557)
- Fixed the construction of the chiplets virtual table (#1514) (#1556)
- Fixed the construction of the chiplets bus (#1516) (#1525)
- Decorators are now allowed in empty basic blocks (#1466)
Expand Down
42 changes: 27 additions & 15 deletions processor/src/decoder/aux_trace/block_stack_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@ impl<E: FieldElement<BaseField = Felt>> AuxColumnBuilder<E> for BlockStackColumn
let op_code = op_code_felt.as_int() as u8;

match op_code {
OPCODE_RESPAN => {
get_block_stack_table_removal_multiplicand(main_trace, i, true, alphas)
},
OPCODE_END => get_block_stack_table_removal_multiplicand(main_trace, i, false, alphas),
OPCODE_RESPAN => get_block_stack_table_respan_multiplicand(main_trace, i, alphas),
OPCODE_END => get_block_stack_table_end_multiplicand(main_trace, i, alphas),
_ => E::ONE,
}
}
Expand All @@ -48,18 +46,33 @@ impl<E: FieldElement<BaseField = Felt>> AuxColumnBuilder<E> for BlockStackColumn
// ================================================================================================

/// Computes the multiplicand representing the removal of a row from the block stack table.
fn get_block_stack_table_removal_multiplicand<E: FieldElement<BaseField = Felt>>(
fn get_block_stack_table_respan_multiplicand<E: FieldElement<BaseField = Felt>>(
Comment on lines 48 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comment here needs to be updated.

main_trace: &MainTrace,
i: RowIndex,
is_respan: bool,
alphas: &[E],
) -> E {
let block_id = main_trace.addr(i);
let (parent_id, is_loop) = if is_respan {
(main_trace.decoder_hasher_state_element(1, i + 1), ZERO)
} else {
(main_trace.addr(i + 1), main_trace.is_loop_flag(i))
};
let parent_id = main_trace.decoder_hasher_state_element(1, i + 1);
let is_loop = ZERO;

// Note: the last 8 elements are set to ZERO, so we omit them here.
let elements = [ONE, block_id, parent_id, is_loop];

let mut table_row = E::ZERO;
for (&alpha, &element) in alphas.iter().zip(elements.iter()) {
table_row += alpha.mul_base(element);
}
table_row
}

fn get_block_stack_table_end_multiplicand<E: FieldElement<BaseField = Felt>>(
Comment on lines +67 to +68
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to add a brief comment here.

main_trace: &MainTrace,
i: RowIndex,
alphas: &[E],
) -> E {
let block_id = main_trace.addr(i);
let parent_id = main_trace.addr(i + 1);
let is_loop = main_trace.is_loop_flag(i);

let elements = if main_trace.is_call_flag(i) == ONE || main_trace.is_syscall_flag(i) == ONE {
let parent_ctx = main_trace.ctx(i + 1);
Expand Down Expand Up @@ -91,12 +104,11 @@ fn get_block_stack_table_removal_multiplicand<E: FieldElement<BaseField = Felt>>
result
};

let mut value = E::ZERO;

let mut table_row = E::ZERO;
for (&alpha, &element) in alphas.iter().zip(elements.iter()) {
value += alpha.mul_base(element);
table_row += alpha.mul_base(element);
}
value
table_row
}

/// Computes the multiplicand representing the inclusion of a new row to the block stack table.
Expand Down
5 changes: 5 additions & 0 deletions processor/src/decoder/aux_trace/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ impl AuxTraceBuilder {
let p2 = block_hash_column_builder.build_aux_column(main_trace, rand_elements);
let p3 = op_group_table_column_builder.build_aux_column(main_trace, rand_elements);

debug_assert_eq!(
*p1.last().unwrap(),
E::ONE,
"block stack table is not empty at the end of program execution"
);
debug_assert_eq!(
*p2.last().unwrap(),
E::ONE,
Expand Down
Loading