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(ssa): Immediately simplify away RefCount instructions in ACIR functions #6893

Merged
merged 17 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from 3 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
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,8 @@ impl<'a> Context<'a> {
unreachable!("Expected all load instructions to be removed before acir_gen")
}
Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. } => {
// Do nothing. Only Brillig needs to worry about reference counted arrays
// Only Brillig needs to worry about reference counted arrays
unreachable!("Expected all Rc instructions to be removed before acir_gen")
}
Instruction::RangeCheck { value, max_bit_size, assert_message } => {
let acir_var = self.convert_numeric_value(*value, dfg)?;
Expand Down
74 changes: 68 additions & 6 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use super::{
basic_block::{BasicBlock, BasicBlockId},
call_stack::{CallStack, CallStackHelper, CallStackId},
function::FunctionId,
function::{FunctionId, RuntimeType},
instruction::{
Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction,
},
Expand All @@ -27,8 +27,23 @@
/// owning most data in a function and handing out Ids to this data that can be
/// shared without worrying about ownership.
#[serde_as]
#[derive(Debug, Default, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Serialize, Deserialize)]
pub(crate) struct DataFlowGraph {
/// Runtime of the [Function] that owns this [DataFlowGraph].
/// This might change during the `runtime_separation` pass where
/// ACIR functions are cloned as Brillig functions.
runtime: RuntimeType,

/// Indicate whether we are past the runtime separation step.
/// Before this step the DFG accepts any instruction, because
/// an ACIR function might be cloned as a Brillig function.
/// After the separation, we can instantly remove instructions
/// that would just be ignored by the runtime.
///
/// TODO(#6841): This would not be necessary if the SSA was
/// already generated for a specific runtime.
is_runtime_separated: bool,

/// All of the instructions in a function
instructions: DenseMap<Instruction>,

Expand All @@ -42,7 +57,7 @@
/// Call instructions require the func signature, but
/// other instructions may need some more reading on my part
#[serde_as(as = "HashMap<DisplayFromStr, _>")]
results: HashMap<InstructionId, smallvec::SmallVec<[ValueId; 1]>>,

Check warning on line 60 in compiler/noirc_evaluator/src/ssa/ir/dfg.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (smallvec)

/// Storage for all of the values defined in this
/// function.
Expand Down Expand Up @@ -101,6 +116,42 @@
}

impl DataFlowGraph {
pub(crate) fn new(runtime: RuntimeType) -> Self {
Self {
runtime,
is_runtime_separated: false,
instructions: Default::default(),
results: Default::default(),
values: Default::default(),
constants: Default::default(),
functions: Default::default(),
intrinsics: Default::default(),
foreign_functions: Default::default(),
blocks: Default::default(),
replaced_value_ids: Default::default(),
locations: Default::default(),
call_stack_data: Default::default(),
data_bus: Default::default(),
aakoshh marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// Runtime type of the function.
pub(crate) fn runtime(&self) -> RuntimeType {
self.runtime
}

/// Set runtime type of the function.
pub(crate) fn set_runtime(&mut self, runtime: RuntimeType) {
self.runtime = runtime;
}

/// Mark the runtime as separated. After this we can drop
/// instructions that would be ignored by the runtime,
/// instead of inserting them and carrying them to the end.
pub(crate) fn set_runtime_separated(&mut self) {
self.is_runtime_separated = true;
}

/// Creates a new basic block with no parameters.
/// After being created, the block is unreachable in the current function
/// until another block is made to jump to it.
Expand Down Expand Up @@ -165,6 +216,11 @@
id
}

/// Check if the function runtime would simply ignore this instruction.
pub(crate) fn is_handled_by_runtime(&self, instruction: &Instruction) -> bool {
!(self.runtime().is_acir() && instruction.is_brillig_only())
}

fn insert_instruction_without_simplification(
&mut self,
instruction_data: Instruction,
Expand All @@ -178,7 +234,7 @@
id
}

pub(crate) fn insert_instruction_and_results_without_simplification(
fn insert_instruction_and_results_without_simplification(
&mut self,
instruction_data: Instruction,
block: BasicBlockId,
Expand All @@ -195,14 +251,18 @@
InsertInstructionResult::Results(id, self.instruction_results(id))
}

/// Inserts a new instruction at the end of the given block and returns its results
/// Simplifies a new instruction and inserts it at the end of the given block and returns its results.
/// If the instruction is not handled by the current runtime, `InstructionRemoved` is returned.
pub(crate) fn insert_instruction_and_results(
&mut self,
instruction: Instruction,
block: BasicBlockId,
ctrl_typevars: Option<Vec<Type>>,
call_stack: CallStackId,
) -> InsertInstructionResult {
if self.is_runtime_separated && !self.is_handled_by_runtime(&instruction) {
return InsertInstructionResult::InstructionRemoved;
}
match instruction.simplify(self, block, ctrl_typevars.clone(), call_stack) {
SimplifyResult::SimplifiedTo(simplification) => {
InsertInstructionResult::SimplifiedTo(simplification)
Expand Down Expand Up @@ -345,7 +405,7 @@
instruction: InstructionId,
ctrl_typevars: Option<Vec<Type>>,
) {
let mut results = smallvec::SmallVec::new();

Check warning on line 408 in compiler/noirc_evaluator/src/ssa/ir/dfg.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (smallvec)
let mut position = 0;
self.for_each_instruction_result_type(instruction, ctrl_typevars, |this, typ| {
let result = this.values.insert(Value::Instruction { typ, position, instruction });
Expand Down Expand Up @@ -694,12 +754,14 @@

#[cfg(test)]
mod tests {
use noirc_frontend::monomorphization::ast::InlineType;

use super::DataFlowGraph;
use crate::ssa::ir::{instruction::Instruction, types::Type};
use crate::ssa::ir::{dfg::RuntimeType, instruction::Instruction, types::Type};

#[test]
fn make_instruction() {
let mut dfg = DataFlowGraph::default();
let mut dfg = DataFlowGraph::new(RuntimeType::Acir(InlineType::Inline));
let ins = Instruction::Allocate;
let ins_id = dfg.make_instruction(ins, Some(vec![Type::field()]));

Expand Down
49 changes: 39 additions & 10 deletions compiler/noirc_evaluator/src/ssa/ir/function.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use std::collections::BTreeSet;
use std::collections::{BTreeSet, HashSet};

use iter_extended::vecmap;
use noirc_frontend::monomorphization::ast::InlineType;
use serde::{Deserialize, Serialize};

use super::basic_block::BasicBlockId;
use super::dfg::DataFlowGraph;
use super::instruction::TerminatorInstruction;
use super::instruction::{Instruction, TerminatorInstruction};
use super::map::Id;
use super::types::Type;
use super::value::ValueId;
Expand Down Expand Up @@ -72,8 +72,6 @@ pub(crate) struct Function {

id: FunctionId,

runtime: RuntimeType,

/// The DataFlowGraph holds the majority of data pertaining to the function
/// including its blocks, instructions, and values.
pub(crate) dfg: DataFlowGraph,
Expand All @@ -84,22 +82,22 @@ impl Function {
///
/// Note that any parameters or attributes of the function must be manually added later.
pub(crate) fn new(name: String, id: FunctionId) -> Self {
let mut dfg = DataFlowGraph::default();
let mut dfg = DataFlowGraph::new(RuntimeType::Acir(InlineType::default()));
let entry_block = dfg.make_block();
Self { name, id, entry_block, dfg, runtime: RuntimeType::Acir(InlineType::default()) }
Self { name, id, entry_block, dfg }
}

/// Creates a new function as a clone of the one passed in with the passed in id.
pub(crate) fn clone_with_id(id: FunctionId, another: &Function) -> Self {
let dfg = another.dfg.clone();
let entry_block = another.entry_block;
Self { name: another.name.clone(), id, entry_block, dfg, runtime: another.runtime }
Self { name: another.name.clone(), id, entry_block, dfg }
}

/// Takes the signature (function name & runtime) from a function but does not copy the body.
pub(crate) fn clone_signature(id: FunctionId, another: &Function) -> Self {
let mut new_function = Function::new(another.name.clone(), id);
new_function.runtime = another.runtime;
new_function.set_runtime(another.runtime());
new_function
}

Expand All @@ -116,12 +114,12 @@ impl Function {

/// Runtime type of the function.
pub(crate) fn runtime(&self) -> RuntimeType {
self.runtime
self.dfg.runtime()
}

/// Set runtime type of the function.
pub(crate) fn set_runtime(&mut self, runtime: RuntimeType) {
self.runtime = runtime;
self.dfg.set_runtime(runtime);
}

pub(crate) fn is_no_predicates(&self) -> bool {
Expand Down Expand Up @@ -195,6 +193,37 @@ impl Function {

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

/// Remove instructions from all the reachable blocks in a function based on a predicate,
/// keeping the ones where the predicate returns `true`.
pub(crate) fn retain_instructions(&mut self, pred: impl Fn(&Instruction) -> bool) {
for block_id in self.reachable_blocks() {
let mut to_remove = HashSet::new();
let block = &self.dfg[block_id];
for instruction_id in block.instructions() {
let instruction = &self.dfg[*instruction_id];
if !pred(instruction) {
to_remove.insert(*instruction_id);
}
}
if !to_remove.is_empty() {
let block = &mut self.dfg[block_id];
block.instructions_mut().retain(|id| !to_remove.contains(id));
}
}
}

/// Remove all instructions that aren't handled by the runtime, which is now
/// considered final. Seal the [DataFlowGraph] so it instantly simplifies
/// away instructions that the runtime would have to ignore.
///
/// TODO(#6841): Remove once the SSA generated is for a specific runtime already.
pub(crate) fn separate_runtime(&mut self) {
if self.runtime().is_acir() {
self.retain_instructions(|i| !i.is_brillig_only());
}
self.dfg.set_runtime_separated();
}
}

impl Clone for Function {
Expand Down
6 changes: 6 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@
// These can fail.
Constrain(..) | RangeCheck { .. } => true,

// This should never be side-effectful

Check warning on line 401 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (effectful)
MakeArray { .. } => false,

// Some binary math can overflow or underflow
Expand Down Expand Up @@ -1037,6 +1037,12 @@
Instruction::MakeArray { .. } => None,
}
}

/// Some instructions are only to be used in Brillig and should be eliminated
/// after runtime separation, never to be be reintroduced in an ACIR runtime.
pub(crate) fn is_brillig_only(&self) -> bool {
matches!(self, Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. })
}
}

/// Given a chain of operations like:
Expand Down
8 changes: 4 additions & 4 deletions compiler/noirc_evaluator/src/ssa/ir/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ pub(crate) type ValueId = Id<Value>;
pub(crate) enum Value {
/// This value was created due to an instruction
///
/// instruction -- This is the instruction which defined it
/// typ -- This is the `Type` of the instruction
/// position -- Returns the position in the results
/// vector that this `Value` is located.
/// * `instruction`: This is the instruction which defined it
/// * `typ`: This is the `Type` of the instruction
/// * `position`: Returns the position in the results vector that this `Value` is located.
///
/// Example, if you add two numbers together, then the resulting
/// value would have position `0`, the typ would be the type
/// of the operands, and the instruction would map to an add instruction.
Expand Down
14 changes: 9 additions & 5 deletions compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@

// Some functions might be unreachable now (for example an acir function only called from brillig)
prune_unreachable_functions(ssa);

// Finally we can mark each function as having their runtimes be separated.

Check warning on line 58 in compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (runtimes)
for func in ssa.functions.values_mut() {
func.separate_runtime();
}
}

fn collect_acir_functions_called_from_brillig(
Expand All @@ -64,13 +69,12 @@
processed_functions: &mut HashSet<(/* within_brillig */ bool, FunctionId)>,
) {
// Processed functions needs the within brillig flag, since it is possible to call the same function from both brillig and acir
if processed_functions.contains(&(within_brillig, current_func_id)) {
if !processed_functions.insert((within_brillig, current_func_id)) {
return;
}
processed_functions.insert((within_brillig, current_func_id));

let func = &ssa.functions[&current_func_id];
if matches!(func.runtime(), RuntimeType::Brillig(_)) {
if func.runtime().is_brillig() {
within_brillig = true;
}

Expand All @@ -79,7 +83,7 @@
if within_brillig {
for called_func_id in called_functions.iter() {
let called_func = &ssa.functions[called_func_id];
if matches!(called_func.runtime(), RuntimeType::Acir(_)) {
if called_func.runtime().is_acir() {
self.acir_functions_called_from_brillig.insert(*called_func_id);
}
}
Expand Down Expand Up @@ -110,7 +114,7 @@

fn replace_calls_to_mapped_functions(&self, ssa: &mut Ssa) {
for (_function_id, func) in ssa.functions.iter_mut() {
if matches!(func.runtime(), RuntimeType::Brillig(_)) {
if func.runtime().is_brillig() {
for called_func_value_id in called_functions_values(func).iter() {
let Value::Function(called_func_id) = &func.dfg[*called_func_value_id] else {
unreachable!("Value should be a function")
Expand Down Expand Up @@ -248,10 +252,10 @@

fn find_func_by_name<'ssa>(
ssa: &'ssa Ssa,
funcs: &BTreeSet<FunctionId>,

Check warning on line 255 in compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (funcs)
name: &str,
) -> &'ssa Function {
funcs

Check warning on line 258 in compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (funcs)
.iter()
.find_map(|id| {
let func = ssa.functions.get(id).unwrap();
Expand Down
Loading