Skip to content

Commit

Permalink
feat: Sync from noir (AztecProtocol#6203)
Browse files Browse the repository at this point in the history
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
feat: Optimize array sets in if conditions (alternate version)
(noir-lang/noir#4716)
chore: rename instruction checks for side effects
(noir-lang/noir#4945)
chore: Switch Noir JS to use execute program instead of circuit
(noir-lang/noir#4965)
fix: Use annotated type when checking declaration
(noir-lang/noir#4966)
feat: handle empty response foreign calls without an external resolver
(noir-lang/noir#4959)
feat: Complex outputs from acir call
(noir-lang/noir#4952)
END_COMMIT_OVERRIDE

---------

Co-authored-by: TomAFrench <tom@tomfren.ch>
Co-authored-by: vezenovm <mvezenov@gmail.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
  • Loading branch information
4 people authored May 4, 2024
1 parent 4b43295 commit 3ed41a0
Show file tree
Hide file tree
Showing 42 changed files with 944 additions and 191 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
d4c68066ab35ce1c52510cf0c038fb627a0677c3
a87c655c6c8c077c71e3372cc9181b7870348a3d
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ impl<'block> BrilligBlock<'block> {
destination_variable,
);
}
Instruction::ArraySet { array, index, value, .. } => {
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 Down Expand Up @@ -700,6 +700,9 @@ impl<'block> BrilligBlock<'block> {
Instruction::EnableSideEffects { .. } => {
todo!("enable_side_effects not supported by brillig")
}
Instruction::IfElse { .. } => {
unreachable!("IfElse instructions should not be possible in brillig")
}
};

let dead_variables = self
Expand Down
1 change: 1 addition & 0 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ pub(crate) fn optimize_into_acir(
// Run the inlining pass again to handle functions with `InlineType::NoPredicates`.
// Before flattening is run, we treat functions marked with the `InlineType::NoPredicates` as an entry point.
.run_pass(Ssa::inline_functions_with_no_predicates, "After Inlining:")
.run_pass(Ssa::remove_if_else, "After Remove IfElse:")
.run_pass(Ssa::fold_constants, "After Constant Folding:")
.run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffects removal:")
.run_pass(Ssa::fold_constants_using_constraints, "After Constraint Folding:")
Expand Down
14 changes: 9 additions & 5 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,9 @@ impl<'a> Context<'a> {
assert_message.clone(),
)?;
}
Instruction::IfElse { .. } => {
unreachable!("IfElse instruction remaining in acir-gen")
}
}

self.acir_context.set_call_stack(CallStack::new());
Expand Down Expand Up @@ -732,11 +735,10 @@ impl<'a> Context<'a> {
assert!(!matches!(inline_type, InlineType::Inline), "ICE: Got an ACIR function named {} that should have already been inlined", func.name());

let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg));
// TODO(https://github.com/noir-lang/noir/issues/4608): handle complex return types from ACIR functions
let output_count =
result_ids.iter().fold(0usize, |sum, result_id| {
sum + dfg.try_get_array_length(*result_id).unwrap_or(1)
});
let output_count = result_ids
.iter()
.map(|result_id| dfg.type_of_value(*result_id).flattened_size())
.sum();

let acir_function_id = ssa
.entry_point_to_generated_index
Expand All @@ -748,6 +750,7 @@ impl<'a> Context<'a> {
output_count,
self.current_side_effects_enabled_var,
)?;

let output_values =
self.convert_vars_to_values(output_vars, dfg, result_ids);

Expand Down Expand Up @@ -1028,6 +1031,7 @@ impl<'a> Context<'a> {
});
}
};

if self.acir_context.is_constant_one(&self.current_side_effects_enabled_var) {
// Report the error if side effects are enabled.
if index >= array_size {
Expand Down
23 changes: 22 additions & 1 deletion noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,10 @@ impl<'dfg> InsertInstructionResult<'dfg> {
match self {
InsertInstructionResult::SimplifiedTo(value) => *value,
InsertInstructionResult::SimplifiedToMultiple(values) => values[0],
InsertInstructionResult::Results(_, results) => results[0],
InsertInstructionResult::Results(_, results) => {
assert_eq!(results.len(), 1);
results[0]
}
InsertInstructionResult::InstructionRemoved => {
panic!("Instruction was removed, no results")
}
Expand Down Expand Up @@ -583,6 +586,24 @@ impl<'dfg> InsertInstructionResult<'dfg> {
}
}

impl<'dfg> std::ops::Index<usize> for InsertInstructionResult<'dfg> {
type Output = ValueId;

fn index(&self, index: usize) -> &Self::Output {
match self {
InsertInstructionResult::Results(_, results) => &results[index],
InsertInstructionResult::SimplifiedTo(result) => {
assert_eq!(index, 0);
result
}
InsertInstructionResult::SimplifiedToMultiple(results) => &results[index],
InsertInstructionResult::InstructionRemoved => {
panic!("Cannot index into InsertInstructionResult::InstructionRemoved")
}
}
}
}

#[cfg(test)]
mod tests {
use super::DataFlowGraph;
Expand Down
144 changes: 116 additions & 28 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use fxhash::FxHasher;
use iter_extended::vecmap;
use noirc_frontend::hir_def::types::Type as HirType;

use crate::ssa::opt::flatten_cfg::value_merger::ValueMerger;

use super::{
basic_block::BasicBlockId,
dfg::{CallStack, DataFlowGraph},
Expand Down Expand Up @@ -216,6 +218,14 @@ pub(crate) enum Instruction {
/// implemented via reference counting. In ACIR code this is done with im::Vector and these
/// DecrementRc instructions are ignored.
DecrementRc { value: ValueId },

/// Merge two values returned from opposite branches of a conditional into one.
IfElse {
then_condition: ValueId,
then_value: ValueId,
else_condition: ValueId,
else_value: ValueId,
},
}

impl Instruction {
Expand All @@ -229,10 +239,12 @@ impl Instruction {
match self {
Instruction::Binary(binary) => binary.result_type(),
Instruction::Cast(_, typ) => InstructionResultType::Known(typ.clone()),
Instruction::Not(value) | Instruction::Truncate { value, .. } => {
Instruction::Not(value)
| Instruction::Truncate { value, .. }
| Instruction::ArraySet { array: value, .. }
| Instruction::IfElse { then_value: value, .. } => {
InstructionResultType::Operand(*value)
}
Instruction::ArraySet { array, .. } => InstructionResultType::Operand(*array),
Instruction::Constrain(..)
| Instruction::Store { .. }
| Instruction::IncrementRc { .. }
Expand All @@ -252,20 +264,11 @@ impl Instruction {
matches!(self.result_type(), InstructionResultType::Unknown)
}

/// Pure `Instructions` are instructions which have no side-effects and results are a function of the inputs only,
/// i.e. there are no interactions with memory.
///
/// Pure instructions can be replaced with the results of another pure instruction with the same inputs.
pub(crate) fn is_pure(&self, dfg: &DataFlowGraph) -> bool {
/// Indicates if the instruction can be safely replaced with the results of another instruction with the same inputs.
pub(crate) fn can_be_deduplicated(&self, dfg: &DataFlowGraph) -> bool {
use Instruction::*;

match self {
Binary(bin) => {
// In ACIR, a division with a false predicate outputs (0,0), so it cannot replace another instruction unless they have the same predicate
bin.operator != BinaryOp::Div
}
Cast(_, _) | Truncate { .. } | Not(_) => true,

// These either have side-effects or interact with memory
Constrain(..)
| EnableSideEffects { .. }
Expand All @@ -276,31 +279,37 @@ impl Instruction {
| DecrementRc { .. }
| RangeCheck { .. } => false,

// These can have different behavior depending on the EnableSideEffectsIf context.
// Enabling constant folding for these potentially enables replacing an enabled
// array get with one that was disabled. See
// https://github.com/noir-lang/noir/pull/4716#issuecomment-2047846328.
ArrayGet { .. } | ArraySet { .. } => false,

Call { func, .. } => match dfg[*func] {
Value::Intrinsic(intrinsic) => !intrinsic.has_side_effects(),
_ => false,
},

// These can have different behavior depending on the EnableSideEffectsIf context.
// Replacing them with a similar instruction potentially enables replacing an instruction
// with one that was disabled. See
// https://github.com/noir-lang/noir/pull/4716#issuecomment-2047846328.
Binary(_)
| Cast(_, _)
| Not(_)
| Truncate { .. }
| IfElse { .. }
| ArrayGet { .. }
| ArraySet { .. } => !self.requires_acir_gen_predicate(dfg),
}
}

pub(crate) fn has_side_effects(&self, dfg: &DataFlowGraph) -> bool {
pub(crate) fn can_eliminate_if_unused(&self, dfg: &DataFlowGraph) -> bool {
use Instruction::*;
match self {
Binary(binary) => {
if matches!(binary.operator, BinaryOp::Div | BinaryOp::Mod) {
if let Some(rhs) = dfg.get_numeric_constant(binary.rhs) {
rhs == FieldElement::zero()
rhs != FieldElement::zero()
} else {
true
false
}
} else {
false
true
}
}
Cast(_, _)
Expand All @@ -309,32 +318,67 @@ impl Instruction {
| Allocate
| Load { .. }
| ArrayGet { .. }
| ArraySet { .. } => false,
| IfElse { .. }
| ArraySet { .. } => true,

Constrain(..)
| Store { .. }
| EnableSideEffects { .. }
| IncrementRc { .. }
| DecrementRc { .. }
| RangeCheck { .. } => true,
| RangeCheck { .. } => false,

// Some `Intrinsic`s have side effects so we must check what kind of `Call` this is.
Call { func, .. } => match dfg[*func] {
Value::Intrinsic(intrinsic) => intrinsic.has_side_effects(),
Value::Intrinsic(intrinsic) => !intrinsic.has_side_effects(),

// All foreign functions are treated as having side effects.
// This is because they can be used to pass information
// from the ACVM to the external world during execution.
Value::ForeignFunction(_) => true,
Value::ForeignFunction(_) => false,

// We must assume that functions contain a side effect as we cannot inspect more deeply.
Value::Function(_) => true,
Value::Function(_) => false,

_ => false,
},
}
}

/// If true the instruction will depends on enable_side_effects context during acir-gen
fn requires_acir_gen_predicate(&self, dfg: &DataFlowGraph) -> bool {
match self {
Instruction::Binary(binary)
if matches!(binary.operator, BinaryOp::Div | BinaryOp::Mod) =>
{
true
}
Instruction::EnableSideEffects { .. }
| Instruction::ArrayGet { .. }
| Instruction::ArraySet { .. } => true,

Instruction::Call { func, .. } => match dfg[*func] {
Value::Function(_) => true,
Value::Intrinsic(intrinsic) => {
matches!(intrinsic, Intrinsic::SliceInsert | Intrinsic::SliceRemove)
}
_ => false,
},
Instruction::Cast(_, _)
| Instruction::Binary(_)
| Instruction::Not(_)
| Instruction::Truncate { .. }
| Instruction::Constrain(_, _, _)
| Instruction::RangeCheck { .. }
| Instruction::Allocate
| Instruction::Load { .. }
| Instruction::Store { .. }
| Instruction::IfElse { .. }
| Instruction::IncrementRc { .. }
| Instruction::DecrementRc { .. } => false,
}
}

/// Maps each ValueId inside this instruction to a new ValueId, returning the new instruction.
/// Note that the returned instruction is fresh and will not have an assigned InstructionId
/// until it is manually inserted in a DataFlowGraph later.
Expand Down Expand Up @@ -397,6 +441,14 @@ impl Instruction {
assert_message: assert_message.clone(),
}
}
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
Instruction::IfElse {
then_condition: f(*then_condition),
then_value: f(*then_value),
else_condition: f(*else_condition),
else_value: f(*else_value),
}
}
}
}

Expand Down Expand Up @@ -451,6 +503,12 @@ impl Instruction {
| Instruction::RangeCheck { value, .. } => {
f(*value);
}
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
f(*then_condition);
f(*then_value);
f(*else_condition);
f(*else_value);
}
}
}

Expand Down Expand Up @@ -602,6 +660,36 @@ impl Instruction {
None
}
}
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
let typ = dfg.type_of_value(*then_value);

if let Some(constant) = dfg.get_numeric_constant(*then_condition) {
if constant.is_one() {
return SimplifiedTo(*then_value);
} else if constant.is_zero() {
return SimplifiedTo(*else_value);
}
}

if matches!(&typ, Type::Numeric(_)) {
let then_condition = *then_condition;
let then_value = *then_value;
let else_condition = *else_condition;
let else_value = *else_value;

let result = ValueMerger::merge_numeric_values(
dfg,
block,
then_condition,
else_condition,
then_value,
else_value,
);
SimplifiedTo(result)
} else {
None
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,9 @@ fn simplify_slice_push_back(
slice_sizes.insert(set_last_slice_value, slice_size / element_size);
slice_sizes.insert(new_slice, slice_size / element_size);

let mut value_merger = ValueMerger::new(dfg, block, &mut slice_sizes);
let unknown = &mut HashMap::default();
let mut value_merger = ValueMerger::new(dfg, block, &mut slice_sizes, unknown, None);

let new_slice = value_merger.merge_values(
len_not_equals_capacity,
len_equals_capacity,
Expand Down
12 changes: 11 additions & 1 deletion noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ fn display_instruction_inner(
let index = show(*index);
let value = show(*value);
let mutable = if *mutable { " mut" } else { "" };
writeln!(f, "array_set{mutable} {array}, index {index}, value {value}",)
writeln!(f, "array_set{mutable} {array}, index {index}, value {value}")
}
Instruction::IncrementRc { value } => {
writeln!(f, "inc_rc {}", show(*value))
Expand All @@ -192,6 +192,16 @@ fn display_instruction_inner(
Instruction::RangeCheck { value, max_bit_size, .. } => {
writeln!(f, "range_check {} to {} bits", show(*value), *max_bit_size,)
}
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
let then_condition = show(*then_condition);
let then_value = show(*then_value);
let else_condition = show(*else_condition);
let else_value = show(*else_value);
writeln!(
f,
"if {then_condition} then {then_value} else if {else_condition} then {else_value}"
)
}
}
}

Expand Down
Loading

0 comments on commit 3ed41a0

Please sign in to comment.