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.

Sign up for GitHub

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

chore(avm)!: radix opcode - remove immediates #10696

Merged
merged 3 commits into from
Dec 17, 2024
Merged

chore(avm)!: radix opcode - remove immediates #10696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d936eb
10371: Noir/Brillig compilation changes
jeanmon Dec 12, 2024
c36cec2
10371: transpiler and siimulator changes as well as tests
jeanmon Dec 12, 2024
4ba22aa
10371: cpp changes
jeanmon Dec 13, 2024
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
19 changes: 10 additions & 9 deletions avm-transpiler/src/transpile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1064,29 +1064,30 @@ fn handle_black_box_function(avm_instrs: &mut Vec<AvmInstruction>, operation: &B
..Default::default()
});
}
BlackBoxOp::ToRadix { input, radix, output, output_bits } => {
let num_limbs = output.size as u32;
BlackBoxOp::ToRadix { input, radix, output_pointer, num_limbs, output_bits } => {
let input_offset = input.to_usize() as u32;
let output_offset = output.pointer.to_usize() as u32;
let radix_offset = radix.to_usize() as u32;
let output_offset = output_pointer.to_usize() as u32;
let num_limbs_offset = num_limbs.to_usize() as u32;
let output_bits_offset = output_bits.to_usize() as u32;

avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::TORADIXBE,
indirect: Some(
AddressingModeBuilder::default()
.direct_operand(input)
.indirect_operand(&output.pointer)
.direct_operand(radix)
.direct_operand(num_limbs)
.direct_operand(output_bits)
.indirect_operand(output_pointer)
.build(),
),
operands: vec![
AvmOperand::U16 { value: input_offset as u16 },
AvmOperand::U16 { value: output_offset as u16 },
AvmOperand::U16 { value: radix_offset as u16 },
],
immediates: vec![
AvmOperand::U16 { value: num_limbs as u16 },
AvmOperand::U8 { value: *output_bits as u8 },
AvmOperand::U16 { value: num_limbs_offset as u16 },
AvmOperand::U16 { value: output_bits_offset as u16 },
AvmOperand::U16 { value: output_offset as u16 },
],
..Default::default()
});
Expand Down
16 changes: 11 additions & 5 deletions barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,9 @@ struct BlackBoxOp {
struct ToRadix {
Program::MemoryAddress input;
Program::MemoryAddress radix;
Program::HeapArray output;
bool output_bits;
Program::MemoryAddress output_pointer;
Program::MemoryAddress num_limbs;
Program::MemoryAddress output_bits;

friend bool operator==(const ToRadix&, const ToRadix&);
std::vector<uint8_t> bincodeSerialize() const;
Expand Down Expand Up @@ -4611,7 +4612,10 @@ inline bool operator==(const BlackBoxOp::ToRadix& lhs, const BlackBoxOp::ToRadix
if (!(lhs.radix == rhs.radix)) {
return false;
}
if (!(lhs.output == rhs.output)) {
if (!(lhs.output_pointer == rhs.output_pointer)) {
return false;
}
if (!(lhs.num_limbs == rhs.num_limbs)) {
return false;
}
if (!(lhs.output_bits == rhs.output_bits)) {
Expand Down Expand Up @@ -4646,7 +4650,8 @@ void serde::Serializable<Program::BlackBoxOp::ToRadix>::serialize(const Program:
{
serde::Serializable<decltype(obj.input)>::serialize(obj.input, serializer);
serde::Serializable<decltype(obj.radix)>::serialize(obj.radix, serializer);
serde::Serializable<decltype(obj.output)>::serialize(obj.output, serializer);
serde::Serializable<decltype(obj.output_pointer)>::serialize(obj.output_pointer, serializer);
serde::Serializable<decltype(obj.num_limbs)>::serialize(obj.num_limbs, serializer);
serde::Serializable<decltype(obj.output_bits)>::serialize(obj.output_bits, serializer);
}

Expand All @@ -4658,7 +4663,8 @@ Program::BlackBoxOp::ToRadix serde::Deserializable<Program::BlackBoxOp::ToRadix>
Program::BlackBoxOp::ToRadix obj;
obj.input = serde::Deserializable<decltype(obj.input)>::deserialize(deserializer);
obj.radix = serde::Deserializable<decltype(obj.radix)>::deserialize(deserializer);
obj.output = serde::Deserializable<decltype(obj.output)>::deserialize(deserializer);
obj.output_pointer = serde::Deserializable<decltype(obj.output_pointer)>::deserialize(deserializer);
obj.num_limbs = serde::Deserializable<decltype(obj.num_limbs)>::deserialize(deserializer);
obj.output_bits = serde::Deserializable<decltype(obj.output_bits)>::deserialize(deserializer);
return obj;
}
Expand Down
115 changes: 34 additions & 81 deletions barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ class AvmExecutionTests : public ::testing::Test {
}
};

class AvmExecutionTestsToRadix : public AvmExecutionTests, public testing::WithParamInterface<bool> {};

// Basic positive test with an ADD and RETURN opcode.
// Parsing, trace generation and proving is verified.
TEST_F(AvmExecutionTests, basicAddReturn)
Expand Down Expand Up @@ -850,85 +852,15 @@ TEST_F(AvmExecutionTests, setAndCastOpcodes)
validate_trace(std::move(trace), public_inputs);
}

// Positive test with TO_RADIX_BE.
TEST_F(AvmExecutionTests, toRadixBeOpcodeBytes)
{
std::string bytecode_hex =
to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
"00" // dst_offset
+ to_hex(AvmMemoryTag::U32) + "00" // val
+ to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
"01" // dst_offset
+ to_hex(AvmMemoryTag::U32) + "01" // val
+ to_hex(OpCode::CALLDATACOPY) + // opcode CALLDATACOPY
"00" // Indirect flag
"0000" // cd_offset
"0001" // copy_size
"0001" // dst_offset
+ to_hex(OpCode::SET_8) + // opcode SET for indirect src
"00" // Indirect flag
"11" // dst_offset 17
+ to_hex(AvmMemoryTag::U32) + "01" // value 1 (i.e. where the src from calldata is copied)
+ to_hex(OpCode::SET_8) + // opcode SET for indirect dst
"00" // Indirect flag
"15" // dst_offset 21
+ to_hex(AvmMemoryTag::U32) + "05" // value 5 (i.e. where the dst will be written to)
+ to_hex(OpCode::SET_8) + // opcode SET for indirect dst
"00" // Indirect flag
"80" // radix_offset 80
+ to_hex(AvmMemoryTag::U32) + "02" // value 2 (i.e. radix 2 - perform bitwise decomposition)
+ to_hex(OpCode::TORADIXBE) + // opcode TO_RADIX_BE
"03" // Indirect flag
"0011" // src_offset 17 (indirect)
"0015" // dst_offset 21 (indirect)
"0080" // radix_offset 80 (direct)
"0100" // limbs: 256
"00" // output_bits: false
+ to_hex(OpCode::SET_16) + // opcode SET (for return size)
"00" // Indirect flag
"0200" // dst_offset=512
+ to_hex(AvmMemoryTag::U32) + //
"0100" // val: 256
+ to_hex(OpCode::RETURN) + // opcode RETURN
"00" // Indirect flag
"0005" // ret offset 5
"0200"; // ret size offset 512

auto bytecode = hex_to_bytes(bytecode_hex);
auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode);
ASSERT_TRUE(is_ok(error));

// Assign a vector that we will mutate internally in gen_trace to store the return values;
std::vector<FF> returndata;
ExecutionHints execution_hints;
auto trace =
gen_trace(bytecode, std::vector<FF>{ FF::modulus - FF(1) }, public_inputs, returndata, execution_hints);

// Find the first row enabling the TORADIXBE selector
// Expected output is bitwise decomposition of MODULUS - 1..could hardcode the result but it's a bit long
size_t num_limbs = 256;
std::vector<FF> expected_output(num_limbs);
// Extract each bit.
for (size_t i = 0; i < num_limbs; i++) {
auto byte_index = num_limbs - i - 1;
FF expected_limb = (FF::modulus - 1) >> i & 1;
expected_output[byte_index] = expected_limb;
}
EXPECT_EQ(returndata, expected_output);

validate_trace(std::move(trace), public_inputs, { FF::modulus - FF(1) }, returndata);
}

// Positive test with TO_RADIX_BE.
TEST_F(AvmExecutionTests, toRadixBeOpcodeBitsMode)
namespace {
std::vector<uint8_t> gen_bytecode_radix(bool is_bit_mode)
{
const std::string bit_mode_hex = is_bit_mode ? "01" : "00";
std::string bytecode_hex = to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
"00" // dst_offset
+ to_hex(AvmMemoryTag::U32) //
+ "00" // val
+ to_hex(AvmMemoryTag::U32) + //
"00" // val
+ to_hex(OpCode::SET_8) + // opcode SET
"00" // Indirect flag
"01" // dst_offset
Expand All @@ -949,18 +881,28 @@ TEST_F(AvmExecutionTests, toRadixBeOpcodeBitsMode)
"15" // dst_offset 21
+ to_hex(AvmMemoryTag::U32) + //
"05" // value 5 (i.e. where the dst will be written to)
+ to_hex(OpCode::SET_8) + // opcode SET for indirect dst
+ to_hex(OpCode::SET_8) + // opcode SET (direct radix_offset)
"00" // Indirect flag
"80" // radix_offset 80
+ to_hex(AvmMemoryTag::U32) + //
"02" // value 2 (i.e. radix 2 - perform bitwise decomposition)
+ to_hex(OpCode::SET_16) + // opcode SET (direct number_limbs_offset)
"00" // Indirect flag
"0090" // number_limbs_offset 0x90
+ to_hex(AvmMemoryTag::U32) + //
"0100" // value 256
+ to_hex(OpCode::SET_8) + // opcode SET (direct output_bits_offset)
"00" // Indirect flag
"95" // output_bits_offset 0x95
+ to_hex(AvmMemoryTag::U1) + //
bit_mode_hex // bit 1 (true for bits mode)
+ to_hex(OpCode::TORADIXBE) + // opcode TO_RADIX_BE
"03" // Indirect flag
"0011" // Indirect flag
"0011" // src_offset 17 (indirect)
"0015" // dst_offset 21 (indirect)
"0080" // radix_offset 80 (direct)
"0100" // limbs: 256
"01" // output_bits: true
"0090" // num_limbs_offset (direct)
"0095" // output_bits_offset (direct)
"0015" // dst_offset 21 (indirect)
+ to_hex(OpCode::SET_16) + // opcode SET (for return size)
"00" // Indirect flag
"0200" // dst_offset=512
Expand All @@ -971,7 +913,15 @@ TEST_F(AvmExecutionTests, toRadixBeOpcodeBitsMode)
"0005" // ret offset 5
"0200"; // ret size offset 512

auto bytecode = hex_to_bytes(bytecode_hex);
return hex_to_bytes(bytecode_hex);
}
} // namespace

// Positive test for TORADIXBE opcode parametrized by a boolean toggling bit vs bytes mode.
TEST_P(AvmExecutionTestsToRadix, ParamTest)
{
const bool is_bit_mode = GetParam();
auto bytecode = gen_bytecode_radix(is_bit_mode);
auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode);
ASSERT_TRUE(is_ok(error));

Expand All @@ -996,6 +946,9 @@ TEST_F(AvmExecutionTests, toRadixBeOpcodeBitsMode)
validate_trace(std::move(trace), public_inputs, { FF::modulus - FF(1) }, returndata);
}

// Run the test for TORADIXBE in bit mode and then in bytes mode.
INSTANTIATE_TEST_SUITE_P(AvmExecutionTests, AvmExecutionTestsToRadix, testing::ValuesIn({ true, false }));

// // Positive test with SHA256COMPRESSION.
TEST_F(AvmExecutionTests, sha256CompressionOpcode)
{
Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,12 @@ const std::unordered_map<OpCode, std::vector<OperandType>> OPCODE_WIRE_FORMAT =
{ OperandType::INDIRECT8, OperandType::UINT16, OperandType::UINT16, OperandType::UINT16, OperandType::UINT16 } },
// Gadget - Conversion
{ OpCode::TORADIXBE,
{ OperandType::INDIRECT8,
{ OperandType::INDIRECT16,
OperandType::UINT16,
OperandType::UINT16,
OperandType::UINT16,
OperandType::UINT16,
OperandType::UINT8 } },
OperandType::UINT16 } },
};

const std::unordered_map<OperandType, uint32_t> OPERAND_TYPE_SIZE = {
Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,12 +961,12 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder,

// Conversions
case OpCode::TORADIXBE:
error = trace_builder.op_to_radix_be(std::get<uint8_t>(inst.operands.at(0)),
error = trace_builder.op_to_radix_be(std::get<uint16_t>(inst.operands.at(0)),
std::get<uint16_t>(inst.operands.at(1)),
std::get<uint16_t>(inst.operands.at(2)),
std::get<uint16_t>(inst.operands.at(3)),
std::get<uint16_t>(inst.operands.at(4)),
std::get<uint8_t>(inst.operands.at(5)));
std::get<uint16_t>(inst.operands.at(5)));
break;

default:
Expand Down
47 changes: 29 additions & 18 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4381,46 +4381,57 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect,
*
* @param indirect A byte encoding information about indirect/direct memory access.
* @param src_offset An index in memory pointing to the input of the To_Radix_BE conversion.
* @param dst_offset An index in memory pointing to the output of the To_Radix_BE conversion.
* @param radix_offset An index in memory pointing to the strict upper bound of each converted limb, i.e., 0 <= limb
* < radix.
* @param num_limbs The number of limbs to the value into.
* @param output_bits Should the output be U1s instead of U8s?
* @param num_limbs_offset Offset pointing to the number of limbs to the value into.
* @param output_bits_offset Offset pointing to a boolean telling whether bits (U1) or bytes (U8) are in the output
* @param dst_offset An index in memory pointing to the output of the To_Radix_BE conversion.
*/
AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect,
AvmError AvmTraceBuilder::op_to_radix_be(uint16_t indirect,
uint32_t src_offset,
uint32_t dst_offset,
uint32_t radix_offset,
uint32_t num_limbs,
uint8_t output_bits)
uint32_t num_limbs_offset,
uint32_t output_bits_offset,
uint32_t dst_offset)
{
// We keep the first encountered error
AvmError error = AvmError::NO_ERROR;
auto clk = static_cast<uint32_t>(main_trace.size()) + 1;

// write output as bits or bytes
AvmMemoryTag w_in_tag = output_bits > 0 ? AvmMemoryTag::U1 // bits mode
: AvmMemoryTag::U8;

auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr)
.resolve({ src_offset, dst_offset, radix_offset }, mem_trace_builder);
auto [resolved_src_offset, resolved_dst_offset, resolved_radix_offset] = resolved_addrs;
auto [resolved_addrs, res_error] =
Addressing<5>::fromWire(indirect, call_ptr)
.resolve({ src_offset, radix_offset, num_limbs_offset, output_bits_offset, dst_offset }, mem_trace_builder);
auto [resolved_src_offset,
resolved_radix_offset,
resolved_num_limbs_offset,
resolved_output_bits_offset,
resolved_dst_offset] = resolved_addrs;
error = res_error;

if (is_ok(error) && !check_tag(AvmMemoryTag::U32, resolved_radix_offset) &&
!check_tag(AvmMemoryTag::U32, resolved_num_limbs_offset) &&
!check_tag(AvmMemoryTag::U1, resolved_output_bits_offset)) {
error = AvmError::CHECK_TAG_ERROR;
}

const auto num_limbs = static_cast<uint32_t>(unconstrained_read_from_memory(resolved_num_limbs_offset));

// Constrain gas cost
gas_trace_builder.constrain_gas(clk, OpCode::TORADIXBE, num_limbs);

const auto output_bits = static_cast<uint8_t>(unconstrained_read_from_memory(resolved_output_bits_offset));

// write output as bits or bytes
AvmMemoryTag w_in_tag = output_bits > 0 ? AvmMemoryTag::U1 // bits mode
: AvmMemoryTag::U8;

auto read_src = constrained_read_from_memory(
call_ptr, clk, resolved_src_offset, AvmMemoryTag::FF, w_in_tag, IntermRegister::IA);
// TODO(8603): once instructions can have multiple different tags for reads, constrain the radix's read
// TODO(9497): if simulator fails tag check here, witgen will not. Raise error flag!
// auto read_radix = constrained_read_from_memory(
// call_ptr, clk, resolved_radix_offset, AvmMemoryTag::U32, AvmMemoryTag::U32, IntermRegister::IB);

if (is_ok(error) && !check_tag(AvmMemoryTag::U32, resolved_radix_offset)) {
error = AvmError::CHECK_TAG_ERROR;
}

auto read_radix = unconstrained_read_from_memory(resolved_radix_offset);

FF input = read_src.val;
Expand Down
8 changes: 4 additions & 4 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,12 @@ class AvmTraceBuilder {
uint32_t output_offset,
uint32_t point_length_offset);
// Conversions
AvmError op_to_radix_be(uint8_t indirect,
AvmError op_to_radix_be(uint16_t indirect,
uint32_t src_offset,
uint32_t dst_offset,
uint32_t radix_offset,
uint32_t num_limbs,
uint8_t output_bits);
uint32_t num_limbs_offset,
uint32_t output_bits_offset,
uint32_t dst_offset);

std::vector<Row> finalize(bool apply_end_gas_assertions = false);
void reset();
Expand Down
Loading
Loading