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

chore: Optimize Brillig array copies [DO NOT MERGE] #2669

Closed
wants to merge 1 commit into from
Closed
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
68 changes: 23 additions & 45 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::HashMap;

use crate::brillig::brillig_ir::{
BrilligBinaryOp, BrilligContext, BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE,
};
Expand Down Expand Up @@ -27,6 +29,8 @@
pub(crate) block_id: BasicBlockId,
/// Context for creating brillig opcodes
pub(crate) brillig_context: &'block mut BrilligContext,
/// Forms an equivalence set between arrays
pub(crate) array_maps: HashMap<ValueId, ValueId>,
}

impl<'block> BrilligBlock<'block> {
Expand All @@ -37,7 +41,12 @@
block_id: BasicBlockId,
dfg: &DataFlowGraph,
) {
let mut brillig_block = BrilligBlock { function_context, block_id, brillig_context };
let mut brillig_block = BrilligBlock {
function_context,
block_id,
brillig_context,
array_maps: Default::default(),
};

brillig_block.convert_block(dfg);
}
Expand Down Expand Up @@ -236,8 +245,11 @@
self.brillig_context.allocate_variable_instruction(address_register);
}
Instruction::Store { address, value } => {
let source_variable = match self.array_maps.get(value) {
Some(real_map_pointer) => self.convert_ssa_value(*real_map_pointer, dfg),
None => self.convert_ssa_value(*value, dfg),
};
let address_register = self.convert_ssa_register_value(*address, dfg);
let source_variable = self.convert_ssa_value(*value, dfg);

self.brillig_context.store_variable_instruction(address_register, source_variable);
}
Expand Down Expand Up @@ -497,16 +509,15 @@
let index_register = self.convert_ssa_register_value(*index, dfg);
let value_variable = self.convert_ssa_value(*value, dfg);

// Store destination in map

let result_ids = dfg.instruction_results(instruction_id);
let destination_variable =
self.function_context.create_variable(self.brillig_context, result_ids[0], dfg);

self.convert_ssa_array_set(
source_variable,
destination_variable,
index_register,
value_variable,
);
// Equivalence set

self.array_maps.insert(result_ids[0], *array);

self.convert_ssa_array_set(source_variable, index_register, value_variable);
}
_ => todo!("ICE: Instruction not supported {instruction:?}"),
};
Expand Down Expand Up @@ -595,50 +606,17 @@
fn convert_ssa_array_set(
&mut self,
source_variable: RegisterOrMemory,
destination_variable: RegisterOrMemory,
index_register: RegisterIndex,
value_variable: RegisterOrMemory,
) {
let destination_pointer = match destination_variable {
let source_pointer = match source_variable {
RegisterOrMemory::HeapArray(HeapArray { pointer, .. }) => pointer,
RegisterOrMemory::HeapVector(HeapVector { pointer, .. }) => pointer,
_ => unreachable!("ICE: array set returns non-array"),
};

// First issue a array copy to the destination
let (source_pointer, source_size_as_register) = match source_variable {
RegisterOrMemory::HeapArray(HeapArray { size, pointer }) => {
let source_size_register = self.brillig_context.allocate_register();
self.brillig_context.const_instruction(source_size_register, size.into());
(pointer, source_size_register)
}
RegisterOrMemory::HeapVector(HeapVector { size, pointer }) => {
let source_size_register = self.brillig_context.allocate_register();
self.brillig_context.mov_instruction(source_size_register, size);
(pointer, source_size_register)
}
_ => unreachable!("ICE: array set on non-array"),
};

self.brillig_context
.allocate_array_instruction(destination_pointer, source_size_as_register);

self.brillig_context.copy_array_instruction(
source_pointer,
destination_pointer,
source_size_as_register,
);

if let RegisterOrMemory::HeapVector(HeapVector { size: target_size, .. }) =
destination_variable
{
self.brillig_context.mov_instruction(target_size, source_size_as_register);
}

// Then set the value in the newly created array
self.store_variable_in_array(destination_pointer, index_register, value_variable);

self.brillig_context.deallocate_register(source_size_as_register);
self.store_variable_in_array(source_pointer, index_register, value_variable);
}

pub(crate) fn store_variable_in_array(
Expand Down Expand Up @@ -858,7 +836,7 @@

/// Slices have a tuple structure (slice length, slice contents) to enable logic
/// that uses dynamic slice lengths (such as with merging slices in the flattening pass).
/// This method codegens an update to the slice length.

Check warning on line 839 in compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (codegens)
///
/// The binary operation performed on the slice length is always an addition or subtraction of `1`.
/// This is because the slice length holds the user length (length as displayed by a `.len()` call),
Expand Down
7 changes: 7 additions & 0 deletions tooling/nargo_cli/tests/execution_success/oof/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "brillig_slow_array"
type = "bin"
authors = [""]
compiler_version = "0.1"

[dependencies]
1 change: 1 addition & 0 deletions tooling/nargo_cli/tests/execution_success/oof/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
data = "3"
11 changes: 11 additions & 0 deletions tooling/nargo_cli/tests/execution_success/oof/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
unconstrained fn main(
data: Field,
) -> pub [u1;2048]{
let mut bits = [0; 2048];
for i in 0..2048 {
if i != data {
bits[i] = 1;
}
}
bits
}
Loading