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(brillig): Hoist shared constants across functions to the global space #7216

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
6 changes: 4 additions & 2 deletions compiler/noirc_evaluator/src/brillig/brillig_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pub(crate) mod brillig_block_variables;
pub(crate) mod brillig_fn;
pub(crate) mod brillig_globals;
pub(crate) mod brillig_slice_ops;
mod constant_allocation;
pub(crate) mod constant_allocation;
mod variable_liveness;

use acvm::FieldElement;
Expand All @@ -20,14 +20,15 @@ use super::{
};
use crate::{
errors::InternalError,
ssa::ir::{call_stack::CallStack, function::Function},
ssa::ir::{call_stack::CallStack, function::Function, types::NumericType},
};

/// Converting an SSA function into Brillig bytecode.
pub(crate) fn convert_ssa_function(
func: &Function,
enable_debug_trace: bool,
globals: &HashMap<ValueId, BrilligVariable>,
hoisted_global_constants: &HashMap<(FieldElement, NumericType), BrilligVariable>,
) -> BrilligArtifact<FieldElement> {
let mut brillig_context = BrilligContext::new(enable_debug_trace);

Expand All @@ -44,6 +45,7 @@ pub(crate) fn convert_ssa_function(
block,
&func.dfg,
globals,
hoisted_global_constants,
);
}

Expand Down
47 changes: 44 additions & 3 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ use num_bigint::BigUint;
use std::sync::Arc;

use super::brillig_black_box::convert_black_box_call;
use super::brillig_block_variables::BlockVariables;
use super::brillig_block_variables::{allocate_value_with_type, BlockVariables};
use super::brillig_fn::FunctionContext;
use super::brillig_globals::HoistedConstantsToBrilligGlobals;
use super::constant_allocation::InstructionLocation;

/// Generate the compilation artifacts for compiling a function into brillig bytecode.
Expand All @@ -44,6 +45,7 @@ pub(crate) struct BrilligBlock<'block, Registers: RegisterAllocator> {
pub(crate) last_uses: HashMap<InstructionId, HashSet<ValueId>>,

pub(crate) globals: &'block HashMap<ValueId, BrilligVariable>,
pub(crate) hoisted_global_constants: &'block HoistedConstantsToBrilligGlobals,

pub(crate) building_globals: bool,
}
Expand All @@ -56,11 +58,17 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
block_id: BasicBlockId,
dfg: &DataFlowGraph,
globals: &'block HashMap<ValueId, BrilligVariable>,
hoisted_global_constants: &'block HoistedConstantsToBrilligGlobals,
) {
let live_in = function_context.liveness.get_live_in(&block_id);

let mut live_in_no_globals = HashSet::default();
for value in live_in {
if let Value::NumericConstant { constant, typ } = dfg[*value] {
if hoisted_global_constants.get(&(constant, typ)).is_some() {
continue;
}
}
if !dfg.is_global(*value) {
live_in_no_globals.insert(*value);
}
Expand All @@ -84,6 +92,7 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
variables,
last_uses,
globals,
hoisted_global_constants,
building_globals: false,
};

Expand All @@ -94,7 +103,10 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
&mut self,
globals: &DataFlowGraph,
used_globals: &HashSet<ValueId>,
) {
hoisted_global_constants: &HashSet<(FieldElement, NumericType)>,
) -> HashMap<(FieldElement, NumericType), BrilligVariable> {
let mut new_hoisted_constants = HashMap::default();

for (id, value) in globals.values_iter() {
if !used_globals.contains(&id) {
continue;
Expand All @@ -113,6 +125,17 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
}
}
}

for (constant, typ) in hoisted_global_constants {
let constant = *constant;
let typ = *typ;
let new_variable = allocate_value_with_type(self.brillig_context, Type::Numeric(typ));
self.brillig_context.const_instruction(new_variable.extract_single_addr(), constant);
if new_hoisted_constants.insert((constant, typ), new_variable).is_some() {
unreachable!("ICE: ({constant:?}, {typ:?}) was already in cache");
}
}
new_hoisted_constants
}

fn convert_block(&mut self, dfg: &DataFlowGraph) {
Expand Down Expand Up @@ -904,7 +927,8 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {

for dead_variable in dead_variables {
// Globals are reserved throughout the entirety of the program
if !dfg.is_global(*dead_variable) {
let not_hoisted_global = self.get_hoisted_global(dfg, *dead_variable).is_none();
if !dfg.is_global(*dead_variable) && not_hoisted_global {
self.variables.remove_variable(
dead_variable,
self.function_context,
Expand Down Expand Up @@ -1622,6 +1646,10 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
let value_id = dfg.resolve(value_id);
let value = &dfg[value_id];

if let Some(variable) = self.get_hoisted_global(dfg, value_id) {
return variable;
}

match value {
Value::Global(_) => {
unreachable!("Expected global value to be resolve to its inner value");
Expand Down Expand Up @@ -1962,6 +1990,19 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
}
}
}

fn get_hoisted_global(
&self,
dfg: &DataFlowGraph,
value_id: ValueId,
) -> Option<BrilligVariable> {
if let Value::NumericConstant { constant, typ } = &dfg[value_id] {
if let Some(variable) = self.hoisted_global_constants.get(&(*constant, *typ)) {
return Some(*variable);
}
}
None
}
}

/// Returns the type of the operation considering the types of the operands
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use super::brillig_fn::FunctionContext;

#[derive(Debug, Default)]
pub(crate) struct BlockVariables {
available_variables: HashSet<ValueId>,
pub(crate) available_variables: HashSet<ValueId>,
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
}

impl BlockVariables {
Expand Down
Loading
Loading