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): Treat globals as constant in a function's DFG #7040

Merged
merged 41 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
41 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
8a052b0
access globals directly inside of the dfg
vezenovm Jan 13, 2025
e14ad65
fmt and clippy
vezenovm Jan 13, 2025
2db6171
cleanup
vezenovm Jan 13, 2025
12c4d3b
better get_numeric_constant func
vezenovm Jan 13, 2025
dcef98d
clippy in tests
vezenovm Jan 13, 2025
e166f24
Update compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
vezenovm Jan 13, 2025
9b8122b
check globals when fetching instructions in relevant case
vezenovm Jan 13, 2025
e05405c
Merge remote-tracking branch 'origin/mv/treat-globals-as-const-in-dfg…
vezenovm Jan 13, 2025
433da77
Update compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs
vezenovm Jan 14, 2025
a7a8e8a
Merge branch 'master' into mv/treat-globals-as-const-in-dfg
vezenovm Jan 14, 2025
381ba57
Merge branch 'master' into mv/treat-globals-as-const-in-dfg
vezenovm Jan 14, 2025
2bbed37
pr review
vezenovm Jan 14, 2025
e54caf6
Merge branch 'master' into mv/treat-globals-as-const-in-dfg
vezenovm Jan 15, 2025
c8e3abc
Merge branch 'master' into mv/treat-globals-as-const-in-dfg
vezenovm Jan 15, 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
9 changes: 8 additions & 1 deletion compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use super::{
ir::{
basic_block::BasicBlock,
call_stack::{CallStack, CallStackId},
dfg::InsertInstructionResult,
dfg::{GlobalsGraph, InsertInstructionResult},
function::RuntimeType,
instruction::{ConstrainError, InstructionId, Intrinsic},
types::NumericType,
Expand Down Expand Up @@ -73,6 +73,13 @@ impl FunctionBuilder {
self.current_function.set_runtime(runtime);
}

pub(crate) fn set_globals(&mut self, globals: Arc<GlobalsGraph>) {
for (_, value) in globals.values_iter() {
self.current_function.dfg.make_global(value.get_type().into_owned());
}
self.current_function.set_globals(globals);
}

/// Finish the current function and create a new function.
///
/// A FunctionBuilder can always only work on one function at a time, so care
Expand Down
89 changes: 78 additions & 11 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::borrow::Cow;
use std::{borrow::Cow, sync::Arc};

use crate::ssa::{function_builder::data_bus::DataBus, ir::instruction::SimplifyResult};

Expand Down Expand Up @@ -102,6 +102,31 @@ pub(crate) struct DataFlowGraph {

#[serde(skip)]
pub(crate) data_bus: DataBus,

pub(crate) globals: Arc<GlobalsGraph>,
}

/// The GlobalsGraph contains the actual global data.
/// Global data is expected to only be numeric constants or array constants (which are represented by Instruction::MakeArray).
/// The global's data will shared across functions and should be accessible inside of a function's DataFlowGraph.
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub(crate) struct GlobalsGraph {
/// Storage for all of the global values
values: DenseMap<Value>,
/// All of the instructions in the global value space.
/// These are expected to all be Instruction::MakeArray
instructions: DenseMap<Instruction>,
}

impl GlobalsGraph {
pub(crate) fn from_dfg(dfg: DataFlowGraph) -> Self {
Self { values: dfg.values, instructions: dfg.instructions }
}

/// Iterate over every Value in this DFG in no particular order, including unused Values
pub(crate) fn values_iter(&self) -> impl ExactSizeIterator<Item = (ValueId, &Value)> {
self.values.iter()
}
}

impl DataFlowGraph {
Expand Down Expand Up @@ -511,7 +536,7 @@ impl DataFlowGraph {
&self,
value: ValueId,
) -> Option<(FieldElement, NumericType)> {
match &self.values[self.resolve(value)] {
match &self[self.resolve(value)] {
Value::NumericConstant { constant, typ } => Some((*constant, *typ)),
_ => None,
}
Expand All @@ -520,13 +545,15 @@ impl DataFlowGraph {
/// Returns the Value::Array associated with this ValueId if it refers to an array constant.
/// Otherwise, this returns None.
pub(crate) fn get_array_constant(&self, value: ValueId) -> Option<(im::Vector<ValueId>, Type)> {
match &self.values[self.resolve(value)] {
Value::Instruction { instruction, .. } => match &self.instructions[*instruction] {
let value = self.resolve(value);
if let Some(instruction) = self.get_instruction(value) {
match instruction {
Instruction::MakeArray { elements, typ } => Some((elements.clone(), typ.clone())),
_ => None,
},
}
} else {
// Arrays are shared, so cloning them is cheap
_ => None,
None
}
}

Expand Down Expand Up @@ -619,17 +646,21 @@ impl DataFlowGraph {

/// True if the given ValueId refers to a (recursively) constant value
pub(crate) fn is_constant(&self, argument: ValueId) -> bool {
match &self[self.resolve(argument)] {
let argument = self.resolve(argument);
if self.is_global(argument) {
return true;
}
match &self[argument] {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
Value::Param { .. } => false,
Value::Instruction { instruction, .. } => match &self[*instruction] {
Instruction::MakeArray { elements, .. } => {
elements.iter().all(|element| self.is_constant(*element))
}
_ => 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,
Value::Global(_) => {
unreachable!("The global value case should already be handled");
}
_ => true,
}
}
Expand All @@ -642,6 +673,24 @@ impl DataFlowGraph {
false
}
}

pub(crate) fn is_global(&self, value: ValueId) -> bool {
matches!(self.values[value], Value::Global(_))
}

pub(crate) fn get_instruction(&self, value: ValueId) -> Option<&Instruction> {
match &self[value] {
asterite marked this conversation as resolved.
Show resolved Hide resolved
Value::Instruction { instruction, .. } => {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
let instruction = if self.is_global(value) {
&self.globals[*instruction]
} else {
&self[*instruction]
};
Some(instruction)
}
_ => None,
}
}
}

impl std::ops::Index<InstructionId> for DataFlowGraph {
Expand All @@ -660,7 +709,11 @@ impl std::ops::IndexMut<InstructionId> for DataFlowGraph {
impl std::ops::Index<ValueId> for DataFlowGraph {
type Output = Value;
fn index(&self, id: ValueId) -> &Self::Output {
&self.values[id]
let value = &self.values[id];
if matches!(value, Value::Global(_)) {
return &self.globals[id];
}
value
}
jfecher marked this conversation as resolved.
Show resolved Hide resolved
}

Expand All @@ -678,6 +731,20 @@ impl std::ops::IndexMut<BasicBlockId> for DataFlowGraph {
}
}

impl std::ops::Index<ValueId> for GlobalsGraph {
type Output = Value;
fn index(&self, id: ValueId) -> &Self::Output {
&self.values[id]
}
}

impl std::ops::Index<InstructionId> for GlobalsGraph {
type Output = Instruction;
fn index(&self, id: InstructionId) -> &Self::Output {
&self.instructions[id]
}
}

// The result of calling DataFlowGraph::insert_instruction can
// be a list of results or a single ValueId if the instruction was simplified
// to an existing value.
Expand Down
8 changes: 7 additions & 1 deletion compiler/noirc_evaluator/src/ssa/ir/function.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::collections::BTreeSet;
use std::sync::Arc;

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::dfg::{DataFlowGraph, GlobalsGraph};
use super::instruction::TerminatorInstruction;
use super::map::Id;
use super::types::Type;
Expand Down Expand Up @@ -112,6 +113,7 @@ impl Function {
pub(crate) fn clone_signature(id: FunctionId, another: &Function) -> Self {
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
}

Expand All @@ -136,6 +138,10 @@ impl Function {
self.dfg.set_runtime(runtime);
}

pub(crate) fn set_globals(&mut self, globals: Arc<GlobalsGraph>) {
self.dfg.globals = globals;
}

pub(crate) fn is_no_predicates(&self) -> bool {
match self.runtime() {
RuntimeType::Acir(inline_type) => matches!(inline_type, InlineType::NoPredicates),
Expand Down
35 changes: 17 additions & 18 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,28 +1093,27 @@ fn try_optimize_array_get_from_previous_set(
// Arbitrary number of maximum tries just to prevent this optimization from taking too long.
let max_tries = 5;
for _ in 0..max_tries {
match &dfg[array_id] {
Value::Instruction { instruction, .. } => {
match &dfg[*instruction] {
Instruction::ArraySet { array, index, value, .. } => {
if let Some(constant) = dfg.get_numeric_constant(*index) {
if constant == target_index {
return SimplifyResult::SimplifiedTo(*value);
}

array_id = *array; // recur
} else {
return SimplifyResult::None;
if let Some(instruction) = dfg.get_instruction(array_id) {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
match instruction {
Instruction::ArraySet { array, index, value, .. } => {
if let Some(constant) = dfg.get_numeric_constant(*index) {
if constant == target_index {
return SimplifyResult::SimplifiedTo(*value);
}

array_id = *array; // recur
} else {
return SimplifyResult::None;
}
Instruction::MakeArray { elements: array, typ: _ } => {
elements = Some(array.clone());
break;
}
_ => return SimplifyResult::None,
}
Instruction::MakeArray { elements: array, typ: _ } => {
elements = Some(array.clone());
break;
}
_ => return SimplifyResult::None,
}
_ => return SimplifyResult::None,
} else {
return SimplifyResult::None;
}
}

Expand Down
8 changes: 7 additions & 1 deletion compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,13 @@ fn value(dfg: &DataFlowGraph, id: ValueId) -> String {
Value::Function(id) => id.to_string(),
Value::Intrinsic(intrinsic) => intrinsic.to_string(),
Value::ForeignFunction(function) => function.clone(),
Value::Param { .. } | Value::Instruction { .. } => id.to_string(),
Value::Param { .. } | Value::Instruction { .. } => {
if dfg.is_global(id) {
format!("g{}", id.to_u32())
asterite marked this conversation as resolved.
Show resolved Hide resolved
} else {
id.to_string()
}
}
Value::Global(_) => {
format!("g{}", id.to_u32())
}
Expand Down
14 changes: 11 additions & 3 deletions compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
//! with a non-literal target can be replaced with a call to an apply function.
//! The apply function is a dispatch function that takes the function id as a parameter
//! and dispatches to the correct target.
use std::collections::{BTreeMap, BTreeSet, HashSet};
use std::{
collections::{BTreeMap, BTreeSet, HashSet},
sync::Arc,
};

use acvm::FieldElement;
use iter_extended::vecmap;
Expand All @@ -13,6 +16,7 @@ use crate::ssa::{
function_builder::FunctionBuilder,
ir::{
basic_block::BasicBlockId,
dfg::GlobalsGraph,
function::{Function, FunctionId, Signature},
instruction::{BinaryOp, Instruction},
types::{NumericType, Type},
Expand Down Expand Up @@ -278,8 +282,10 @@ fn create_apply_function(
function_ids: Vec<FunctionId>,
) -> FunctionId {
assert!(!function_ids.is_empty());
ssa.add_fn(|id| {
let globals_dfg = std::mem::take(&mut ssa.globals.dfg);
let new_id = ssa.add_fn(|id| {
let mut function_builder = FunctionBuilder::new("apply".to_string(), id);
function_builder.set_globals(Arc::new(GlobalsGraph::from_dfg(globals_dfg.clone())));
jfecher marked this conversation as resolved.
Show resolved Hide resolved
let target_id = function_builder.add_parameter(Type::field());
let params_ids = vecmap(signature.params, |typ| function_builder.add_parameter(typ));

Expand Down Expand Up @@ -334,7 +340,9 @@ fn create_apply_function(
}
}
function_builder.current_function
})
});
ssa.globals.dfg = globals_dfg;
new_id
}

/// Crates a return block, if no previous return exists, it will create a final return
Expand Down
44 changes: 22 additions & 22 deletions compiler/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ impl InlineContext {
let source = &ssa.functions[&entry_point];
let mut builder = FunctionBuilder::new(source.name().to_owned(), entry_point);
builder.set_runtime(source.runtime());
builder.current_function.set_globals(source.dfg.globals.clone());

Self {
builder,
recursion_level: 0,
Expand All @@ -382,7 +384,7 @@ impl InlineContext {
let mut context = PerFunctionContext::new(&mut self, entry_point, &ssa.globals);
context.inlining_entry = true;

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

Expand Down Expand Up @@ -476,13 +478,30 @@ impl<'function> PerFunctionContext<'function> {
}

let new_value = match &self.source_function.dfg[id] {
value @ Value::Instruction { .. } => {
value @ Value::Instruction { instruction, .. } => {
// TODO: Inlining the global into the function is only a temporary measure
// until Brillig gen with globals is working end to end
if self.source_function.dfg.is_global(id) {
let Instruction::MakeArray { elements, typ } = &self.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<_>>();
return self.context.builder.insert_make_array(elements, typ.clone());
}
unreachable!("All Value::Instructions should already be known during inlining after creating the original inlined instruction. Unknown value {id} = {value:?}")
}
value @ Value::Param { .. } => {
unreachable!("All Value::Params should already be known from previous calls to translate_block. Unknown value {id} = {value:?}")
}
Value::NumericConstant { constant, typ } => {
// TODO: Inlining the global into the function is only a temporary measure
// until Brillig gen with globals is working end to end.
// The dfg indexes a global's inner value directly, so we will need to check here
// whether we have a global.
self.context.builder.numeric_constant(*constant, *typ)
}
Value::Function(function) => self.context.builder.import_function(*function),
Expand All @@ -491,26 +510,7 @@ impl<'function> PerFunctionContext<'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
match &self.globals.dfg[id] {
Value::Instruction { instruction, .. } => {
let Instruction::MakeArray { elements, typ } =
&self.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"),
}
panic!("Expected a global to be resolved to its inner value");
}
};

Expand Down
Loading
Loading