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 globals in monomorphization and SSA gen #6985

Merged
merged 27 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c7d8009
cleanup global generation with GlobalsBuilder
vezenovm Jan 8, 2025
4b19a7b
all tests paassing
vezenovm Jan 8, 2025
c8e08d3
cargo fmt
vezenovm Jan 8, 2025
8a24746
fmt and clippy
vezenovm Jan 8, 2025
e3358e3
some comments for context
vezenovm Jan 8, 2025
c3948fe
nargo fmt
vezenovm Jan 8, 2025
328004e
merge master and conflicts w/ printer
vezenovm Jan 8, 2025
40c50fc
remove type from Value::Global and cleanup
vezenovm Jan 8, 2025
7744179
bring back global type
vezenovm Jan 8, 2025
68ddd14
Update compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
vezenovm Jan 8, 2025
6823019
only print globals when printing ssa if globals exist
vezenovm Jan 8, 2025
73ceea5
Merge remote-tracking branch 'origin/mv/ssa-globals-1' into mv/ssa-gl…
vezenovm Jan 8, 2025
cbe377e
make an eval method
vezenovm Jan 8, 2025
2d750fb
Update compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
vezenovm Jan 8, 2025
faeb30f
Update compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
vezenovm Jan 8, 2025
2322cb6
Update test_programs/execution_success/global_var_regression_simple/s…
vezenovm Jan 8, 2025
08a2aba
Merge branch 'master' into mv/ssa-globals-1
vezenovm Jan 8, 2025
fa6d9cf
cargo mft
vezenovm Jan 8, 2025
200964f
Merge branch 'master' into mv/ssa-globals-1
vezenovm Jan 8, 2025
f98c713
merge conflicts w/ master
vezenovm Jan 9, 2025
ad19fc3
Merge branch 'master' into mv/ssa-globals-1
vezenovm Jan 9, 2025
bb5a9ea
Update compiler/noirc_frontend/src/hir/comptime/value.rs
vezenovm Jan 9, 2025
74a7358
do ot duplicate codegen
vezenovm Jan 9, 2025
df5eef0
Merge branch 'master' into mv/ssa-globals-1
vezenovm Jan 9, 2025
584cc92
Merge branch 'master' into mv/ssa-globals-1
vezenovm Jan 10, 2025
8ff2934
switch to Function from DataFlowGraph to represent globals
vezenovm Jan 10, 2025
aae17de
remove unnecessary lifetime on InlineContext
vezenovm Jan 10, 2025
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
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1892,6 +1892,9 @@ impl<'a> Context<'a> {
Value::Instruction { .. } | Value::Param { .. } => {
unreachable!("ICE: Should have been in cache {value_id} {value:?}")
}
Value::Global(_) => {
unreachable!("ICE: All globals should have been inlined");
}
};
self.ssa_values.insert(value_id, acir_value.clone());
acir_value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,10 @@ impl<'block> BrilligBlock<'block> {
}
}
}
Value::Instruction { .. } | Value::Param { .. } | Value::NumericConstant { .. } => {
Value::Instruction { .. }
| Value::Param { .. }
| Value::NumericConstant { .. }
| Value::Global(_) => {
unreachable!("unsupported function call type {:?}", dfg[*func])
}
},
Expand Down Expand Up @@ -1557,6 +1560,9 @@ impl<'block> BrilligBlock<'block> {
let value = &dfg[value_id];

match value {
Value::Global(_) => {
unreachable!("ICE: All globals should have been inlined");
}
Value::Param { .. } | Value::Instruction { .. } => {
// All block parameters and instruction results should have already been
// converted to registers so we fetch from the cache.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ pub(crate) fn collect_variables_of_value(
let value = &dfg[value_id];

match value {
Value::Instruction { .. } | Value::Param { .. } | Value::NumericConstant { .. } => {
Some(value_id)
}
Value::Instruction { .. }
| Value::Param { .. }
| Value::NumericConstant { .. }
| Value::Global(_) => Some(value_id),
// Functions are not variables in a defunctionalized SSA. Only constant function values should appear.
Value::ForeignFunction(_) | Value::Function(_) | Value::Intrinsic(..) => None,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,8 @@ impl DependencyContext {
}
Value::Instruction { .. }
| Value::NumericConstant { .. }
| Value::Param { .. } => {
| Value::Param { .. }
| Value::Global(_) => {
panic!(
"calling non-function value with ID {func_id} in function {}",
function.name()
Expand Down Expand Up @@ -618,7 +619,8 @@ impl Context {
}
Value::Instruction { .. }
| Value::NumericConstant { .. }
| Value::Param { .. } => {
| Value::Param { .. }
| Value::Global(_) => {
panic!("At the point we are running disconnect there shouldn't be any other values as arguments")
}
}
Expand Down
7 changes: 7 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,10 @@ impl DataFlowGraph {
id
}

pub(crate) fn make_global(&mut self, typ: Type) -> ValueId {
self.values.insert(Value::Global(typ))
}

/// Gets or creates a ValueId for the given FunctionId.
pub(crate) fn import_function(&mut self, function: FunctionId) -> ValueId {
if let Some(existing) = self.functions.get(&function) {
Expand Down Expand Up @@ -589,6 +593,9 @@ impl DataFlowGraph {
}
_ => false,
},
// TODO: Make this true and handle instruction simplifications with globals.
// Currently all globals are inlined as a temporary measure so this is fine to have as false.
Value::Global(_) => false,
_ => true,
}
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ fn value(dfg: &DataFlowGraph, id: ValueId) -> String {
Value::Intrinsic(intrinsic) => intrinsic.to_string(),
Value::ForeignFunction(function) => function.clone(),
Value::Param { .. } | Value::Instruction { .. } => id.to_string(),
Value::Global(_) => {
format!("@{id}")
}
jfecher marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
20 changes: 17 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ir/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,27 @@ pub(crate) enum Value {
/// 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.
Instruction { instruction: InstructionId, position: usize, typ: Type },
Instruction {
instruction: InstructionId,
position: usize,
typ: Type,
},

/// This Value originates from a block parameter. Since function parameters
/// are also represented as block parameters, this includes function parameters as well.
///
/// position -- the index of this Value in the block parameters list
Param { block: BasicBlockId, position: usize, typ: Type },
Param {
block: BasicBlockId,
position: usize,
typ: Type,
},
vezenovm marked this conversation as resolved.
Show resolved Hide resolved

/// This Value originates from a numeric constant
NumericConstant { constant: FieldElement, typ: NumericType },
NumericConstant {
constant: FieldElement,
typ: NumericType,
},

/// This Value refers to a function in the IR.
/// Functions always have the type Type::Function.
Expand All @@ -53,6 +64,8 @@ pub(crate) enum Value {
/// ForeignFunction's always have the type Type::Function and have similar semantics to Function,
/// other than generating different backend operations and being only accessible through Brillig.
ForeignFunction(String),

Global(Type),
}

impl Value {
Expand All @@ -64,6 +77,7 @@ impl Value {
Value::Function { .. } | Value::Intrinsic { .. } | Value::ForeignFunction { .. } => {
Cow::Owned(Type::Function)
}
Value::Global(typ) => Cow::Borrowed(typ),
}
}
}
51 changes: 42 additions & 9 deletions compiler/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::ssa::{
instruction::{Instruction, InstructionId, TerminatorInstruction},
value::{Value, ValueId},
},
ssa_gen::Ssa,
ssa_gen::{context::GlobalsContext, Ssa},
};
use fxhash::FxHashMap as HashMap;

Expand Down Expand Up @@ -80,7 +80,7 @@ impl Ssa {
/// This works using an internal FunctionBuilder to build a new main function from scratch.
/// Doing it this way properly handles importing instructions between functions and lets us
/// reuse the existing API at the cost of essentially cloning each of main's instructions.
struct InlineContext {
struct InlineContext<'global> {
recursion_level: u32,
builder: FunctionBuilder,

Expand All @@ -98,20 +98,22 @@ struct InlineContext {

// These are the functions of the program that we shouldn't inline.
functions_not_to_inline: BTreeSet<FunctionId>,

globals: &'global GlobalsContext,
}

/// The per-function inlining context contains information that is only valid for one function.
/// For example, each function has its own DataFlowGraph, and thus each function needs a translation
/// layer to translate between BlockId to BlockId for the current function and the function to
/// inline into. The same goes for ValueIds, InstructionIds, and for storing other data like
/// parameter to argument mappings.
struct PerFunctionContext<'function> {
struct PerFunctionContext<'function, 'global> {
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
/// The source function is the function we're currently inlining into the function being built.
source_function: &'function Function,

/// The shared inlining context for all functions. This notably contains the FunctionBuilder used
/// to build the function we're inlining into.
context: &'function mut InlineContext,
context: &'function mut InlineContext<'global>,

/// Maps ValueIds in the function being inlined to the new ValueIds to use in the function
/// being inlined into. This mapping also contains the mapping from parameter values to
Expand Down Expand Up @@ -347,18 +349,18 @@ fn compute_function_interface_cost(func: &Function) -> usize {
func.parameters().len() + func.returns().len()
}

impl InlineContext {
impl<'global> InlineContext<'global> {
/// Create a new context object for the function inlining pass.
/// This starts off with an empty mapping of instructions for main's parameters.
/// The function being inlined into will always be the main function, although it is
/// actually a copy that is created in case the original main is still needed from a function
/// that could not be inlined calling it.
fn new(
ssa: &Ssa,
ssa: &'global Ssa,
entry_point: FunctionId,
inline_no_predicates_functions: bool,
functions_not_to_inline: BTreeSet<FunctionId>,
) -> InlineContext {
) -> Self {
let source = &ssa.functions[&entry_point];
let mut builder = FunctionBuilder::new(source.name().to_owned(), entry_point);
builder.set_runtime(source.runtime());
Expand All @@ -369,6 +371,7 @@ impl InlineContext {
call_stack: CallStackId::root(),
inline_no_predicates_functions,
functions_not_to_inline,
globals: &ssa.globals,
}
}

Expand All @@ -379,6 +382,10 @@ impl InlineContext {
let mut context = PerFunctionContext::new(&mut self, entry_point);
context.inlining_entry = true;

for (_, value) in ssa.globals.dfg.values_iter() {
context.context.builder.current_function.dfg.make_global(value.get_type().into_owned());
}

// The entry block is already inserted so we have to add it to context.blocks and add
// its parameters here. Failing to do so would cause context.translate_block() to add
// a fresh block for the entry block rather than use the existing one.
Expand Down Expand Up @@ -437,12 +444,15 @@ impl InlineContext {
}
}

impl<'function> PerFunctionContext<'function> {
impl<'function, 'global> PerFunctionContext<'function, 'global> {
/// Create a new PerFunctionContext from the source function.
/// The value and block mappings for this context are initially empty except
/// for containing the mapping between parameters in the source_function and
/// the arguments of the destination function.
fn new(context: &'function mut InlineContext, source_function: &'function Function) -> Self {
fn new(
context: &'function mut InlineContext<'global>,
source_function: &'function Function,
) -> Self {
Self {
context,
source_function,
Expand Down Expand Up @@ -478,6 +488,29 @@ impl<'function> PerFunctionContext<'function> {
Value::ForeignFunction(function) => {
self.context.builder.import_foreign_function(function)
}
Value::Global(_) => {
// TODO: Inlining the global into the function is only a temporary measure
// until Brillig gen with globals is working end to end
let new_id = match &self.context.globals.dfg[id] {
Value::Instruction { instruction, .. } => {
let Instruction::MakeArray { elements, typ } =
&self.context.globals.dfg[*instruction]
else {
panic!("Only expect Instruction::MakeArray for a global");
};
let elements = elements
.iter()
.map(|element| self.translate_value(*element))
.collect::<im::Vector<_>>();
self.context.builder.insert_make_array(elements, typ.clone())
}
Value::NumericConstant { constant, typ } => {
self.context.builder.numeric_constant(*constant, *typ)
}
_ => panic!("Expected only an instruction or a numeric constant"),
};
new_id
}
};

self.values.insert(id, new_value);
Expand Down
15 changes: 12 additions & 3 deletions compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::ssa::{
post_order::PostOrder,
value::{Value, ValueId},
},
ssa_gen::Ssa,
ssa_gen::{context::GlobalsContext, Ssa},
};
use fxhash::FxHashMap as HashMap;
use iter_extended::vecmap;
Expand All @@ -25,7 +25,7 @@ impl Ssa {
let mut context = Context::default();
context.populate_functions(&self.functions);
for function in self.functions.values_mut() {
context.normalize_ids(function);
context.normalize_ids(function, &self.globals);
}
self.functions = context.functions.into_btree();
}
Expand Down Expand Up @@ -65,13 +65,17 @@ impl Context {
}
}

fn normalize_ids(&mut self, old_function: &mut Function) {
fn normalize_ids(&mut self, old_function: &mut Function, globals: &GlobalsContext) {
self.new_ids.blocks.clear();
self.new_ids.values.clear();

let new_function_id = self.new_ids.function_ids[&old_function.id()];
let new_function = &mut self.functions[new_function_id];

for (_, value) in globals.dfg.values_iter() {
new_function.dfg.make_global(value.get_type().into_owned());
}

let mut reachable_blocks = PostOrder::with_function(old_function).into_vec();
reachable_blocks.reverse();

Expand Down Expand Up @@ -192,6 +196,11 @@ impl IdMaps {
}
Value::Intrinsic(intrinsic) => new_function.dfg.import_intrinsic(*intrinsic),
Value::ForeignFunction(name) => new_function.dfg.import_foreign_function(name),
Value::Global(_) => {
// Globals are computed at compile-time and thus are expected to be remain normalized
// between SSA passes
old_value
},
}
}
}
Loading
Loading