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): Generic ValueId to show resolved status #6487

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
7d2b615
Experiment with a generic ValueId
aakoshh Nov 8, 2024
73664a6
More fixes
aakoshh Nov 8, 2024
ebed1de
Index by ValueId
aakoshh Nov 8, 2024
4354822
Merge remote-tracking branch 'origin/master' into exp-resolved-value-id
aakoshh Nov 8, 2024
ee85702
mem2reg
aakoshh Nov 9, 2024
998381c
Resolution
aakoshh Nov 9, 2024
6cbbd53
Fix remaining issues
aakoshh Nov 9, 2024
6694447
Fix clippy
aakoshh Nov 9, 2024
b40d43e
Fix set_value_from_id
aakoshh Nov 9, 2024
c370239
Use if let Some
aakoshh Nov 9, 2024
cb121a5
Merge branch 'master' into exp-resolved-value-id
aakoshh Nov 11, 2024
e4b3580
Add back resolve in acir_gen
aakoshh Nov 12, 2024
d63f74d
Use ResolvedValueId in Brillig gen
aakoshh Nov 12, 2024
2f39247
Merge branch 'exp-resolved-value-id' of github.com:noir-lang/noir int…
aakoshh Nov 12, 2024
61a7b60
Use ResolvedValueId in ConstantAllocation
aakoshh Nov 12, 2024
fa70f95
Use ResolvedValueId in array_set last_use
aakoshh Nov 12, 2024
faebc03
Merge remote-tracking branch 'origin/master' into exp-resolved-value-id
aakoshh Nov 12, 2024
db384b7
Merge branch 'master' into exp-resolved-value-id
aakoshh Nov 12, 2024
46eb9ec
Merge remote-tracking branch 'origin/master' into exp-resolved-value-id
aakoshh Nov 13, 2024
748a292
Merge branch 'exp-resolved-value-id' of github.com:noir-lang/noir int…
aakoshh Nov 13, 2024
01c999f
Merge remote-tracking branch 'origin/master' into exp-resolved-value-id
aakoshh Nov 14, 2024
91c6923
Merge remote-tracking branch 'origin/master' into exp-resolved-value-id
aakoshh Nov 18, 2024
ed4117a
More ResolvedValueId in brillig_gen
aakoshh Nov 18, 2024
00ba58f
Merge remote-tracking branch 'origin/master' into exp-resolved-value-id
aakoshh Nov 18, 2024
72ad1db
Remove Hash from Resolved, add module specific variants
aakoshh Nov 18, 2024
efc9a61
Add lifetime to Resolved so it is non-trivial to persist it
aakoshh Nov 19, 2024
cc4b64d
Remove use of .resolved() except in tests
aakoshh Nov 19, 2024
94843cd
Try to set stack size to 8MB
aakoshh Nov 19, 2024
cfe03b0
Remove ContextResolved and FinalResolved where it's easy. Use just th…
aakoshh Nov 19, 2024
12501e5
Merge branch 'master' into exp-resolved-value-id
aakoshh Nov 19, 2024
3a39470
Merge remote-tracking branch 'origin/master' into exp-resolved-value-id
aakoshh Nov 19, 2024
c9ec8ef
Merge branch 'exp-resolved-value-id' of github.com:noir-lang/noir int…
aakoshh Nov 19, 2024
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
42 changes: 24 additions & 18 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::brillig::brillig_ir::{
};
use crate::ssa::ir::dfg::CallStack;
use crate::ssa::ir::instruction::ConstrainError;
use crate::ssa::ir::value::ResolvedValueId;
use crate::ssa::ir::{
basic_block::BasicBlockId,
dfg::DataFlowGraph,
Expand Down Expand Up @@ -41,7 +42,7 @@ pub(crate) struct BrilligBlock<'block> {
/// Tracks the available variable during the codegen of the block
pub(crate) variables: BlockVariables,
/// For each instruction, the set of values that are not used anymore after it.
pub(crate) last_uses: HashMap<InstructionId, HashSet<ValueId>>,
pub(crate) last_uses: HashMap<InstructionId, HashSet<ResolvedValueId>>,
}

impl<'block> BrilligBlock<'block> {
Expand Down Expand Up @@ -149,8 +150,8 @@ impl<'block> BrilligBlock<'block> {
let target_block = &dfg[*destination_block];
for (src, dest) in arguments.iter().zip(target_block.parameters()) {
// Destinations are block parameters so they should have been allocated previously.
let destination =
self.variables.get_allocation(self.function_context, *dest, dfg);
let dest = dfg.resolve(*dest);
let destination = self.variables.get_allocation(self.function_context, dest);
let source = self.convert_ssa_value(*src, dfg);
self.brillig_context
.mov_instruction(destination.extract_register(), source.extract_register());
Expand Down Expand Up @@ -772,7 +773,7 @@ impl<'block> BrilligBlock<'block> {

for dead_variable in dead_variables {
self.variables.remove_variable(
dead_variable,
*dead_variable,
self.function_context,
self.brillig_context,
);
Expand Down Expand Up @@ -1479,19 +1480,18 @@ impl<'block> BrilligBlock<'block> {
Value::Param { .. } | Value::Instruction { .. } => {
// All block parameters and instruction results should have already been
// converted to registers so we fetch from the cache.

self.variables.get_allocation(self.function_context, value_id, dfg)
self.variables.get_allocation(self.function_context, value_id)
}
Value::NumericConstant { constant, .. } => {
// Constants might have been converted previously or not, so we get or create and
// (re)initialize the value inside.
if self.variables.is_allocated(&value_id) {
self.variables.get_allocation(self.function_context, value_id, dfg)
if self.variables.is_allocated(value_id) {
self.variables.get_allocation(self.function_context, value_id)
} else {
let new_variable = self.variables.define_variable(
self.function_context,
self.brillig_context,
value_id,
value_id.into(),
dfg,
);

Expand All @@ -1501,13 +1501,13 @@ impl<'block> BrilligBlock<'block> {
}
}
Value::Array { array, typ } => {
if self.variables.is_allocated(&value_id) {
self.variables.get_allocation(self.function_context, value_id, dfg)
if self.variables.is_allocated(value_id) {
self.variables.get_allocation(self.function_context, value_id)
} else {
let new_variable = self.variables.define_variable(
self.function_context,
self.brillig_context,
value_id,
value_id.into(),
dfg,
);

Expand Down Expand Up @@ -1548,13 +1548,13 @@ impl<'block> BrilligBlock<'block> {
let new_variable = self.variables.define_variable(
self.function_context,
self.brillig_context,
value_id,
value_id.into(),
dfg,
);

self.brillig_context.const_instruction(
new_variable.extract_single_addr(),
value_id.to_usize().into(),
value_id.raw().to_usize().into(),
);
new_variable
}
Expand All @@ -1577,11 +1577,13 @@ impl<'block> BrilligBlock<'block> {
let item_types = typ.clone().element_types();

// Find out if we are repeating the same item over and over
let first_item = data.iter().take(item_types.len()).copied().collect();
let first_item: Vec<ResolvedValueId> =
data.iter().take(item_types.len()).map(|v| v.resolved()).collect();
let mut is_repeating = true;

for item_index in (item_types.len()..data.len()).step_by(item_types.len()) {
let item: Vec<_> = (0..item_types.len()).map(|i| data[item_index + i]).collect();
let item: Vec<_> =
(0..item_types.len()).map(|i| data[item_index + i].resolved()).collect();
if first_item != item {
is_repeating = false;
break;
Expand All @@ -1597,7 +1599,11 @@ impl<'block> BrilligBlock<'block> {
&& item_types.iter().all(|typ| matches!(typ, Type::Numeric(_)))
{
self.initialize_constant_array_runtime(
item_types, first_item, item_count, pointer, dfg,
item_types,
vecmap(first_item, |v| v.into()),
item_count,
pointer,
dfg,
);
} else {
self.initialize_constant_array_comptime(data, dfg, pointer);
Expand Down Expand Up @@ -1688,7 +1694,7 @@ impl<'block> BrilligBlock<'block> {

fn initialize_constant_array_comptime(
&mut self,
data: &im::Vector<crate::ssa::ir::map::Id<Value>>,
data: &im::Vector<ValueId>,
dfg: &DataFlowGraph,
pointer: MemoryAddress,
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,22 @@ use crate::{
ssa::ir::{
dfg::DataFlowGraph,
types::{CompositeType, Type},
value::ValueId,
value::{ResolvedValueId, ValueId},
},
};

use super::brillig_fn::FunctionContext;

#[derive(Debug, Default)]
pub(crate) struct BlockVariables {
available_variables: HashSet<ValueId>,
// Since we're generating Brillig bytecode here, there shouldn't be any more value ID replacements
// and we can use the final resolved ID.
available_variables: HashSet<ResolvedValueId>,
}

impl BlockVariables {
/// Creates a BlockVariables instance. It uses the variables that are live in to the block and the global available variables (block parameters)
pub(crate) fn new(live_in: HashSet<ValueId>) -> Self {
pub(crate) fn new(live_in: HashSet<ResolvedValueId>) -> Self {
BlockVariables { available_variables: live_in }
}

Expand Down Expand Up @@ -82,32 +84,29 @@ impl BlockVariables {
/// Removes a variable so it's not used anymore within this block.
pub(crate) fn remove_variable(
&mut self,
value_id: &ValueId,
value_id: ResolvedValueId,
function_context: &mut FunctionContext,
brillig_context: &mut BrilligContext<FieldElement, Stack>,
) {
assert!(self.available_variables.remove(value_id), "ICE: Variable is not available");
assert!(self.available_variables.remove(&value_id), "ICE: Variable is not available");
let variable = function_context
.ssa_value_allocations
.get(value_id)
.get(&value_id)
.expect("ICE: Variable allocation not found");
brillig_context.deallocate_register(variable.extract_register());
}

/// Checks if a variable is allocated.
pub(crate) fn is_allocated(&self, value_id: &ValueId) -> bool {
self.available_variables.contains(value_id)
pub(crate) fn is_allocated(&self, value_id: ResolvedValueId) -> bool {
self.available_variables.contains(&value_id)
}

/// For a given SSA value id, return the corresponding cached allocation.
pub(crate) fn get_allocation(
&mut self,
function_context: &FunctionContext,
value_id: ValueId,
dfg: &DataFlowGraph,
value_id: ResolvedValueId,
) -> BrilligVariable {
let value_id = dfg.resolve(value_id);

assert!(
self.available_variables.contains(&value_id),
"ICE: ValueId {value_id:?} is not available"
Expand All @@ -127,7 +126,7 @@ pub(crate) fn compute_array_length(item_typ: &CompositeType, elem_count: usize)

/// For a given value_id, allocates the necessary registers to hold it.
pub(crate) fn allocate_value<F, Registers: RegisterAllocator>(
value_id: ValueId,
value_id: ResolvedValueId,
brillig_context: &mut BrilligContext<F, Registers>,
dfg: &DataFlowGraph,
) -> BrilligVariable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
function::{Function, FunctionId},
post_order::PostOrder,
types::Type,
value::ValueId,
value::ResolvedValueId,
},
};
use fxhash::FxHashMap as HashMap;
Expand All @@ -20,7 +20,7 @@ use super::{constant_allocation::ConstantAllocation, variable_liveness::Variable
pub(crate) struct FunctionContext {
pub(crate) function_id: FunctionId,
/// Map from SSA values its allocation. Since values can be only defined once in SSA form, we insert them here on when we allocate them at their definition.
pub(crate) ssa_value_allocations: HashMap<ValueId, BrilligVariable>,
pub(crate) ssa_value_allocations: HashMap<ResolvedValueId, BrilligVariable>,
/// The block ids of the function in reverse post order.
pub(crate) blocks: Vec<BasicBlockId>,
/// Liveness information for each variable in the function.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::ssa::ir::{
function::Function,
instruction::InstructionId,
post_order::PostOrder,
value::{Value, ValueId},
value::{ResolvedValueId, Value, ValueId},
};

use super::variable_liveness::{collect_variables_of_value, variables_used_in_instruction};
Expand All @@ -23,7 +23,7 @@ pub(crate) enum InstructionLocation {
}

pub(crate) struct ConstantAllocation {
constant_usage: HashMap<ValueId, HashMap<BasicBlockId, Vec<InstructionLocation>>>,
constant_usage: HashMap<ResolvedValueId, HashMap<BasicBlockId, Vec<InstructionLocation>>>,
allocation_points: HashMap<BasicBlockId, HashMap<InstructionLocation, Vec<ValueId>>>,
dominator_tree: DominatorTree,
blocks_within_loops: HashSet<BasicBlockId>,
Expand Down Expand Up @@ -65,7 +65,7 @@ impl ConstantAllocation {

fn collect_constant_usage(&mut self, func: &Function) {
let mut record_if_constant =
|block_id: BasicBlockId, value_id: ValueId, location: InstructionLocation| {
|block_id: BasicBlockId, value_id: ResolvedValueId, location: InstructionLocation| {
if is_constant_value(value_id, &func.dfg) {
self.constant_usage
.entry(value_id)
Expand Down Expand Up @@ -101,8 +101,9 @@ impl ConstantAllocation {
fn decide_allocation_points(&mut self, func: &Function) {
for (constant_id, usage_in_blocks) in self.constant_usage.iter() {
let block_ids: Vec<_> = usage_in_blocks.iter().map(|(block_id, _)| *block_id).collect();
let constant_id = constant_id.into();

let allocation_point = self.decide_allocation_point(*constant_id, &block_ids, func);
let allocation_point = self.decide_allocation_point(constant_id, &block_ids, func);

// If the allocation point is one of the places where it's used, we take the first usage in the allocation point.
// Otherwise, we allocate it at the terminator of the allocation point.
Expand All @@ -121,7 +122,7 @@ impl ConstantAllocation {
.or_default()
.entry(location)
.or_default()
.push(*constant_id);
.push(constant_id);
}
}

Expand Down Expand Up @@ -165,8 +166,8 @@ impl ConstantAllocation {
}
}

pub(crate) fn is_constant_value(id: ValueId, dfg: &DataFlowGraph) -> bool {
matches!(&dfg[dfg.resolve(id)], Value::NumericConstant { .. } | Value::Array { .. })
fn is_constant_value(id: ResolvedValueId, dfg: &DataFlowGraph) -> bool {
matches!(&dfg[id], Value::NumericConstant { .. } | Value::Array { .. })
}

/// For a given function, finds all the blocks that are within loops
Expand Down
Loading
Loading