Skip to content

Commit

Permalink
fix: unknown slice lengths coming from as_slice (#4725)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #4722

## Summary\*



## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <tom@tomfren.ch>
  • Loading branch information
sirasistant and TomAFrench authored Apr 5, 2024
1 parent 59fb832 commit f21129e
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 0 deletions.
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());
}

0 comments on commit f21129e

Please sign in to comment.