Skip to content

Commit

Permalink
feat(avm/brillig)!: take addresses in calldatacopy (#8388)
Browse files Browse the repository at this point in the history 
Makes calldatacopy more flexible. It's also needed to use this opcode in
user-space.

As I mentioned we'll probably want most or all opcode immediates to be
addresses for maximum flexibility. This is a first experimentation on
the pains of doing it :)

---------

Co-authored-by: sirasistant <sirasistant@gmail.com>
  • Loading branch information
@fcarreiro @sirasistant
fcarreiro and sirasistant authored Sep 9, 2024
1 parent 075036e commit eab944c
Show file tree
Hide file tree
Showing 25 changed files with 926 additions and 507 deletions.
6 changes: 3 additions & 3 deletions avm-transpiler/src/transpile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ pub fn brillig_to_avm(
],
});
}
BrilligOpcode::CalldataCopy { destination_address, size, offset } => {
BrilligOpcode::CalldataCopy { destination_address, size_address, offset_address } => {
avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::CALLDATACOPY,
indirect: Some(ALL_DIRECT),
operands: vec![
AvmOperand::U32 {
value: *offset as u32, // cdOffset (calldata offset)
value: offset_address.to_usize() as u32, // cdOffset (calldata offset)
},
AvmOperand::U32 { value: *size as u32 },
AvmOperand::U32 { value: size_address.to_usize() as u32 }, // sizeOffset
AvmOperand::U32 {
value: destination_address.to_usize() as u32, // dstOffset
},
Expand Down
16 changes: 8 additions & 8 deletions barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,8 +632,8 @@ struct BrilligOpcode {

struct CalldataCopy {
Program::MemoryAddress destination_address;
uint64_t size;
uint64_t offset;
Program::MemoryAddress size_address;
Program::MemoryAddress offset_address;

friend bool operator==(const CalldataCopy&, const CalldataCopy&);
std::vector<uint8_t> bincodeSerialize() const;
Expand Down Expand Up @@ -6320,10 +6320,10 @@ inline bool operator==(const BrilligOpcode::CalldataCopy& lhs, const BrilligOpco
if (!(lhs.destination_address == rhs.destination_address)) {
return false;
}
if (!(lhs.size == rhs.size)) {
if (!(lhs.size_address == rhs.size_address)) {
return false;
}
if (!(lhs.offset == rhs.offset)) {
if (!(lhs.offset_address == rhs.offset_address)) {
return false;
}
return true;
Expand Down Expand Up @@ -6354,8 +6354,8 @@ void serde::Serializable<Program::BrilligOpcode::CalldataCopy>::serialize(
const Program::BrilligOpcode::CalldataCopy& obj, Serializer& serializer)
{
serde::Serializable<decltype(obj.destination_address)>::serialize(obj.destination_address, serializer);
serde::Serializable<decltype(obj.size)>::serialize(obj.size, serializer);
serde::Serializable<decltype(obj.offset)>::serialize(obj.offset, serializer);
serde::Serializable<decltype(obj.size_address)>::serialize(obj.size_address, serializer);
serde::Serializable<decltype(obj.offset_address)>::serialize(obj.offset_address, serializer);
}

template <>
Expand All @@ -6365,8 +6365,8 @@ Program::BrilligOpcode::CalldataCopy serde::Deserializable<Program::BrilligOpcod
{
Program::BrilligOpcode::CalldataCopy obj;
obj.destination_address = serde::Deserializable<decltype(obj.destination_address)>::deserialize(deserializer);
obj.size = serde::Deserializable<decltype(obj.size)>::deserialize(deserializer);
obj.offset = serde::Deserializable<decltype(obj.offset)>::deserialize(deserializer);
obj.size_address = serde::Deserializable<decltype(obj.size_address)>::deserialize(deserializer);
obj.offset_address = serde::Deserializable<decltype(obj.offset_address)>::deserialize(deserializer);
return obj;
}

Expand Down
41 changes: 31 additions & 10 deletions barretenberg/cpp/src/barretenberg/vm/avm/tests/arithmetic.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,9 @@ TEST_F(AvmArithmeticTestsFF, addition)
{
std::vector<FF> const calldata = { 37, 4, 11 };
gen_trace_builder(calldata);
trace_builder.op_calldata_copy(0, 0, 3, 0);
trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
trace_builder.op_set(0, 3, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);

// Memory layout: [37,4,11,0,0,0,....]
trace_builder.op_add(0, 0, 1, 4, AvmMemoryTag::FF); // [37,4,11,0,41,0,....]
Expand All @@ -415,7 +417,9 @@ TEST_F(AvmArithmeticTestsFF, subtraction)
{
std::vector<FF> const calldata = { 8, 4, 17 };
gen_trace_builder(calldata);
trace_builder.op_calldata_copy(0, 0, 3, 0);
trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
trace_builder.op_set(0, 3, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);

// Memory layout: [8,4,17,0,0,0,....]
trace_builder.op_sub(0, 2, 0, 1, AvmMemoryTag::FF); // [8,9,17,0,0,0....]
Expand All @@ -436,7 +440,9 @@ TEST_F(AvmArithmeticTestsFF, multiplication)
{
std::vector<FF> const calldata = { 5, 0, 20 };
gen_trace_builder(calldata);
trace_builder.op_calldata_copy(0, 0, 3, 0);
trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
trace_builder.op_set(0, 3, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);

// Memory layout: [5,0,20,0,0,0,....]
trace_builder.op_mul(0, 2, 0, 1, AvmMemoryTag::FF); // [5,100,20,0,0,0....]
Expand All @@ -456,8 +462,10 @@ TEST_F(AvmArithmeticTestsFF, multiplication)
// Test on multiplication by zero over finite field type.
TEST_F(AvmArithmeticTestsFF, multiplicationByZero)
{
std::vector<FF> const calldata = { 127 };
std::vector<FF> const calldata = { 127, 0, 0 };
gen_trace_builder(calldata);
trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
trace_builder.op_set(0, 3, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);

// Memory layout: [127,0,0,0,0,0,....]
Expand All @@ -480,7 +488,9 @@ TEST_F(AvmArithmeticTestsFF, fDivision)
{
std::vector<FF> const calldata = { 15, 315 };
gen_trace_builder(calldata);
trace_builder.op_calldata_copy(0, 0, 2, 0);
trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
trace_builder.op_set(0, 2, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);

// Memory layout: [15,315,0,0,0,0,....]
trace_builder.op_fdiv(0, 1, 0, 2); // [15,315,21,0,0,0....]
Expand All @@ -504,8 +514,10 @@ TEST_F(AvmArithmeticTestsFF, fDivision)
// Test on division with zero numerator over finite field type.
TEST_F(AvmArithmeticTestsFF, fDivisionNumeratorZero)
{
std::vector<FF> const calldata = { 15 };
std::vector<FF> const calldata = { 15, 0, 0 };
gen_trace_builder(calldata);
trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
trace_builder.op_set(0, 3, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);

// Memory layout: [15,0,0,0,0,0,....]
Expand All @@ -531,8 +543,10 @@ TEST_F(AvmArithmeticTestsFF, fDivisionNumeratorZero)
// We check that the operator error flag is raised.
TEST_F(AvmArithmeticTestsFF, fDivisionByZeroError)
{
std::vector<FF> const calldata = { 15 };
std::vector<FF> const calldata = { 15, 0, 0 };
gen_trace_builder(calldata);
trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
trace_builder.op_set(0, 3, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);

// Memory layout: [15,0,0,0,0,0,....]
Expand Down Expand Up @@ -585,7 +599,9 @@ TEST_F(AvmArithmeticTestsFF, mixedOperationsWithError)
{
std::vector<FF> const calldata = { 45, 23, 12 };
gen_trace_builder(calldata);
trace_builder.op_calldata_copy(0, 0, 3, 2);
trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
trace_builder.op_set(0, 3, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);

// Memory layout: [0,0,45,23,12,0,0,0,....]
trace_builder.op_add(0, 2, 3, 4, AvmMemoryTag::FF); // [0,0,45,23,68,0,0,0,....]
Expand All @@ -610,7 +626,9 @@ TEST_F(AvmArithmeticTestsFF, equality)
FF elem = FF::modulus - FF(1);
std::vector<FF> const calldata = { elem, elem };
gen_trace_builder(calldata);
trace_builder.op_calldata_copy(0, 0, 2, 0);
trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
trace_builder.op_set(0, 2, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);
trace_builder.op_eq(0, 0, 1, 2, AvmMemoryTag::FF); // Memory Layout [q - 1, q - 1, 1, 0..]
trace_builder.op_return(0, 0, 3);
auto trace = trace_builder.finalize();
Expand All @@ -631,7 +649,10 @@ TEST_F(AvmArithmeticTestsFF, nonEquality)
FF elem = FF::modulus - FF(1);
std::vector<FF> const calldata = { elem, elem + FF(1) };
gen_trace_builder(calldata);
trace_builder.op_calldata_copy(0, 0, 2, 0);
trace_builder.op_set(0, 0, 0, AvmMemoryTag::U32);
trace_builder.op_set(0, 2, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);

trace_builder.op_eq(0, 0, 1, 2, AvmMemoryTag::FF); // Memory Layout [q - 1, q, 0, 0..]
trace_builder.op_return(0, 0, 3);
auto trace = trace_builder.finalize();
Expand Down
2 changes: 2 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm/tests/cast.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ TEST_F(AvmCastTests, truncationFFToU16ModMinus1)
{
calldata = { FF::modulus - 1 };
trace_builder = AvmTraceBuilder(public_inputs, {}, 0, calldata);
trace_builder.op_set(0, 1, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);
trace_builder.op_cast(0, 0, 1, AvmMemoryTag::U16);
trace_builder.op_return(0, 0, 0);
Expand All @@ -184,6 +185,7 @@ TEST_F(AvmCastTests, truncationFFToU16ModMinus2)
{
calldata = { FF::modulus_minus_two };
trace_builder = AvmTraceBuilder(public_inputs, {}, 0, calldata);
trace_builder.op_set(0, 1, 1, AvmMemoryTag::U32);
trace_builder.op_calldata_copy(0, 0, 1, 0);
trace_builder.op_cast(0, 0, 1, AvmMemoryTag::U16);
trace_builder.op_return(0, 0, 0);
Expand Down
Loading

0 comments on commit eab944c

Please sign in to comment.