Skip to content

Commit

Permalink
fix: entrypoint register allocation
Browse files Browse the repository at this point in the history
  • Loading branch information
@sirasistant @fcarreiro
sirasistant authored and fcarreiro committed Sep 6, 2024
1 parent 9de462a commit 6add047
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 69 deletions.
106 changes: 55 additions & 51 deletions noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
arguments: &[BrilligParameter],
return_parameters: &[BrilligParameter],
) {
// We need to allocate the variable for every argument first so any register allocation doesn't mangle the expected order.
let mut argument_variables = self.allocate_function_arguments(arguments);

let calldata_size = Self::flattened_tuple_size(arguments);
let return_data_size = Self::flattened_tuple_size(return_parameters);

Expand All @@ -64,90 +67,91 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
// Copy calldata
self.copy_and_cast_calldata(arguments);

// Allocate the variables for every argument:
let mut current_calldata_pointer = Self::calldata_start_offset();

let mut argument_variables: Vec<_> = arguments
.iter()
.map(|argument| match argument {
BrilligParameter::SingleAddr(bit_size) => {
let single_address = self.allocate_register();
let var = BrilligVariable::SingleAddr(SingleAddrVariable {
address: single_address,
bit_size: *bit_size,
});
self.mov_instruction(single_address, MemoryAddress(current_calldata_pointer));
current_calldata_pointer += 1;
var
}
BrilligParameter::Array(_, _) => {
let pointer_to_the_array_in_calldata =
self.make_usize_constant_instruction(current_calldata_pointer.into());
let rc_register = self.make_usize_constant_instruction(1_usize.into());
let flattened_size = Self::flattened_size(argument);
let var = BrilligVariable::BrilligArray(BrilligArray {
pointer: pointer_to_the_array_in_calldata.address,
size: flattened_size,
rc: rc_register.address,
});

current_calldata_pointer += flattened_size;
var
}
BrilligParameter::Slice(_, _) => {
let pointer_to_the_array_in_calldata =
self.make_usize_constant_instruction(current_calldata_pointer.into());

let flattened_size = Self::flattened_size(argument);
let size_register = self.make_usize_constant_instruction(flattened_size.into());
let rc_register = self.make_usize_constant_instruction(1_usize.into());

let var = BrilligVariable::BrilligVector(BrilligVector {
pointer: pointer_to_the_array_in_calldata.address,
size: size_register.address,
rc: rc_register.address,
});

current_calldata_pointer += flattened_size;
var
}
})
.collect();

// Deflatten arrays
// Initialize the variables with the calldata
for (argument_variable, argument) in argument_variables.iter_mut().zip(arguments) {
match (argument_variable, argument) {
(BrilligVariable::SingleAddr(single_address), BrilligParameter::SingleAddr(_)) => {
self.mov_instruction(
single_address.address,
MemoryAddress(current_calldata_pointer),
);
current_calldata_pointer += 1;
}
(
BrilligVariable::BrilligArray(array),
BrilligParameter::Array(item_type, item_count),
) => {
let flattened_size = array.size;
self.usize_const_instruction(array.pointer, current_calldata_pointer.into());
self.usize_const_instruction(array.rc, 1_usize.into());

// Deflatten the array
let deflattened_address =
self.deflatten_array(item_type, array.size, array.pointer);
self.mov_instruction(array.pointer, deflattened_address);
array.size = item_type.len() * item_count;
self.deallocate_register(deflattened_address);

current_calldata_pointer += flattened_size;
}
(
BrilligVariable::BrilligVector(vector),
BrilligParameter::Slice(item_type, item_count),
) => {
let flattened_size = Self::flattened_size(argument);
self.usize_const_instruction(vector.pointer, current_calldata_pointer.into());
self.usize_const_instruction(vector.rc, 1_usize.into());
self.usize_const_instruction(vector.size, flattened_size.into());

// Deflatten the vector
let deflattened_address =
self.deflatten_array(item_type, flattened_size, vector.pointer);
self.mov_instruction(vector.pointer, deflattened_address);
self.usize_const_instruction(
vector.size,
(item_type.len() * item_count).into(),
);

self.deallocate_register(deflattened_address);

current_calldata_pointer += flattened_size;
}
_ => {}
_ => unreachable!("ICE: cannot match variables against arguments"),
}
}
}

fn allocate_function_arguments(
&mut self,
arguments: &[BrilligParameter],
) -> Vec<BrilligVariable> {
arguments
.iter()
.map(|argument| match argument {
BrilligParameter::SingleAddr(bit_size) => {
BrilligVariable::SingleAddr(SingleAddrVariable {
address: self.allocate_register(),
bit_size: *bit_size,
})
}
BrilligParameter::Array(_, _) => {
let flattened_size = Self::flattened_size(argument);
BrilligVariable::BrilligArray(BrilligArray {
pointer: self.allocate_register(),
size: flattened_size,
rc: self.allocate_register(),
})
}
BrilligParameter::Slice(_, _) => BrilligVariable::BrilligVector(BrilligVector {
pointer: self.allocate_register(),
size: self.allocate_register(),
rc: self.allocate_register(),
}),
})
.collect()
}

fn copy_and_cast_calldata(&mut self, arguments: &[BrilligParameter]) {
let calldata_size = Self::flattened_tuple_size(arguments);
self.calldata_copy_instruction(
Expand Down
36 changes: 30 additions & 6 deletions noir/noir-repo/tooling/debugger/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ mod tests {
})
);

// Execute the first Brillig opcode (calldata copy)
// Const
let result = context.step_into_opcode();
assert!(matches!(result, DebugCommandResult::Ok));
assert_eq!(
Expand All @@ -1058,7 +1058,7 @@ mod tests {
})
);

// execute the second Brillig opcode (const)
// Const
let result = context.step_into_opcode();
assert!(matches!(result, DebugCommandResult::Ok));
assert_eq!(
Expand All @@ -1070,26 +1070,50 @@ mod tests {
})
);

// try to execute the third Brillig opcode (and resolve the foreign call)
// Calldatacopy
let result = context.step_into_opcode();
assert!(matches!(result, DebugCommandResult::Ok));
assert_eq!(
context.get_current_debug_location(),
Some(DebugLocation {
circuit_id: 0,
opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 2 },
opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 3 },
brillig_function_id: Some(BrilligFunctionId(0)),
})
);

// retry the third Brillig opcode (foreign call should be finished)
// Const
let result = context.step_into_opcode();
assert!(matches!(result, DebugCommandResult::Ok));
assert_eq!(
context.get_current_debug_location(),
Some(DebugLocation {
circuit_id: 0,
opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 3 },
opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 4 },
brillig_function_id: Some(BrilligFunctionId(0)),
})
);

// try to execute the Brillig opcode (and resolve the foreign call)
let result = context.step_into_opcode();
assert!(matches!(result, DebugCommandResult::Ok));
assert_eq!(
context.get_current_debug_location(),
Some(DebugLocation {
circuit_id: 0,
opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 4 },
brillig_function_id: Some(BrilligFunctionId(0)),
})
);

// retry the Brillig opcode (foreign call should be finished)
let result = context.step_into_opcode();
assert!(matches!(result, DebugCommandResult::Ok));
assert_eq!(
context.get_current_debug_location(),
Some(DebugLocation {
circuit_id: 0,
opcode_location: OpcodeLocation::Brillig { acir_index: 0, brillig_index: 5 },
brillig_function_id: Some(BrilligFunctionId(0)),
})
);
Expand Down
13 changes: 5 additions & 8 deletions yarn-project/simulator/src/avm/opcodes/external_calls.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { mock } from 'jest-mock-extended';

import { type PublicSideEffectTraceInterface } from '../../public/side_effect_trace_interface.js';
import { type AvmContext } from '../avm_context.js';
import { Field, Uint8, Uint32 } from '../avm_memory_types.js';
import { Field, TypeTag, Uint8, Uint32 } from '../avm_memory_types.js';
import { markBytecodeAsAvm } from '../bytecode_utils.js';
import { adjustCalldataIndex, initContext, initHostStorage, initPersistableStateManager } from '../fixtures/index.js';
import { type HostStorage } from '../journal/host_storage.js';
Expand All @@ -14,7 +14,7 @@ import { mockGetBytecode, mockTraceFork } from '../test_utils.js';
import { L2GasLeft } from './context_getters.js';
import { Call, Return, Revert, StaticCall } from './external_calls.js';
import { type Instruction } from './instruction.js';
import { CalldataCopy } from './memory.js';
import { CalldataCopy, Set } from './memory.js';
import { SStore } from './storage.js';

describe('External Calls', () => {
Expand Down Expand Up @@ -83,12 +83,9 @@ describe('External Calls', () => {
// const otherContextInstructionsL2GasCost = 780; // Includes the cost of the call itself
const otherContextInstructionsBytecode = markBytecodeAsAvm(
encodeToBytecode([
new CalldataCopy(
/*indirect=*/ 0,
/*csOffset=*/ adjustCalldataIndex(0),
/*copySize=*/ argsSize,
/*dstOffset=*/ 0,
),
new Set(/*indirect=*/ 0, TypeTag.UINT32, adjustCalldataIndex(0), /*dstOffset=*/ 0),
new Set(/*indirect=*/ 0, TypeTag.UINT32, argsSize, /*dstOffset=*/ 1),
new CalldataCopy(/*indirect=*/ 0, /*csOffsetAddress=*/ 0, /*copySizeOffset=*/ 1, /*dstOffset=*/ 0),
new SStore(/*indirect=*/ 0, /*srcOffset=*/ valueOffset, /*slotOffset=*/ slotOffset),
new Return(/*indirect=*/ 0, /*retOffset=*/ 0, /*size=*/ 2),
]),
Expand Down
8 changes: 4 additions & 4 deletions yarn-project/simulator/src/avm/opcodes/memory.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,14 +432,14 @@ describe('Memory instructions', () => {
const buf = Buffer.from([
CalldataCopy.opcode, // opcode
0x01, // indirect
...Buffer.from('12345678', 'hex'), // cdOffset
...Buffer.from('23456789', 'hex'), // copysize
...Buffer.from('12345678', 'hex'), // cdOffsetAddress
...Buffer.from('23456789', 'hex'), // copysizeOffset
...Buffer.from('3456789a', 'hex'), // dstOffset
]);
const inst = new CalldataCopy(
/*indirect=*/ 0x01,
/*cdOffset=*/ 0x12345678,
/*copysize=*/ 0x23456789,
/*cdOffsetAddress=*/ 0x12345678,
/*copysizeOffset=*/ 0x23456789,
/*dstOffset=*/ 0x3456789a,
);

Expand Down

0 comments on commit 6add047

Please sign in to comment.