Skip to content

Commit

Permalink
fix: unknown slice lengths coming from as_slice (noir-lang/noir#4725)
Browse files Browse the repository at this point in the history
chore: remove unused env vars from `Cross.toml` (noir-lang/noir#4717)
feat: improve nargo check cli with --override flag and feedback for existing files (noir-lang/noir#4575)
feat: Allow slices to brillig entry points (noir-lang/noir#4713)
chore: simplify how `acvm_backend.wasm` is embedded (noir-lang/noir#4703)
fix(acvm): Mark outputs of Opcode::Call solvable (noir-lang/noir#4708)
fix: Field comparisons (noir-lang/noir#4704)
feat(acvm_js): Execute program  (noir-lang/noir#4694)
chore: simplify how blns is loaded into tests (noir-lang/noir#4705)
fix(ssa): Do not use get_value_max_num_bits when we want pure type information (noir-lang/noir#4700)
chore: remove conditional compilation around `acvm_js` package (noir-lang/noir#4702)
feat(docs): Documenting noir codegen (noir-lang/noir#4454)
chore: check for references to private functions during path resolution (noir-lang/noir#4622)
chore: fix clippy errors (noir-lang/noir#4684)
fix: Last use analysis & make it an SSA pass (noir-lang/noir#4686)
feat: improve SSA type-awareness in EQ and MUL instructions (noir-lang/noir#4691)
feat: improve optimisations on range constraints (noir-lang/noir#4690)
chore: remove last traces of nix (noir-lang/noir#4679)
chore: Use is_entry_point helper on RuntimeType (noir-lang/noir#4678)
  • Loading branch information
AztecBot committed Apr 5, 2024
2 parents d60d64a + 39aba05 commit 40cf856
Show file tree
Hide file tree
Showing 15 changed files with 289 additions and 47 deletions.
2 changes: 0 additions & 2 deletions noir/noir-repo/.github/Cross.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
passthrough = [
"HOME",
"RUST_BACKTRACE",
"BARRETENBERG_BIN_DIR",
"BLNS_JSON_PATH"
]
volumes = [
"HOME",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl FunctionContext {
function_id.to_string()
}

fn ssa_type_to_parameter(typ: &Type) -> BrilligParameter {
pub(crate) fn ssa_type_to_parameter(typ: &Type) -> BrilligParameter {
match typ {
Type::Numeric(_) | Type::Reference(_) => {
BrilligParameter::SingleAddr(get_bit_size_from_ssa_type(typ))
Expand All @@ -81,26 +81,13 @@ impl FunctionContext {
}),
*size,
),
Type::Slice(item_type) => {
BrilligParameter::Slice(vecmap(item_type.iter(), |item_typ| {
FunctionContext::ssa_type_to_parameter(item_typ)
}))
Type::Slice(_) => {
panic!("ICE: Slice parameters cannot be derived from type information")
}
_ => unimplemented!("Unsupported function parameter/return type {typ:?}"),
}
}

/// Collects the parameters of a given function
pub(crate) fn parameters(func: &Function) -> Vec<BrilligParameter> {
func.parameters()
.iter()
.map(|&value_id| {
let typ = func.dfg.type_of_value(value_id);
FunctionContext::ssa_type_to_parameter(&typ)
})
.collect()
}

/// Collects the return values of a given function
pub(crate) fn return_values(func: &Function) -> Vec<BrilligParameter> {
func.returns()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ use std::collections::{BTreeMap, HashMap};

use crate::ssa::ir::dfg::CallStack;

/// Represents a parameter or a return value of a function.
/// Represents a parameter or a return value of an entry point function.
#[derive(Debug, Clone)]
pub(crate) enum BrilligParameter {
/// A single address parameter or return value. Holds the bit size of the parameter.
SingleAddr(u32),
/// An array parameter or return value. Holds the type of an array item and its size.
Array(Vec<BrilligParameter>, usize),
/// A slice parameter or return value. Holds the type of a slice item.
Slice(Vec<BrilligParameter>),
/// Only known-length slices can be passed to brillig entry points, so the size is available as well.
Slice(Vec<BrilligParameter>, usize),
}

/// The result of compiling and linking brillig artifacts.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{
artifact::{BrilligArtifact, BrilligParameter},
brillig_variable::{BrilligArray, BrilligVariable, SingleAddrVariable},
brillig_variable::{BrilligArray, BrilligVariable, BrilligVector, SingleAddrVariable},
debug_show::DebugShow,
registers::BrilligRegistersContext,
BrilligBinaryOp, BrilligContext, ReservedRegisters,
Expand Down Expand Up @@ -83,24 +83,56 @@ impl BrilligContext {
current_calldata_pointer += flattened_size;
var
}
BrilligParameter::Slice(_) => unimplemented!("Unsupported slices as parameter"),
BrilligParameter::Slice(_, _) => {
let pointer_to_the_array_in_calldata =
self.make_usize_constant_instruction(current_calldata_pointer.into());

let flattened_size = BrilligContext::flattened_size(argument);
let size_register = self.make_usize_constant_instruction(flattened_size.into());
let rc_register = self.make_usize_constant_instruction(1_usize.into());

let var = BrilligVariable::BrilligVector(BrilligVector {
pointer: pointer_to_the_array_in_calldata.address,
size: size_register.address,
rc: rc_register.address,
});

current_calldata_pointer += flattened_size;
var
}
})
.collect();

// Deflatten arrays
for (argument_variable, argument) in argument_variables.iter_mut().zip(arguments) {
if let (
BrilligVariable::BrilligArray(array),
BrilligParameter::Array(item_type, item_count),
) = (argument_variable, argument)
{
if BrilligContext::has_nested_arrays(item_type) {
match (argument_variable, argument) {
(
BrilligVariable::BrilligArray(array),
BrilligParameter::Array(item_type, item_count),
) => {
let deflattened_address =
self.deflatten_array(item_type, array.size, array.pointer);
self.mov_instruction(array.pointer, deflattened_address);
array.size = item_type.len() * item_count;
self.deallocate_register(deflattened_address);
}
(
BrilligVariable::BrilligVector(vector),
BrilligParameter::Slice(item_type, item_count),
) => {
let flattened_size = BrilligContext::flattened_size(argument);

let deflattened_address =
self.deflatten_array(item_type, flattened_size, vector.pointer);
self.mov_instruction(vector.pointer, deflattened_address);
self.usize_const_instruction(
vector.size,
(item_type.len() * item_count).into(),
);

self.deallocate_register(deflattened_address);
}
_ => {}
}
}
}
Expand All @@ -112,10 +144,10 @@ impl BrilligContext {
fn flat_bit_sizes(param: &BrilligParameter) -> Box<dyn Iterator<Item = u32> + '_> {
match param {
BrilligParameter::SingleAddr(bit_size) => Box::new(std::iter::once(*bit_size)),
BrilligParameter::Array(item_types, item_count) => Box::new(
BrilligParameter::Array(item_types, item_count)
| BrilligParameter::Slice(item_types, item_count) => Box::new(
(0..*item_count).flat_map(move |_| item_types.iter().flat_map(flat_bit_sizes)),
),
BrilligParameter::Slice(..) => unimplemented!("Unsupported slices as parameter"),
}
}

Expand All @@ -134,13 +166,11 @@ impl BrilligContext {
fn flattened_size(param: &BrilligParameter) -> usize {
match param {
BrilligParameter::SingleAddr(_) => 1,
BrilligParameter::Array(item_types, item_count) => {
BrilligParameter::Array(item_types, item_count)
| BrilligParameter::Slice(item_types, item_count) => {
let item_size: usize = item_types.iter().map(BrilligContext::flattened_size).sum();
item_count * item_size
}
BrilligParameter::Slice(_) => {
unreachable!("ICE: Slices cannot be passed as entry point arguments")
}
}
}

Expand Down Expand Up @@ -457,8 +487,8 @@ mod tests {
use acvm::FieldElement;

use crate::brillig::brillig_ir::{
artifact::BrilligParameter,
brillig_variable::BrilligArray,
entry_point::BrilligParameter,
tests::{create_and_run_vm, create_context, create_entry_point_bytecode},
};

Expand Down
1 change: 1 addition & 0 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ pub(crate) fn optimize_into_acir(
.run_pass(Ssa::inline_functions, "After Inlining:")
// Run mem2reg with the CFG separated into blocks
.run_pass(Ssa::mem2reg, "After Mem2Reg:")
.run_pass(Ssa::as_slice_optimization, "After `as_slice` optimization")
.try_run_pass(Ssa::evaluate_assert_constant, "After Assert Constant:")?
.try_run_pass(Ssa::unroll_loops, "After Unrolling:")?
.run_pass(Ssa::simplify_cfg, "After Simplifying:")
Expand Down
46 changes: 42 additions & 4 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use super::{
},
ssa_gen::Ssa,
};
use crate::brillig::brillig_ir::artifact::GeneratedBrillig;
use crate::brillig::brillig_ir::artifact::{BrilligParameter, GeneratedBrillig};
use crate::brillig::brillig_ir::BrilligContext;
use crate::brillig::{brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext, Brillig};
use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
Expand Down Expand Up @@ -297,12 +297,14 @@ impl Context {
let typ = dfg.type_of_value(*param_id);
self.create_value_from_type(&typ, &mut |this, _| Ok(this.acir_context.add_variable()))
})?;
let arguments = self.gen_brillig_parameters(dfg[main_func.entry_block()].parameters(), dfg);

let witness_inputs = self.acir_context.extract_witness(&inputs);

let outputs: Vec<AcirType> =
vecmap(main_func.returns(), |result_id| dfg.type_of_value(*result_id).into());

let code = self.gen_brillig_for(main_func, brillig)?;
let code = self.gen_brillig_for(main_func, arguments, brillig)?;

// We specifically do not attempt execution of the brillig code being generated as this can result in it being
// replaced with constraints on witnesses to the program outputs.
Expand Down Expand Up @@ -594,8 +596,9 @@ impl Context {
}

let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg));
let arguments = self.gen_brillig_parameters(arguments, dfg);

let code = self.gen_brillig_for(func, brillig)?;
let code = self.gen_brillig_for(func, arguments, brillig)?;

let outputs: Vec<AcirType> = vecmap(result_ids, |result_id| {
dfg.type_of_value(*result_id).into()
Expand Down Expand Up @@ -673,14 +676,49 @@ impl Context {
Ok(())
}

fn gen_brillig_parameters(
&self,
values: &[ValueId],
dfg: &DataFlowGraph,
) -> Vec<BrilligParameter> {
values
.iter()
.map(|&value_id| {
let typ = dfg.type_of_value(value_id);
if let Type::Slice(item_types) = typ {
let len = match self
.ssa_values
.get(&value_id)
.expect("ICE: Unknown slice input to brillig")
{
AcirValue::DynamicArray(AcirDynamicArray { len, .. }) => *len,
AcirValue::Array(array) => array.len(),
_ => unreachable!("ICE: Slice value is not an array"),
};

BrilligParameter::Slice(
item_types
.iter()
.map(BrilligFunctionContext::ssa_type_to_parameter)
.collect(),
len / item_types.len(),
)
} else {
BrilligFunctionContext::ssa_type_to_parameter(&typ)
}
})
.collect()
}

fn gen_brillig_for(
&self,
func: &Function,
arguments: Vec<BrilligParameter>,
brillig: &Brillig,
) -> Result<GeneratedBrillig, InternalError> {
// Create the entry point artifact
let mut entry_point = BrilligContext::new_entry_point_artifact(
BrilligFunctionContext::parameters(func),
arguments,
BrilligFunctionContext::return_values(func),
BrilligFunctionContext::function_id_to_function_label(func.id()),
);
Expand Down
24 changes: 24 additions & 0 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,30 @@ impl DataFlowGraph {
value_id
}

/// Replaces an instruction result with a fresh id.
pub(crate) fn replace_result(
&mut self,
instruction_id: InstructionId,
prev_value_id: ValueId,
) -> ValueId {
let typ = self.type_of_value(prev_value_id);
let results = self.results.get_mut(&instruction_id).unwrap();
let res_position = results
.iter()
.position(|&id| id == prev_value_id)
.expect("Result id not found while replacing");

let value_id = self.values.insert(Value::Instruction {
typ,
position: res_position,
instruction: instruction_id,
});

// Replace the value in list of results for this instruction
results[res_position] = value_id;
value_id
}

/// Returns the number of instructions
/// inserted into functions.
pub(crate) fn num_instructions(&self) -> usize {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
use crate::ssa::{
ir::{
function::Function,
instruction::{Instruction, InstructionId, Intrinsic},
types::Type,
value::Value,
},
ssa_gen::Ssa,
};
use fxhash::FxHashMap as HashMap;

impl Ssa {
/// A simple SSA pass to find any calls to `Intrinsic::AsSlice` and replacing any references to the length of the
/// resulting slice with the length of the array from which it was generated.
///
/// This allows the length of a slice generated from an array to be used in locations where a constant value is
/// necessary when the value of the array is unknown.
///
/// Note that this pass must be placed before loop unrolling to be useful.
#[tracing::instrument(level = "trace", skip(self))]
pub(crate) fn as_slice_optimization(mut self) -> Self {
for func in self.functions.values_mut() {
let known_slice_lengths = known_slice_lengths(func);
replace_known_slice_lengths(func, known_slice_lengths);
}
self
}
}

fn known_slice_lengths(func: &Function) -> HashMap<InstructionId, usize> {
let mut known_slice_lengths = HashMap::default();
for block_id in func.reachable_blocks() {
let block = &func.dfg[block_id];
for instruction_id in block.instructions() {
let (target_func, arguments) = match &func.dfg[*instruction_id] {
Instruction::Call { func, arguments } => (func, arguments),
_ => continue,
};

match &func.dfg[*target_func] {
Value::Intrinsic(Intrinsic::AsSlice) => {
let array_typ = func.dfg.type_of_value(arguments[0]);
if let Type::Array(_, length) = array_typ {
known_slice_lengths.insert(*instruction_id, length);
} else {
unreachable!("AsSlice called with non-array {}", array_typ);
}
}
_ => continue,
};
}
}
known_slice_lengths
}

fn replace_known_slice_lengths(
func: &mut Function,
known_slice_lengths: HashMap<InstructionId, usize>,
) {
known_slice_lengths.into_iter().for_each(|(instruction_id, known_length)| {
let call_returns = func.dfg.instruction_results(instruction_id);
let original_slice_length = call_returns[0];

// We won't use the new id for the original unknown length.
// This isn't strictly necessary as a new result will be defined the next time for which the instruction
// is reinserted but this avoids leaving the program in an invalid state.
func.dfg.replace_result(instruction_id, original_slice_length);
let known_length = func.dfg.make_constant(known_length.into(), Type::length_type());
func.dfg.set_value_from_id(original_slice_length, known_length);
});
}
1 change: 1 addition & 0 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! simpler form until the IR only has a single function remaining with 1 block within it.
//! Generally, these passes are also expected to minimize the final amount of instructions.
mod array_set;
mod as_slice_length;
mod assert_constant;
mod bubble_up_constrains;
mod constant_folding;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "array_to_slice_constant_length"
type = "bin"
authors = [""]
compiler_version = ">=0.26.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
val = "42"
Loading

0 comments on commit 40cf856

Please sign in to comment.