Skip to content
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
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ impl<'a> Context<'a> {
let mut warnings = Vec::new();
// Disable the side effects if the binary instruction does not require them
let one = self.acir_context.add_constant(FieldElement::one());
let predicate = if instruction.requires_acir_gen_predicate(dfg) {
let predicate = if instruction.requires_acir_gen_predicate(dfg, &ssa.function_purities) {
self.current_side_effects_enabled_var
} else {
one
Expand Down
11 changes: 0 additions & 11 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use super::{
instruction::{ArrayOffset, ConstrainError, InstructionId, Intrinsic},
types::NumericType,
},
opt::pure::FunctionPurities,
ssa_gen::Ssa,
};

Expand All @@ -49,7 +48,6 @@ pub struct FunctionBuilder {
pub simplify: bool,

globals: Arc<GlobalsGraph>,
purities: Arc<FunctionPurities>,
}

impl FunctionBuilder {
Expand All @@ -67,7 +65,6 @@ impl FunctionBuilder {
error_types: BTreeMap::default(),
simplify: true,
globals: Default::default(),
purities: Default::default(),
}
}

Expand All @@ -76,9 +73,7 @@ impl FunctionBuilder {
pub fn from_existing(function: &Function, function_id: FunctionId) -> Self {
let mut this = Self::new(function.name().to_owned(), function_id);
this.set_globals(function.dfg.globals.clone());
this.purities = function.dfg.function_purities.clone();
this.current_function.set_runtime(function.runtime());
this.current_function.dfg.set_function_purities(this.purities.clone());
this
}

Expand Down Expand Up @@ -107,11 +102,6 @@ impl FunctionBuilder {
self.current_function.set_globals(self.globals.clone());
}

pub fn set_purities(&mut self, purities: Arc<FunctionPurities>) {
self.purities = purities.clone();
self.current_function.dfg.set_function_purities(purities);
}

/// Finish the current function and create a new function.
///
/// A FunctionBuilder can always only work on one function at a time, so care
Expand All @@ -134,7 +124,6 @@ impl FunctionBuilder {
self.current_function.dfg.call_stack_data.get_or_insert_locations(&call_stack);
self.finished_functions.push(old_function);

self.current_function.dfg.set_function_purities(self.purities.clone());
self.apply_globals();
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/src/ssa/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,8 +467,9 @@ impl<'ssa, W: Write> Interpreter<'ssa, W> {

match current_function.runtime() {
RuntimeType::Acir(_) => {
let purities = &self.ssa.function_purities;
self.side_effects_enabled
|| !instruction.requires_acir_gen_predicate(&current_function.dfg)
|| !instruction.requires_acir_gen_predicate(&current_function.dfg, purities)
}
RuntimeType::Brillig(_) => true,
}
Expand Down
16 changes: 1 addition & 15 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use std::{borrow::Cow, sync::Arc};

use crate::ssa::{
function_builder::data_bus::DataBus,
opt::pure::{FunctionPurities, Purity},
};
use crate::ssa::function_builder::data_bus::DataBus;

use super::{
basic_block::{BasicBlock, BasicBlockId},
Expand Down Expand Up @@ -104,9 +101,6 @@ pub(crate) struct DataFlowGraph {
pub(crate) data_bus: DataBus,

pub(crate) globals: Arc<GlobalsGraph>,

#[serde(skip)]
pub(crate) function_purities: Arc<FunctionPurities>,
}

/// The GlobalsGraph contains the actual global data.
Expand Down Expand Up @@ -768,14 +762,6 @@ impl DataFlowGraph {
_ => None,
}
}

pub(crate) fn set_function_purities(&mut self, purities: Arc<FunctionPurities>) {
self.function_purities = purities;
}

pub(crate) fn purity_of(&self, function: FunctionId) -> Option<Purity> {
self.function_purities.get(&function).copied()
}
}

impl std::ops::Index<InstructionId> for DataFlowGraph {
Expand Down
1 change: 0 additions & 1 deletion compiler/noirc_evaluator/src/ssa/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ impl Function {
let mut new_function = Function::new(another.name.clone(), id);
new_function.set_runtime(another.runtime());
new_function.set_globals(another.dfg.globals.clone());
new_function.dfg.set_function_purities(another.dfg.function_purities.clone());
new_function
}

Expand Down
18 changes: 13 additions & 5 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use fxhash::FxHasher64;
use iter_extended::vecmap;
use noirc_frontend::hir_def::types::Type as HirType;

use crate::ssa::opt::pure::Purity;
use crate::ssa::opt::pure::{FunctionPurities, Purity};

use super::{
basic_block::BasicBlockId,
Expand Down Expand Up @@ -377,7 +377,11 @@ impl Instruction {
}

/// If true the instruction will depend on `enable_side_effects` context during acir-gen.
pub(crate) fn requires_acir_gen_predicate(&self, dfg: &DataFlowGraph) -> bool {
pub(crate) fn requires_acir_gen_predicate(
&self,
dfg: &DataFlowGraph,
purities: &FunctionPurities,
) -> bool {
match self {
Instruction::Binary(binary) => binary.requires_acir_gen_predicate(dfg),

Expand All @@ -389,7 +393,7 @@ impl Instruction {
Instruction::EnableSideEffectsIf { .. } | Instruction::ArraySet { .. } => true,

Instruction::Call { func, .. } => match dfg[*func] {
Value::Function(id) => !matches!(dfg.purity_of(id), Some(Purity::Pure)),
Value::Function(id) => !matches!(purities.get(&id), Some(Purity::Pure)),
Value::Intrinsic(intrinsic) => {
// These utilize `noirc_evaluator::acir::Context::get_flattened_index` internally
// which uses the side effects predicate.
Expand All @@ -415,7 +419,11 @@ impl Instruction {
}

/// Indicates if the instruction has a side effect, ie. it can fail, or it interacts with memory.
pub(crate) fn has_side_effects(&self, dfg: &DataFlowGraph) -> bool {
pub(crate) fn has_side_effects(
&self,
dfg: &DataFlowGraph,
purities: &FunctionPurities,
) -> bool {
use Instruction::*;

match self {
Expand All @@ -431,7 +439,7 @@ impl Instruction {
Value::Intrinsic(intrinsic) => intrinsic.has_side_effects(),
// Functions known to be pure have no side effects.
// `PureWithPredicates` functions may still have side effects.
Value::Function(function) => dfg.purity_of(function) != Some(Purity::Pure),
Value::Function(function) => purities.get(&function).copied() != Some(Purity::Pure),
_ => true, // Be conservative and assume other functions can have side effects.
},

Expand Down
8 changes: 5 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::ssa::{
instruction::ArrayOffset,
types::{NumericType, Type},
},
opt::pure::FunctionPurities,
};

use super::{
Expand Down Expand Up @@ -68,7 +69,7 @@ impl Display for Printer<'_> {
}

for function in self.ssa.functions.values() {
display_function(function, self.fm, f)?;
display_function(function, self.fm, &self.ssa.function_purities, f)?;
writeln!(f)?;
}
Ok(())
Expand All @@ -77,17 +78,18 @@ impl Display for Printer<'_> {

impl Display for Function {
fn fmt(&self, f: &mut Formatter<'_>) -> Result {
display_function(self, None, f)
display_function(self, None, &FunctionPurities::default(), f)
}
}

/// Helper function for Function's Display impl to pretty-print the function with the given formatter.
fn display_function(
function: &Function,
files: Option<&fm::FileManager>,
purities: &FunctionPurities,
f: &mut Formatter,
) -> Result {
if let Some(purity) = function.dfg.purity_of(function.id()) {
if let Some(purity) = purities.get(&function.id()).copied() {
writeln!(f, "{} {purity} fn {} {} {{", function.runtime(), function.name(), function.id())?;
} else {
writeln!(f, "{} fn {} {} {{", function.runtime(), function.name(), function.id())?;
Expand Down
43 changes: 27 additions & 16 deletions compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use crate::{
types::{NumericType, Type},
value::{Value, ValueId, ValueMapping},
},
opt::pure::Purity,
opt::pure::{FunctionPurities, Purity},
ssa_gen::Ssa,
},
};
Expand All @@ -57,7 +57,7 @@ impl Ssa {
#[tracing::instrument(level = "trace", skip(self))]
pub(crate) fn fold_constants(mut self) -> Ssa {
for function in self.functions.values_mut() {
function.constant_fold(false, None);
function.constant_fold(false, None, &self.function_purities);
}
self
}
Expand All @@ -70,7 +70,7 @@ impl Ssa {
#[tracing::instrument(level = "trace", skip(self))]
pub(crate) fn fold_constants_using_constraints(mut self) -> Ssa {
for function in self.functions.values_mut() {
function.constant_fold(true, None);
function.constant_fold(true, None, &self.function_purities);
}
self
}
Expand All @@ -96,7 +96,7 @@ impl Ssa {
if function.dfg.runtime().is_brillig() {
continue;
}
function.constant_fold(false, brillig_info);
function.constant_fold(false, brillig_info, &self.function_purities);
}

self
Expand All @@ -106,12 +106,13 @@ impl Ssa {
impl Function {
/// The structure of this pass is simple:
/// Go through each block and re-insert all instructions.
pub(crate) fn constant_fold(
fn constant_fold(
&mut self,
use_constraint_info: bool,
brillig_info: Option<BrilligInfo>,
purities: &FunctionPurities,
) {
let mut context = Context::new(use_constraint_info, brillig_info);
let mut context = Context::new(use_constraint_info, brillig_info, purities);
let mut dom = DominatorTree::with_function(self);
context.block_queue.push_back(self.entry_block());

Expand All @@ -127,6 +128,7 @@ impl Function {
}

struct Context<'a> {
purities: &'a FunctionPurities,
use_constraint_info: bool,
brillig_info: Option<BrilligInfo<'a>>,
/// Maps pre-folded ValueIds to the new ValueIds obtained by re-inserting the instruction.
Expand Down Expand Up @@ -216,9 +218,14 @@ struct ResultCache {
result: Option<(BasicBlockId, Vec<ValueId>)>,
}

impl<'brillig> Context<'brillig> {
fn new(use_constraint_info: bool, brillig_info: Option<BrilligInfo<'brillig>>) -> Self {
impl<'a> Context<'a> {
fn new(
use_constraint_info: bool,
brillig_info: Option<BrilligInfo<'a>>,
purities: &'a FunctionPurities,
) -> Self {
Self {
purities,
use_constraint_info,
brillig_info,
visited_blocks: Default::default(),
Expand Down Expand Up @@ -508,13 +515,13 @@ impl<'brillig> Context<'brillig> {
// 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.
let can_be_deduplicated =
can_be_deduplicated(&instruction, function, self.use_constraint_info);
can_be_deduplicated(&instruction, function, self.use_constraint_info, self.purities);

// We also allow deduplicating MakeArray instructions that we have tracked which haven't
// been mutated.
if can_be_deduplicated || matches!(instruction, Instruction::MakeArray { .. }) {
let use_predicate =
self.use_constraint_info && instruction.requires_acir_gen_predicate(&function.dfg);
let use_predicate = self.use_constraint_info
&& instruction.requires_acir_gen_predicate(&function.dfg, self.purities);
let predicate = use_predicate.then_some(side_effects_enabled_var);

// If we see this make_array again, we can reuse the current result.
Expand Down Expand Up @@ -553,10 +560,12 @@ impl<'brillig> Context<'brillig> {
block: BasicBlockId,
) -> Option<CacheResult> {
let results_for_instruction = self.cached_instruction_results.get(instruction)?;
let predicate = self.use_constraint_info && instruction.requires_acir_gen_predicate(dfg);
let predicate =
self.use_constraint_info && instruction.requires_acir_gen_predicate(dfg, self.purities);
let predicate = predicate.then_some(side_effects_enabled_var);

results_for_instruction.get(&predicate)?.get(block, dom, instruction.has_side_effects(dfg))
let results = results_for_instruction.get(&predicate)?;
results.get(block, dom, instruction.has_side_effects(dfg, self.purities))
}

/// Checks if the given instruction is a call to a brillig function with all constant arguments.
Expand Down Expand Up @@ -778,7 +787,7 @@ impl<'brillig> Context<'brillig> {
Call { arguments, func } if function.runtime().is_brillig() => {
// If we pass a value to a function, it might get modified, making it unsafe for reuse after the call.
let Value::Function(func_id) = &function.dfg[*func] else { return };
if matches!(function.dfg.purity_of(*func_id), None | Some(Purity::Impure)) {
if matches!(self.purities.get(func_id).copied(), None | Some(Purity::Impure)) {
// Arrays passed to functions might be mutated by them if there are no `inc_rc` instructions
// placed *before* the call to protect them. Currently we don't track the ref count in this
// context, so be conservative and do not reuse any array shared with a callee.
Expand Down Expand Up @@ -909,6 +918,7 @@ pub(crate) fn can_be_deduplicated(
instruction: &Instruction,
function: &Function,
deduplicate_with_predicate: bool,
purities: &FunctionPurities,
) -> bool {
use Instruction::*;

Expand All @@ -924,7 +934,7 @@ pub(crate) fn can_be_deduplicated(
Call { func, .. } => {
let purity = match function.dfg[*func] {
Value::Intrinsic(intrinsic) => Some(intrinsic.purity()),
Value::Function(id) => function.dfg.purity_of(id),
Value::Function(id) => purities.get(&id).copied(),
_ => None,
};
match purity {
Expand Down Expand Up @@ -956,7 +966,8 @@ pub(crate) fn can_be_deduplicated(
// with one that was disabled. See
// https://github.com/noir-lang/noir/pull/4716#issuecomment-2047846328.
Binary(_) | ArrayGet { .. } | ArraySet { .. } => {
deduplicate_with_predicate || !instruction.requires_acir_gen_predicate(&function.dfg)
deduplicate_with_predicate
|| !instruction.requires_acir_gen_predicate(&function.dfg, purities)
}
}
}
Expand Down
Loading
Loading