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

feat: Sync from noir #8663

Merged
merged 18 commits into from
Sep 23, 2024
Merged
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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0864e7c945089cc06f8cc9e5c7d933c465d8c892
5598059576c6cbc72474aff4b18bc5e4bb9f08e1
3 changes: 2 additions & 1 deletion barretenberg/acir_tests/run_acir_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ SKIP_ARRAY=(diamond_deps_0 workspace workspace_default_member)
# if HONK is false, we should skip verify_honk_proof
if [ "$HONK" = false ]; then
# Insert the new item into the array
SKIP_ARRAY+=(verify_honk_proof)
# TODO https://github.com/AztecProtocol/barretenberg/issues/1108
SKIP_ARRAY+=(verify_honk_proof regression_5045)
fi

function test() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,15 @@ pub enum BlackBoxFunc {
/// ultimately fail.
RecursiveAggregation,

/// Addition over the embedded curve on which the witness is defined.
/// Addition over the embedded curve on which the witness is defined
/// The opcode makes the following assumptions but does not enforce them because
/// it is more efficient to do it only when required. For instance, adding two
/// points that are on the curve it guarantee to give a point on the curve.
///
/// It assumes that the points are on the curve.
/// If the inputs are the same witnesses index, it will perform a doubling,
/// If not, it assumes that the points' x-coordinates are not equal.
/// It also assumes neither point is the infinity point.
EmbeddedCurveAdd,

/// BigInt addition
Expand Down
44 changes: 25 additions & 19 deletions noir/noir-repo/aztec_macros/src/transforms/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,14 +316,17 @@ fn create_static_check(fname: &str, is_private: bool) -> Statement {
.iter()
.fold(variable("context"), |acc, member| member_access(acc, member))
};
make_statement(StatementKind::Constrain(ConstrainStatement(
make_eq(is_static_call_expr, expression(ExpressionKind::Literal(Literal::Bool(true)))),
Some(expression(ExpressionKind::Literal(Literal::Str(format!(
"Function {} can only be called statically",
fname
))))),
ConstrainKind::Assert,
)))
make_statement(StatementKind::Constrain(ConstrainStatement {
kind: ConstrainKind::Assert,
arguments: vec![
make_eq(is_static_call_expr, expression(ExpressionKind::Literal(Literal::Bool(true)))),
expression(ExpressionKind::Literal(Literal::Str(format!(
"Function {} can only be called statically",
fname
)))),
],
span: Default::default(),
}))
}

/// Creates a check for internal functions ensuring that the caller is self.
Expand All @@ -332,17 +335,20 @@ fn create_static_check(fname: &str, is_private: bool) -> Statement {
/// assert(context.msg_sender() == context.this_address(), "Function can only be called internally");
/// ```
fn create_internal_check(fname: &str) -> Statement {
make_statement(StatementKind::Constrain(ConstrainStatement(
make_eq(
method_call(variable("context"), "msg_sender", vec![]),
method_call(variable("context"), "this_address", vec![]),
),
Some(expression(ExpressionKind::Literal(Literal::Str(format!(
"Function {} can only be called internally",
fname
))))),
ConstrainKind::Assert,
)))
make_statement(StatementKind::Constrain(ConstrainStatement {
kind: ConstrainKind::Assert,
arguments: vec![
make_eq(
method_call(variable("context"), "msg_sender", vec![]),
method_call(variable("context"), "this_address", vec![]),
),
expression(ExpressionKind::Literal(Literal::Str(format!(
"Function {} can only be called internally",
fname
)))),
],
span: Default::default(),
}))
}

/// Creates a call to assert_initialization_matches_address_preimage to be inserted
Expand Down
5 changes: 1 addition & 4 deletions noir/noir-repo/aztec_macros/src/utils/parse_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,7 @@ fn empty_statement(statement: &mut Statement) {
}

fn empty_constrain_statement(constrain_statement: &mut ConstrainStatement) {
empty_expression(&mut constrain_statement.0);
if let Some(expression) = &mut constrain_statement.1 {
empty_expression(expression);
}
empty_expressions(&mut constrain_statement.arguments);
}

fn empty_expressions(expressions: &mut [Expression]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ impl<'block> BrilligBlock<'block> {

self.brillig_context.deallocate_register(items_pointer);
}
Instruction::ArraySet { array, index, value, mutable: _ } => {
Instruction::ArraySet { array, index, value, mutable } => {
let source_variable = self.convert_ssa_value(*array, dfg);
let index_register = self.convert_ssa_single_addr_value(*index, dfg);
let value_variable = self.convert_ssa_value(*value, dfg);
Expand All @@ -688,6 +688,7 @@ impl<'block> BrilligBlock<'block> {
destination_variable,
index_register,
value_variable,
*mutable,
);
}
Instruction::RangeCheck { value, max_bit_size, assert_message } => {
Expand Down Expand Up @@ -868,34 +869,49 @@ impl<'block> BrilligBlock<'block> {
destination_variable: BrilligVariable,
index_register: SingleAddrVariable,
value_variable: BrilligVariable,
mutable: bool,
) {
assert!(index_register.bit_size == BRILLIG_MEMORY_ADDRESSING_BIT_SIZE);
match (source_variable, destination_variable) {
(
BrilligVariable::BrilligArray(source_array),
BrilligVariable::BrilligArray(destination_array),
) => {
self.brillig_context.call_array_copy_procedure(source_array, destination_array);
if !mutable {
self.brillig_context.call_array_copy_procedure(source_array, destination_array);
}
}
(
BrilligVariable::BrilligVector(source_vector),
BrilligVariable::BrilligVector(destination_vector),
) => {
self.brillig_context.call_vector_copy_procedure(source_vector, destination_vector);
if !mutable {
self.brillig_context
.call_vector_copy_procedure(source_vector, destination_vector);
}
}
_ => unreachable!("ICE: array set on non-array"),
}

let destination_for_store = if mutable { source_variable } else { destination_variable };
// Then set the value in the newly created array
let items_pointer =
self.brillig_context.codegen_make_array_or_vector_items_pointer(destination_variable);
self.brillig_context.codegen_make_array_or_vector_items_pointer(destination_for_store);

self.brillig_context.codegen_store_with_offset(
items_pointer,
index_register,
value_variable.extract_register(),
);

// If we mutated the source array we want instructions that use the destination array to point to the source array
if mutable {
self.brillig_context.mov_instruction(
destination_variable.extract_register(),
source_variable.extract_register(),
);
}

self.brillig_context.deallocate_register(items_pointer);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1471,6 +1471,7 @@ impl<F: AcirField> AcirContext<F> {
| BlackBoxFunc::AND
| BlackBoxFunc::XOR
| BlackBoxFunc::AES128Encrypt
| BlackBoxFunc::EmbeddedCurveAdd
);
// Convert `AcirVar` to `FunctionInput`
let inputs = self.prepare_inputs_for_black_box_func_call(inputs, allow_constant_inputs)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,12 @@ impl<'a> Context<'a> {
warnings.extend(self.acir_context.warnings.clone());

// Add the warnings from the alter Ssa passes
Ok(self.acir_context.finish(input_witness, return_witnesses, warnings))
Ok(self.acir_context.finish(
input_witness,
// Don't embed databus return witnesses into the circuit.
if self.data_bus.return_data.is_some() { Vec::new() } else { return_witnesses },
warnings,
))
}

fn initialize_databus(
Expand Down
11 changes: 11 additions & 0 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,17 @@ impl Function {
let returns = vecmap(self.returns(), |ret| self.dfg.type_of_value(*ret));
Signature { params, returns }
}

/// Finds the block of the function with the Return instruction
pub(crate) fn find_last_block(&self) -> BasicBlockId {
for block in self.reachable_blocks() {
if matches!(self.dfg[block].terminator(), Some(TerminatorInstruction::Return { .. })) {
return block;
}
}

unreachable!("SSA Function {} has no reachable return instruction!", self.id())
}
}

impl std::fmt::Display for RuntimeType {
Expand Down
15 changes: 9 additions & 6 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/array_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@ impl Ssa {
#[tracing::instrument(level = "trace", skip(self))]
pub(crate) fn array_set_optimization(mut self) -> Self {
for func in self.functions.values_mut() {
if !func.runtime().is_entry_point() {
let mut reachable_blocks = func.reachable_blocks();
let mut reachable_blocks = func.reachable_blocks();
let block = if !func.runtime().is_entry_point() {
assert_eq!(reachable_blocks.len(), 1, "Expected there to be 1 block remaining in Acir function for array_set optimization");
reachable_blocks.pop_first().unwrap()
} else {
// We only apply the array set optimization in the return block of Brillig functions
func.find_last_block()
};

let block = reachable_blocks.pop_first().unwrap();
let instructions_to_update = analyze_last_uses(&func.dfg, block);
make_mutable(&mut func.dfg, block, instructions_to_update);
}
let instructions_to_update = analyze_last_uses(&func.dfg, block);
make_mutable(&mut func.dfg, block, instructions_to_update);
}
self
}
Expand Down
36 changes: 20 additions & 16 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,8 @@ impl<'f> PerFunctionContext<'f> {
.filter(|param| self.inserter.function.dfg.value_is_reference(**param))
.collect::<BTreeSet<_>>();

// Must collect here as we are immutably borrowing `self` to fetch the reference parameters
let mut values_to_reduce_counts = Vec::new();
for (allocation, instruction) in &references.last_stores {
if let Some(expression) = references.expressions.get(allocation) {
if let Some(aliases) = references.aliases.get(expression) {
Expand All @@ -297,13 +299,15 @@ impl<'f> PerFunctionContext<'f> {
// If `allocation_aliases_parameter` is known to be false
if allocation_aliases_parameter == Some(false) {
self.instructions_to_remove.insert(*instruction);
if let Some(context) = self.load_results.get_mut(allocation) {
context.uses -= 1;
}
values_to_reduce_counts.push(*allocation);
}
}
}
}

for value in values_to_reduce_counts {
self.reduce_load_result_count(value);
}
}

fn increase_load_ref_counts(&mut self, value: ValueId) {
Expand Down Expand Up @@ -406,25 +410,17 @@ impl<'f> PerFunctionContext<'f> {
else {
panic!("Should have a store instruction here");
};
if let Some(context) = self.load_results.get_mut(&address) {
context.uses -= 1;
}
if let Some(context) = self.load_results.get_mut(&value) {
context.uses -= 1;
}
self.reduce_load_result_count(address);
self.reduce_load_result_count(value);
}

let known_value = references.get_known_value(value);
if let Some(known_value) = known_value {
let known_value_is_address = known_value == address;
if known_value_is_address {
self.instructions_to_remove.insert(instruction);
if let Some(context) = self.load_results.get_mut(&address) {
context.uses -= 1;
}
if let Some(context) = self.load_results.get_mut(&value) {
context.uses -= 1;
}
self.reduce_load_result_count(address);
self.reduce_load_result_count(value);
} else {
references.last_stores.insert(address, instruction);
}
Expand Down Expand Up @@ -617,6 +613,13 @@ impl<'f> PerFunctionContext<'f> {
}
}

fn reduce_load_result_count(&mut self, value: ValueId) {
if let Some(context) = self.load_results.get_mut(&value) {
// TODO this was saturating https://github.com/noir-lang/noir/issues/6124
context.uses = context.uses.wrapping_sub(1);
}
}

fn recursively_add_values(&self, value: ValueId, set: &mut HashSet<ValueId>) {
set.insert(value);
if let Some((elements, _)) = self.inserter.function.dfg.get_array_constant(value) {
Expand Down Expand Up @@ -741,7 +744,8 @@ impl<'f> PerFunctionContext<'f> {
if all_loads_removed && !store_alias_used {
self.instructions_to_remove.insert(*store_instruction);
if let Some((_, counter)) = remaining_last_stores.get_mut(store_address) {
*counter -= 1;
// TODO this was saturating https://github.com/noir-lang/noir/issues/6124
*counter = counter.wrapping_sub(1);
}
} else if let Some((_, counter)) = remaining_last_stores.get_mut(store_address) {
*counter += 1;
Expand Down
19 changes: 2 additions & 17 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ use std::collections::{HashMap, HashSet};

use crate::ssa::{
ir::{
basic_block::BasicBlockId,
function::Function,
instruction::{Instruction, InstructionId, TerminatorInstruction},
instruction::{Instruction, InstructionId},
types::Type,
value::ValueId,
},
Expand Down Expand Up @@ -107,7 +106,7 @@ impl Context {
/// Find each dec_rc instruction and if the most recent inc_rc instruction for the same value
/// is not possibly mutated, then we can remove them both. Returns each such pair.
fn find_rcs_to_remove(&mut self, function: &Function) -> HashSet<InstructionId> {
let last_block = Self::find_last_block(function);
let last_block = function.find_last_block();
let mut to_remove = HashSet::new();

for instruction in function.dfg[last_block].instructions() {
Expand All @@ -124,20 +123,6 @@ impl Context {
to_remove
}

/// Finds the block of the function with the Return instruction
fn find_last_block(function: &Function) -> BasicBlockId {
for block in function.reachable_blocks() {
if matches!(
function.dfg[block].terminator(),
Some(TerminatorInstruction::Return { .. })
) {
return block;
}
}

unreachable!("SSA Function {} has no reachable return instruction!", function.id())
}

/// Finds and pops the IncRc for the given array value if possible.
fn pop_rc_for(&mut self, value: ValueId, function: &Function) -> Option<IncRc> {
let typ = function.dfg.type_of_value(value);
Expand Down
Loading
Loading