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

feat(avm): Indirect memory for set opcode #5546

Merged
merged 4 commits into from
Apr 4, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,8 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
break;
// Machine State - Memory
case OpCode::SET: {
uint32_t dst_offset = 0;
uint128_t val = 0;
// Skip the indirect flag at index 0;
AvmMemoryTag in_tag = std::get<AvmMemoryTag>(inst.operands.at(1));
dst_offset = std::get<uint32_t>(inst.operands.at(3));

switch (in_tag) {
case AvmMemoryTag::U8:
Expand All @@ -212,7 +209,8 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
break;
}

trace_builder.set(val, dst_offset, in_tag);
trace_builder.op_set(
std::get<uint8_t>(inst.operands.at(0)), val, std::get<uint32_t>(inst.operands.at(3)), in_tag);
break;
}
case OpCode::MOV:
Expand Down
52 changes: 32 additions & 20 deletions barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -629,35 +629,49 @@ void AvmTraceBuilder::op_xor(
});
}

// TODO: Finish SET opcode implementation. This is a partial implementation
// facilitating testing of arithmetic operations over non finite field types.
// We add an entry in the memory trace and a simplified one in the main trace
// without operation selector.
// TODO: PIL relations for the SET opcode need to be implemented.
// No check is performed that val pertains to type defined by in_tag.
// TODO: Ensure that the bytecode validation and/or deserialization is
// enforcing that val complies to the tag.
/**
* @brief Set a constant from bytecode with direct memory access.
* @brief Set a constant from bytecode with direct or indirect memory access.
* SET opcode is implemented purely as a memory operation. As val is a
* constant passed in the bytecode, the deserialization layer or bytecode
* validation circuit is enforcing that the constant complies to in_tag.
* Therefore, no range check is required as part of this opcode relation.
*
* @param indirect A byte encoding information about indirect/direct memory access.
* @param val The constant to be written upcasted to u128
* @param dst_offset Memory destination offset where val is written to
* @param in_tag The instruction memory tag
*/
void AvmTraceBuilder::set(uint128_t val, uint32_t dst_offset, AvmMemoryTag in_tag)
void AvmTraceBuilder::op_set(uint8_t indirect, uint128_t val, uint32_t dst_offset, AvmMemoryTag in_tag)
{
auto clk = static_cast<uint32_t>(main_trace.size());
auto val_ff = FF{ uint256_t::from_uint128(val) };
auto const clk = static_cast<uint32_t>(main_trace.size());
auto const val_ff = FF{ uint256_t::from_uint128(val) };
uint32_t direct_dst_offset = dst_offset; // Overriden in indirect mode
bool indirect_dst_flag = is_operand_indirect(indirect, 0);
bool tag_match = true;

if (indirect_dst_flag) {
auto read_ind_c =
mem_trace_builder.indirect_read_and_load_from_memory(clk, IndirectRegister::IND_C, dst_offset);
tag_match = read_ind_c.tag_match;
direct_dst_offset = uint32_t(read_ind_c.val);
}

mem_trace_builder.write_into_memory(clk, IntermRegister::IC, dst_offset, val_ff, AvmMemoryTag::U0, in_tag);
mem_trace_builder.write_into_memory(clk, IntermRegister::IC, direct_dst_offset, val_ff, AvmMemoryTag::U0, in_tag);

main_trace.push_back(Row{
.avm_main_clk = clk,
.avm_main_ic = val_ff,
.avm_main_internal_return_ptr = FF(internal_return_ptr),
.avm_main_mem_idx_c = FF(dst_offset),
.avm_main_mem_op_c = FF(1),
.avm_main_pc = FF(pc++),
.avm_main_rwc = FF(1),
.avm_main_w_in_tag = FF(static_cast<uint32_t>(in_tag)),
.avm_main_ind_c = indirect_dst_flag ? dst_offset : 0,
.avm_main_ind_op_c = static_cast<uint32_t>(indirect_dst_flag),
.avm_main_internal_return_ptr = internal_return_ptr,
.avm_main_mem_idx_c = direct_dst_offset,
.avm_main_mem_op_c = 1,
.avm_main_pc = pc++,
.avm_main_rwc = 1,
.avm_main_tag_err = static_cast<uint32_t>(!tag_match),
.avm_main_w_in_tag = static_cast<uint32_t>(in_tag),
});
}

Expand Down Expand Up @@ -722,16 +736,14 @@ void AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t dst
}

/**
* @brief CALLDATACOPY opcode with direct memory access, i.e.,
* @brief CALLDATACOPY opcode with direct or indirect memory access, i.e.,
* direct: M[dst_offset:dst_offset+copy_size] = calldata[cd_offset:cd_offset+copy_size]
* indirect: M[M[dst_offset]:M[dst_offset]+copy_size] = calldata[cd_offset:cd_offset+copy_size]
* Simplified version with exclusively memory store operations and
* values from calldata passed by an array and loaded into
* intermediate registers.
* Assume that caller passes call_data_mem which is large enough so that
* no out-of-bound memory issues occur.
* TODO: taking care of intermediate register values consistency and propagating their
* values to the next row when not overwritten.
* TODO: error handling if dst_offset + copy_size > 2^32 which would lead to
* out-of-bound memory write. Similarly, if cd_offset + copy_size is larger
* than call_data_mem.size()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ class AvmTraceBuilder {
// Bitwise xor with direct or indirect memory access.
void op_xor(uint8_t indirect, uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, AvmMemoryTag in_tag);

// Set a constant from bytecode with direct memory access.
void set(uint128_t val, uint32_t dst_offset, AvmMemoryTag in_tag);
// Set a constant from bytecode with direct or indirect memory access.
void op_set(uint8_t indirect, uint128_t val, uint32_t dst_offset, AvmMemoryTag in_tag);

// Move (copy) the value and tag of a memory cell to another one.
void op_mov(uint8_t indirect, uint32_t src_offset, uint32_t dst_offset);
Expand Down
Loading
Loading