Skip to content

Commit

Permalink
Merge 745c9d5 into 920077d
Browse files Browse the repository at this point in the history
  • Loading branch information
jfecher authored Dec 2, 2024
2 parents 920077d + 745c9d5 commit f600373
Show file tree
Hide file tree
Showing 14 changed files with 98 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ impl<'block> BrilligBlock<'block> {
destination_vector,
source_size_register,
None,
None,
);

// Items
Expand Down Expand Up @@ -813,13 +814,14 @@ impl<'block> BrilligBlock<'block> {
// Initialize the variable
match new_variable {
BrilligVariable::BrilligArray(brillig_array) => {
self.brillig_context.codegen_initialize_array(brillig_array);
self.brillig_context.codegen_initialize_array(brillig_array, None);
}
BrilligVariable::BrilligVector(vector) => {
let size = self
.brillig_context
.make_usize_constant_instruction(array.len().into());
self.brillig_context.codegen_initialize_vector(vector, size, None);
self.brillig_context
.codegen_initialize_vector(vector, size, None, None);
self.brillig_context.deallocate_single_addr(size);
}
_ => unreachable!(
Expand Down Expand Up @@ -1814,7 +1816,7 @@ impl<'block> BrilligBlock<'block> {
unreachable!("ICE: allocate_foreign_call_array() expects an array, got {typ:?}")
};

self.brillig_context.codegen_initialize_array(array);
self.brillig_context.codegen_initialize_array(array, None);

let mut index = 0_usize;
for _ in 0..*size {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
assert!(source_field.bit_size == F::max_num_bits());
assert!(radix.bit_size == 32);

self.codegen_initialize_array(target_array);
self.codegen_initialize_array(target_array, None);

let heap_array = self.codegen_brillig_array_to_heap_array(target_array);

Expand Down
18 changes: 13 additions & 5 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use super::{
BrilligContext, ReservedRegisters, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
};

const INITIAL_ARRAY_REF_COUNT: usize = 0;

impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<F, Registers> {
/// Allocates an array of size `size` and stores the pointer to the array
/// in `pointer_register`
Expand Down Expand Up @@ -419,12 +421,16 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
}

/// Initializes an array, allocating memory to store its representation and initializing the reference counter.
pub(crate) fn codegen_initialize_array(&mut self, array: BrilligArray) {
pub(crate) fn codegen_initialize_array(
&mut self,
array: BrilligArray,
initial_rc: Option<usize>,
) {
self.codegen_allocate_immediate_mem(array.pointer, array.size + 1);
self.indirect_const_instruction(
array.pointer,
BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
1_usize.into(),
initial_rc.unwrap_or(INITIAL_ARRAY_REF_COUNT).into(),
);
}

Expand All @@ -433,12 +439,13 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
vector: BrilligVector,
size: SingleAddrVariable,
capacity: Option<SingleAddrVariable>,
initial_rc: Option<usize>,
) {
// Write RC
self.indirect_const_instruction(
vector.pointer,
BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
1_usize.into(),
initial_rc.unwrap_or(INITIAL_ARRAY_REF_COUNT).into(),
);

// Write size
Expand All @@ -459,6 +466,7 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
vector: BrilligVector,
size: SingleAddrVariable,
capacity: Option<SingleAddrVariable>, // Defaults to size if None
initial_rc: Option<usize>, // Defaults to INITIAL_ARRAY_REF_COUNT if none
) {
let allocation_size = self.allocate_register();
// Allocation size = capacity + 3 (rc, size, capacity)
Expand All @@ -471,7 +479,7 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
self.codegen_allocate_mem(vector.pointer, allocation_size);
self.deallocate_register(allocation_size);

self.codegen_initialize_vector_metadata(vector, size, capacity);
self.codegen_initialize_vector_metadata(vector, size, capacity, initial_rc);
}

/// We don't know the length of a vector returned externally before the call
Expand All @@ -498,7 +506,7 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
self.indirect_const_instruction(
vector.pointer,
BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
1_usize.into(),
INITIAL_ARRAY_REF_COUNT.into(),
);

// Initialize size
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,15 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
let deflattened_items_pointer = if is_vector {
let vector = BrilligVector { pointer: deflattened_array_pointer };

self.codegen_initialize_vector(vector, deflattened_size_variable, None);
self.codegen_initialize_vector(vector, deflattened_size_variable, None, Some(1));

self.codegen_make_vector_items_pointer(vector)
} else {
let arr = BrilligArray {
pointer: deflattened_array_pointer,
size: item_count * item_type.len(),
};
self.codegen_initialize_array(arr);
self.codegen_initialize_array(arr, Some(1));
self.codegen_make_array_items_pointer(arr)
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ pub(crate) fn reallocate_vector_for_insertion<
target_vector,
target_size,
Some(source_capacity),
Some(1),
);
}
});
Expand All @@ -201,6 +202,7 @@ pub(crate) fn reallocate_vector_for_insertion<
target_vector,
target_size,
Some(double_size),
Some(1),
);
brillig_context.deallocate_single_addr(double_size);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ pub(super) fn compile_vector_pop_back_procedure<F: AcirField + DebugToString>(
brillig_context.codegen_update_vector_length(target_vector, target_size);
} else {
// We need to clone the source vector
brillig_context.codegen_initialize_vector(target_vector, target_size, None);
brillig_context.codegen_initialize_vector(target_vector, target_size, None, Some(1));

let target_vector_items_pointer =
brillig_context.codegen_make_vector_items_pointer(target_vector);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ pub(super) fn compile_vector_pop_front_procedure<F: AcirField + DebugToString>(
target_vector,
target_size,
Some(source_capacity),
Some(1),
);
} else {
brillig_context.codegen_initialize_vector(target_vector, target_size, None);
brillig_context.codegen_initialize_vector(target_vector, target_size, None, Some(1));

let target_vector_items_pointer =
brillig_context.codegen_make_vector_items_pointer(target_vector);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub(super) fn compile_vector_remove_procedure<F: AcirField + DebugToString>(
brillig_context
.codegen_vector_items_pointer(target_vector, target_vector_items_pointer);
} else {
brillig_context.codegen_initialize_vector(target_vector, target_size, None);
brillig_context.codegen_initialize_vector(target_vector, target_size, None, Some(1));

// Copy the elements to the left of the index
brillig_context
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ impl<'f> FunctionInserter<'f> {
// another MakeArray instruction. Note that this assumes the function inserter is inserting
// in control-flow order. Otherwise we could refer to ValueIds defined later in the program.
let make_array = if let Instruction::MakeArray { elements, typ } = &instruction {
if self.array_is_constant(elements) {
// Array caching is disabled for brillig functions since they may be mutated directly
if self.array_is_constant(elements) && self.function.runtime().is_acir() {
if let Some(fetched_value) = self.get_cached_array(elements, typ) {
assert_eq!(results.len(), 1);
self.values.insert(results[0], fetched_value);
Expand Down
10 changes: 5 additions & 5 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ impl Instruction {
/// conditional on whether the caller wants the predicate to be taken into account or not.
pub(crate) fn can_be_deduplicated(
&self,
dfg: &DataFlowGraph,
function: &Function,
deduplicate_with_predicate: bool,
) -> bool {
use Instruction::*;
Expand All @@ -438,7 +438,7 @@ impl Instruction {
| IncrementRc { .. }
| DecrementRc { .. } => false,

Call { func, .. } => match dfg[*func] {
Call { func, .. } => match function.dfg[*func] {
Value::Intrinsic(intrinsic) => {
intrinsic.can_be_deduplicated(deduplicate_with_predicate)
}
Expand All @@ -448,8 +448,8 @@ impl Instruction {
// We can deduplicate these instructions if we know the predicate is also the same.
Constrain(..) | RangeCheck { .. } => deduplicate_with_predicate,

// This should never be side-effectful
MakeArray { .. } => true,
// Arrays can be mutated in unconstrained code so we can't deduplicate there
MakeArray { .. } => function.runtime().is_acir(),

// These can have different behavior depending on the EnableSideEffectsIf context.
// Replacing them with a similar instruction potentially enables replacing an instruction
Expand All @@ -462,7 +462,7 @@ impl Instruction {
| IfElse { .. }
| ArrayGet { .. }
| ArraySet { .. } => {
deduplicate_with_predicate || !self.requires_acir_gen_predicate(dfg)
deduplicate_with_predicate || !self.requires_acir_gen_predicate(&function.dfg)
}
}
}
Expand Down
48 changes: 27 additions & 21 deletions compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl Function {
}

context.visited_blocks.insert(block);
context.fold_constants_in_block(&mut self.dfg, &mut dom, block);
context.fold_constants_in_block(self, &mut dom, block);
}
}
}
Expand Down Expand Up @@ -266,49 +266,55 @@ impl<'brillig> Context<'brillig> {

fn fold_constants_in_block(
&mut self,
dfg: &mut DataFlowGraph,
function: &mut Function,
dom: &mut DominatorTree,
block: BasicBlockId,
) {
let instructions = dfg[block].take_instructions();
let instructions = function.dfg[block].take_instructions();

// Default side effect condition variable with an enabled state.
let mut side_effects_enabled_var = dfg.make_constant(FieldElement::one(), Type::bool());
let mut side_effects_enabled_var =
function.dfg.make_constant(FieldElement::one(), Type::bool());

for instruction_id in instructions {
self.fold_constants_into_instruction(
dfg,
function,
dom,
block,
instruction_id,
&mut side_effects_enabled_var,
);
}
self.block_queue.extend(dfg[block].successors());
self.block_queue.extend(function.dfg[block].successors());
}

fn fold_constants_into_instruction(
&mut self,
dfg: &mut DataFlowGraph,
function: &mut Function,
dom: &mut DominatorTree,
mut block: BasicBlockId,
id: InstructionId,
side_effects_enabled_var: &mut ValueId,
) {
let constraint_simplification_mapping = self.get_constraint_map(*side_effects_enabled_var);

let instruction =
Self::resolve_instruction(id, block, dfg, dom, constraint_simplification_mapping);
let instruction = Self::resolve_instruction(
id,
block,
&function.dfg,
dom,
constraint_simplification_mapping,
);

let old_results = dfg.instruction_results(id).to_vec();
let old_results = function.dfg.instruction_results(id).to_vec();

// If a copy of this instruction exists earlier in the block, then reuse the previous results.
if let Some(cache_result) =
self.get_cached(dfg, dom, &instruction, *side_effects_enabled_var, block)
self.get_cached(&function.dfg, dom, &instruction, *side_effects_enabled_var, block)
{
match cache_result {
CacheResult::Cached(cached) => {
Self::replace_result_ids(dfg, &old_results, cached);
Self::replace_result_ids(&mut function.dfg, &old_results, cached);
return;
}
CacheResult::NeedToHoistToCommonBlock(dominator) => {
Expand All @@ -327,7 +333,7 @@ impl<'brillig> Context<'brillig> {
&instruction,
&old_results,
block,
dfg,
&mut function.dfg,
self.brillig_info,
)
.unwrap_or_else(|| {
Expand All @@ -337,16 +343,16 @@ impl<'brillig> Context<'brillig> {
instruction.clone(),
&old_results,
block,
dfg,
&mut function.dfg,
)
});

Self::replace_result_ids(dfg, &old_results, &new_results);
Self::replace_result_ids(&mut function.dfg, &old_results, &new_results);

self.cache_instruction(
instruction.clone(),
new_results,
dfg,
function,
*side_effects_enabled_var,
block,
);
Expand Down Expand Up @@ -433,7 +439,7 @@ impl<'brillig> Context<'brillig> {
&mut self,
instruction: Instruction,
instruction_results: Vec<ValueId>,
dfg: &DataFlowGraph,
function: &Function,
side_effects_enabled_var: ValueId,
block: BasicBlockId,
) {
Expand All @@ -442,21 +448,21 @@ impl<'brillig> Context<'brillig> {
// to map from the more complex to the simpler value.
if let Instruction::Constrain(lhs, rhs, _) = instruction {
// These `ValueId`s should be fully resolved now.
if let Some((complex, simple)) = simplify(dfg, lhs, rhs) {
if let Some((complex, simple)) = simplify(&function.dfg, lhs, rhs) {
self.get_constraint_map(side_effects_enabled_var)
.entry(complex)
.or_default()
.add(dfg, simple, block);
.add(&function.dfg, simple, block);
}
}
}

// If the instruction doesn't have side-effects and if it won't interact with enable_side_effects during acir_gen,
// we cache the results so we can reuse them if the same instruction appears again later in the block.
// Others have side effects representing failure, which are implicit in the ACIR code and can also be deduplicated.
if instruction.can_be_deduplicated(dfg, self.use_constraint_info) {
if instruction.can_be_deduplicated(function, self.use_constraint_info) {
let use_predicate =
self.use_constraint_info && instruction.requires_acir_gen_predicate(dfg);
self.use_constraint_info && instruction.requires_acir_gen_predicate(&function.dfg);
let predicate = use_predicate.then_some(side_effects_enabled_var);

self.cached_instruction_results
Expand Down
Loading

0 comments on commit f600373

Please sign in to comment.