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: manage call stacks using a tree #6791

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
8 changes: 5 additions & 3 deletions compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ impl<'a> Context<'a> {
brillig: &Brillig,
) -> Result<Vec<SsaReport>, RuntimeError> {
let instruction = &dfg[instruction_id];
self.acir_context.set_call_stack(dfg.get_call_stack(instruction_id));
self.acir_context.set_call_stack(dfg.get_instruction_call_stack(instruction_id));
let mut warnings = Vec::new();
match instruction {
Instruction::Binary(binary) => {
Expand Down Expand Up @@ -1822,7 +1822,7 @@ impl<'a> Context<'a> {
) -> Result<(Vec<AcirVar>, Vec<SsaReport>), RuntimeError> {
let (return_values, call_stack) = match terminator {
TerminatorInstruction::Return { return_values, call_stack } => {
(return_values, call_stack.clone())
(return_values, *call_stack)
}
// TODO(https://github.com/noir-lang/noir/issues/4616): Enable recursion on foldable/non-inlined ACIR functions
_ => unreachable!("ICE: Program must have a singular return"),
Expand All @@ -1841,6 +1841,7 @@ impl<'a> Context<'a> {
}
}

let call_stack = dfg.get_call_stack(call_stack);
let warnings = if has_constant_return {
vec![SsaReport::Warning(InternalWarning::ReturnConstant { call_stack })]
} else {
Expand Down Expand Up @@ -2932,7 +2933,8 @@ mod test {
// Set a call stack for testing whether `brillig_locations` in the `GeneratedAcir` was accurately set.
let mut stack = CallStack::unit(Location::dummy());
stack.push_back(Location::dummy());
builder.set_call_stack(stack);
let call_stack = builder.current_function.dfg.get_or_insert_locations(stack);
builder.set_call_stack(call_stack);

let foo_v0 = builder.add_parameter(Type::field());
let foo_v1 = builder.add_parameter(Type::field());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ impl<'block> BrilligBlock<'block> {
/// Converts an SSA instruction into a sequence of Brillig opcodes.
fn convert_ssa_instruction(&mut self, instruction_id: InstructionId, dfg: &DataFlowGraph) {
let instruction = &dfg[instruction_id];
self.brillig_context.set_call_stack(dfg.get_call_stack(instruction_id));
self.brillig_context.set_call_stack(dfg.get_instruction_call_stack(instruction_id));

self.initialize_constants(
&self.function_context.constant_allocation.allocated_at_location(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ impl DependencyContext {
.keys()
.map(|brillig_call| {
SsaReport::Bug(InternalBug::UncheckedBrilligCall {
call_stack: function.dfg.get_call_stack(*brillig_call),
call_stack: function.dfg.get_instruction_call_stack(*brillig_call),
})
})
.collect();
Expand Down Expand Up @@ -516,7 +516,7 @@ impl Context {
// There is a value not in the set, which means that the inputs/outputs of this call have not been properly constrained
if unused_inputs {
warnings.push(SsaReport::Bug(InternalBug::IndependentSubgraph {
call_stack: function.dfg.get_call_stack(
call_stack: function.dfg.get_instruction_call_stack(
self.brillig_return_to_instruction_id[&brillig_output_in_set],
),
}));
Expand Down
26 changes: 14 additions & 12 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::ssa::ir::{
use super::{
ir::{
basic_block::BasicBlock,
dfg::{CallStack, InsertInstructionResult},
dfg::{CallStack, CallStackId, InsertInstructionResult},
function::RuntimeType,
instruction::{ConstrainError, InstructionId, Intrinsic},
types::NumericType,
Expand All @@ -34,10 +34,10 @@ use super::{
/// Contrary to the name, this struct has the capacity to build as many
/// functions as needed, although it is limited to one function at a time.
pub(crate) struct FunctionBuilder {
pub(super) current_function: Function,
pub(crate) current_function: Function,
current_block: BasicBlockId,
finished_functions: Vec<Function>,
call_stack: CallStack,
call_stack: CallStackId,
error_types: BTreeMap<ErrorSelector, HirType>,
}

Expand All @@ -53,7 +53,7 @@ impl FunctionBuilder {
current_block: new_function.entry_block(),
current_function: new_function,
finished_functions: Vec::new(),
call_stack: CallStack::new(),
call_stack: CallStackId::root(),
error_types: BTreeMap::default(),
}
}
Expand All @@ -78,11 +78,13 @@ impl FunctionBuilder {
function_id: FunctionId,
runtime_type: RuntimeType,
) {
let call_stack = self.current_function.dfg.get_call_stack(self.call_stack);
let mut new_function = Function::new(name, function_id);
new_function.set_runtime(runtime_type);
self.current_block = new_function.entry_block();

let old_function = std::mem::replace(&mut self.current_function, new_function);
// Copy the call stack to the new function
self.call_stack = self.current_function.dfg.get_or_insert_locations(call_stack);
self.finished_functions.push(old_function);
}

Expand Down Expand Up @@ -171,7 +173,7 @@ impl FunctionBuilder {
instruction,
block,
ctrl_typevars,
self.call_stack.clone(),
self.call_stack,
)
}

Expand All @@ -196,17 +198,17 @@ impl FunctionBuilder {
}

pub(crate) fn set_location(&mut self, location: Location) -> &mut FunctionBuilder {
self.call_stack = CallStack::unit(location);
self.call_stack = self.current_function.dfg.add_location_to_root(location);
self
}

pub(crate) fn set_call_stack(&mut self, call_stack: CallStack) -> &mut FunctionBuilder {
pub(crate) fn set_call_stack(&mut self, call_stack: CallStackId) -> &mut FunctionBuilder {
self.call_stack = call_stack;
self
}

pub(crate) fn get_call_stack(&self) -> CallStack {
self.call_stack.clone()
self.current_function.dfg.get_call_stack(self.call_stack)
}

/// Insert a Load instruction at the end of the current block, loading from the given address
Expand Down Expand Up @@ -378,7 +380,7 @@ impl FunctionBuilder {
destination: BasicBlockId,
arguments: Vec<ValueId>,
) {
let call_stack = self.call_stack.clone();
let call_stack = self.call_stack;
self.terminate_block_with(TerminatorInstruction::Jmp {
destination,
arguments,
Expand All @@ -394,7 +396,7 @@ impl FunctionBuilder {
then_destination: BasicBlockId,
else_destination: BasicBlockId,
) {
let call_stack = self.call_stack.clone();
let call_stack = self.call_stack;
self.terminate_block_with(TerminatorInstruction::JmpIf {
condition,
then_destination,
Expand All @@ -405,7 +407,7 @@ impl FunctionBuilder {

/// Terminate the current block with a return instruction
pub(crate) fn terminate_with_return(&mut self, return_values: Vec<ValueId>) {
let call_stack = self.call_stack.clone();
let call_stack = self.call_stack;
self.terminate_block_with(TerminatorInstruction::Return { return_values, call_stack });
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/ssa/ir/basic_block.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::{
dfg::CallStack,
dfg::CallStackId,
instruction::{InstructionId, TerminatorInstruction},
map::Id,
value::ValueId,
Expand Down Expand Up @@ -123,7 +123,7 @@ impl BasicBlock {
terminator,
TerminatorInstruction::Return {
return_values: Vec::new(),
call_stack: CallStack::new(),
call_stack: CallStackId::root(),
},
)
}
Expand Down
16 changes: 8 additions & 8 deletions compiler/noirc_evaluator/src/ssa/ir/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl ControlFlowGraph {
#[cfg(test)]
mod tests {
use crate::ssa::ir::{
dfg::CallStack, instruction::TerminatorInstruction, map::Id, types::Type,
dfg::CallStackId, instruction::TerminatorInstruction, map::Id, types::Type,
};

use super::{super::function::Function, ControlFlowGraph};
Expand All @@ -140,7 +140,7 @@ mod tests {
let block_id = func.entry_block();
func.dfg[block_id].set_terminator(TerminatorInstruction::Return {
return_values: vec![],
call_stack: CallStack::new(),
call_stack: CallStackId::root(),
});

ControlFlowGraph::with_function(&func);
Expand Down Expand Up @@ -168,17 +168,17 @@ mod tests {
condition: cond,
then_destination: block2_id,
else_destination: block1_id,
call_stack: CallStack::new(),
call_stack: CallStackId::root(),
});
func.dfg[block1_id].set_terminator(TerminatorInstruction::JmpIf {
condition: cond,
then_destination: block1_id,
else_destination: block2_id,
call_stack: CallStack::new(),
call_stack: CallStackId::root(),
});
func.dfg[block2_id].set_terminator(TerminatorInstruction::Return {
return_values: vec![],
call_stack: CallStack::new(),
call_stack: CallStackId::root(),
});

let mut cfg = ControlFlowGraph::with_function(&func);
Expand Down Expand Up @@ -226,18 +226,18 @@ mod tests {
let ret_block_id = func.dfg.make_block();
func.dfg[ret_block_id].set_terminator(TerminatorInstruction::Return {
return_values: vec![],
call_stack: CallStack::new(),
call_stack: CallStackId::root(),
});
func.dfg[block2_id].set_terminator(TerminatorInstruction::Jmp {
destination: ret_block_id,
arguments: vec![],
call_stack: CallStack::new(),
call_stack: CallStackId::root(),
});
func.dfg[block0_id].set_terminator(TerminatorInstruction::JmpIf {
condition: cond,
then_destination: block1_id,
else_destination: ret_block_id,
call_stack: CallStack::new(),
call_stack: CallStackId::root(),
});

// Recompute new and changed blocks
Expand Down
Loading
Loading