Skip to content

Commit

Permalink
fix: column indexing bugfix and formatting fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
bobbinth committed Sep 16, 2022
1 parent 963f162 commit dbc4368
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 35 deletions.
3 changes: 0 additions & 3 deletions air/src/stack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,14 @@ pub fn enforce_unique_constraints<E: FieldElement>(

// system operations transition constraints.
system_ops::enforce_constraints(frame, result, op_flag);

constraint_offset += system_ops::get_transition_constraint_count();

// field operations transition constraints.
field_ops::enforce_constraints(frame, &mut result[constraint_offset..], op_flag);

constraint_offset += field_ops::get_transition_constraint_count();

// stack manipulation operations transition constraints.
stack_manipulation::enforce_constraints(frame, &mut result[constraint_offset..], op_flag);

constraint_offset += stack_manipulation::get_transition_constraint_count();

constraint_offset
Expand Down
9 changes: 5 additions & 4 deletions air/src/stack/op_flags/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::EvaluationFrame;
use crate::utils::binary_not;
use vm_core::{
decoder::{NUM_OP_BITS, OP_BITS_RANGE, OP_BIT_EXTRA_COL_IDX, USER_OP_HELPERS_OFFSET},
decoder::{IS_LOOP_FLAG_COL_IDX, NUM_OP_BITS, OP_BITS_RANGE, OP_BIT_EXTRA_COL_IDX},
Felt, FieldElement, Operation, DECODER_TRACE_OFFSET, ONE, TRACE_WIDTH, ZERO,
};

Expand Down Expand Up @@ -229,6 +229,7 @@ impl<E: FieldElement> OpFlags<E> {
+ degree6_op_flags[13]
+ degree4_op_flags[6]
+ degree4_op_flags[7]
+ degree4_op_flags[3]
+ degree4_op_flags[4] * binary_not(frame.is_loop());

no_shift_flags[1] = no_shift_flags[0] + no_change_1_flag;
Expand Down Expand Up @@ -330,7 +331,7 @@ impl<E: FieldElement> OpFlags<E> {
f010 + add3_madd_flag + split_loop_flag + degree4_op_flags[5] + shift_left_on_end;

// Flag if the current operation being executed is a control flow operation.
let control_flow = f111 + f1011;
let control_flow = f111 + f1011 + degree4_op_flags[3];

Self {
degree7_op_flags,
Expand Down Expand Up @@ -905,7 +906,7 @@ impl<E: FieldElement> EvaluationFrameExt<E> for &EvaluationFrame<E> {

#[inline]
fn is_loop(&self) -> E {
self.current()[DECODER_TRACE_OFFSET + USER_OP_HELPERS_OFFSET + 4]
self.current()[DECODER_TRACE_OFFSET + IS_LOOP_FLAG_COL_IDX]
}
}

Expand Down Expand Up @@ -934,7 +935,7 @@ pub const fn get_right_shift(idx: usize) -> usize {
}

/// Accepts an integer which is a unique representation of an operation and returns
/// a trace constaining the op bits of the operation.
/// a trace containing the op bits of the operation.
pub fn generate_evaluation_frame(opcode: usize) -> EvaluationFrame<Felt> {
let operation_bit_array = get_op_bits(opcode);

Expand Down
6 changes: 3 additions & 3 deletions air/src/stack/op_flags/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ use super::{
DEGREE_6_OPCODE_ENDS, DEGREE_6_OPCODE_STARTS, DEGREE_7_OPCODE_ENDS, DEGREE_7_OPCODE_STARTS,
NUM_DEGREE_4_OPS, NUM_DEGREE_6_OPS, NUM_DEGREE_7_OPS,
};
use vm_core::{decoder::USER_OP_HELPERS_OFFSET, Operation, DECODER_TRACE_OFFSET, ONE, ZERO};
use vm_core::{decoder::IS_LOOP_FLAG_COL_IDX, Operation, DECODER_TRACE_OFFSET, ONE, ZERO};

/// Asserts the op flag to ONE for degree 7 operation which is being executed in the current
/// frame; assert all the other operation flags to ZERO as they are not present in the current
/// trace.
#[test]
fn degree_7_op_flags() {
for i in DEGREE_7_OPCODE_STARTS..=DEGREE_7_OPCODE_ENDS {
// frame initialised with a degree 7 operation using it's unique opcode.
// frame initialized with a degree 7 operation using it's unique opcode.
let frame = generate_evaluation_frame(i);

// All the operation flags are generated for the given frame.
Expand Down Expand Up @@ -541,7 +541,7 @@ fn composite_flags() {

// ----------------------------------- left shift -----------------------------------------------

frame.current_mut()[DECODER_TRACE_OFFSET + USER_OP_HELPERS_OFFSET + 4] = ONE;
frame.current_mut()[DECODER_TRACE_OFFSET + IS_LOOP_FLAG_COL_IDX] = ONE;

// All the operation flags are generated for the given frame.
let op_flags = OpFlags::new(&frame);
Expand Down
2 changes: 1 addition & 1 deletion air/src/stack/stack_manipulation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub fn enforce_constraints<E: FieldElement>(
) -> usize {
let mut index = 0;

// Enforce constaints of the SWAP operations.
// Enforce constraints of the SWAP operations.
index += enforce_swap_constraints(frame, result, op_flag.swap());

index
Expand Down
33 changes: 14 additions & 19 deletions assembly/src/parsers/io_ops/mem_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ use super::{
};
use vm_core::Felt;

// INSTRUCTION PARERS
// ================================================================================================

/// Appends operations to the span block to execute a memory read operation. This includes reading
/// a single element or an entire word from either local or global memory. Specifically, this
/// handles mem_load, mem_loadw, loc_load, and loc_loadw instructions.
Expand Down Expand Up @@ -142,6 +145,8 @@ mod tests {
AssemblyError, Operation, Token,
};

type ParserFn = fn(&mut Vec<Operation>, &Token, u32, bool, bool) -> Result<(), AssemblyError>;

// TESTS FOR READING FROM MEMORY
// ============================================================================================

Expand Down Expand Up @@ -323,40 +328,30 @@ mod tests {

/// Helper function for optimizing local operations testing. It can be used to
/// test loc_load, loc_store, loc_loadw and loc_storew operations.
fn test_parse_local(
base_op: &str,
is_single: bool,
operation: Operation,
parser: fn(&mut Vec<Operation>, &Token, u32, bool, bool) -> Result<(), AssemblyError>,
) {
fn test_parse_local(base_op: &str, is_single: bool, operation: Operation, parser: ParserFn) {
let num_proc_locals = 1;
let pos = 0;

let mut span_ops: Vec<Operation> = Vec::new();
let op_str = format!("{}.0", base_op);
let op = Token::new(&op_str, pos);
let expexted = vec![Operation::Pad, Operation::FmpAdd, operation];
let expected = vec![Operation::Pad, Operation::FmpAdd, operation];
let msg = format!("Failed to parse {}.0 (address provided by op)", base_op);

parser(&mut span_ops, &op, num_proc_locals, true, is_single).expect(&msg);

assert_eq!(&span_ops, &expexted);
assert_eq!(&span_ops, &expected);
}

/// Helper function for optimizing memory operations testing. It can be used to
/// test mem_load, mem_store, mem_loadw and mem_storew operations.
fn test_parse_mem(
base_op: &str,
is_single: bool,
operation: Operation,
parser: fn(&mut Vec<Operation>, &Token, u32, bool, bool) -> Result<(), AssemblyError>,
) {
fn test_parse_mem(base_op: &str, is_single: bool, operation: Operation, parser: ParserFn) {
let num_proc_locals = 0;
let pos = 0;

// test push with memory address on top of stack
let mut span_ops: Vec<Operation> = Vec::new();
let op = Token::new(&base_op, pos);
let op = Token::new(base_op, pos);
let expected = vec![operation];
let msg = format!("Failed to parse {}", base_op);

Expand All @@ -368,22 +363,22 @@ mod tests {
let mut span_ops: Vec<Operation> = Vec::new();
let op_str = format!("{}.0", base_op);
let op = Token::new(&op_str, pos);
let expexted = vec![Operation::Pad, operation];
let expected = vec![Operation::Pad, operation];
let msg = format!("Failed to parse {}.0", base_op);

parser(&mut span_ops, &op, num_proc_locals, false, is_single).expect(&msg);

assert_eq!(&span_ops, &expexted);
assert_eq!(&span_ops, &expected);

// test push with memory address provided directly (address 2)
let mut span_ops: Vec<Operation> = Vec::new();
let op_str = format!("{}.2", base_op);
let op = Token::new(&op_str, pos);
let expexted = vec![Operation::Push(Felt::new(2)), operation];
let expected = vec![Operation::Push(Felt::new(2)), operation];
let msg = format!("Failed to parse {}.2", base_op);

parser(&mut span_ops, &op, num_proc_locals, false, is_single).expect(&msg);

assert_eq!(&span_ops, &expexted);
assert_eq!(&span_ops, &expected);
}
}
11 changes: 10 additions & 1 deletion core/src/decoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,18 @@ pub const OP_BATCH_2_GROUPS: [Felt; NUM_OP_BATCH_FLAGS] = [ZERO, ZERO, ONE];
/// Operation batch consists of 1 operation group.
pub const OP_BATCH_1_GROUPS: [Felt; NUM_OP_BATCH_FLAGS] = [ZERO, ONE, ONE];

// Index of the op bits extra column in the decoder trace.
/// Index of the op bits extra column in the decoder trace.
pub const OP_BIT_EXTRA_COL_IDX: usize = OP_BATCH_FLAGS_RANGE.end;

/// Index of a flag column which indicates whether an ending block is a body of a loop.
pub const IS_LOOP_BODY_FLAG_COL_IDX: usize = HASHER_STATE_RANGE.start + 4;

/// Index of a flag column which indicates whether an ending block is a LOOP block.
pub const IS_LOOP_FLAG_COL_IDX: usize = HASHER_STATE_RANGE.start + 5;

/// Index of a flag column which indicates whether an ending block is a CALL block.
pub const IS_CALL_FLAG_COL_IDX: usize = HASHER_STATE_RANGE.start + 6;

// --- Column accessors in the auxiliary columns --------------------------------------------------

/// Running product column representing block stack table.
Expand Down
2 changes: 1 addition & 1 deletion miden/tests/integration/exec_iters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ fn test_exec_iter() {
]
.to_elements(),
fmp: next_fmp,
memory: mem.clone(),
memory: mem,
},
VmState {
clk: 15,
Expand Down
8 changes: 5 additions & 3 deletions miden/tests/integration/flow_control/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,15 @@ fn local_fn_call_with_mem_access() {
// calling foo, the value saved into memory[0] before calling foo should still be there.
let source = "
proc.foo
pop.mem.0
mem_store.0
drop
end
begin
pop.mem.0
mem_store.0
drop
call.foo
push.mem.0
mem_load.0
eq.7
end";

Expand Down

0 comments on commit dbc4368

Please sign in to comment.