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

fix: unknown slice lengths coming from as_slice #4725

Merged
merged 4 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions 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
24 changes: 24 additions & 0 deletions 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
71 changes: 71 additions & 0 deletions compiler/noirc_evaluator/src/ssa/opt/as_slice_length.rs
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 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"
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Regression test for https://github.com/noir-lang/noir/issues/4722

unconstrained fn return_array(val: Field) -> [Field; 1] {
[val; 1]
}

fn main(val: Field) {
let array = return_array(val);
assert_constant(array.as_slice().len());
}
Loading