Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/acir/arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl Context<'_> {

// Pass the instruction between array methods rather than the internal fields themselves
let (array, index, store_value) = match dfg[instruction] {
Instruction::ArrayGet { array, index } => (array, index, None),
Instruction::ArrayGet { array, index, offset: _ } => (array, index, None),
Instruction::ArraySet { array, index, value, mutable } => {
mutable_array_set = mutable;
(array, index, Some(value))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use crate::brillig::brillig_ir::{
BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, BrilligBinaryOp, BrilligContext, ReservedRegisters,
};
use crate::ssa::ir::instruction::{ConstrainError, Hint};
use crate::ssa::ir::instruction::{ArrayGetOffset, ConstrainError, Hint};
use crate::ssa::ir::{
basic_block::BasicBlockId,
dfg::DataFlowGraph,
Expand Down Expand Up @@ -792,7 +792,7 @@
let source_variable = self.convert_ssa_single_addr_value(*value, dfg);
self.convert_cast(destination_variable, source_variable);
}
Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array, index, offset } => {
let result_ids = dfg.instruction_results(instruction_id);
let destination_variable = self.variables.define_variable(
self.function_context,
Expand All @@ -806,6 +806,8 @@
let index_variable = self.convert_ssa_single_addr_value(*index, dfg);

if dfg.is_constant(*index) {
// For constant indices it must be the case that they have been offseted during SSA

Check warning on line 809 in compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (offseted)
assert!(*offset != ArrayGetOffset::None);
self.brillig_context.codegen_load_with_offset(
array_variable.extract_register(),
index_variable,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ impl DependencyContext {
// For array get operations, we check the Brillig calls for
// results involving the array in question, to properly
// populate the array element tainted sets
Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array, index, offset: _ } => {
self.process_array_get(*array, *index, &results, function);
// Record all the used arguments as parents of the results
self.update_children(&arguments, &results);
Expand Down
16 changes: 14 additions & 2 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use super::{
basic_block::BasicBlock,
dfg::{GlobalsGraph, InsertInstructionResult},
function::RuntimeType,
instruction::{ConstrainError, InstructionId, Intrinsic},
instruction::{ArrayGetOffset, ConstrainError, InstructionId, Intrinsic},
types::NumericType,
},
opt::pure::FunctionPurities,
Expand Down Expand Up @@ -352,9 +352,21 @@ impl FunctionBuilder {
array: ValueId,
index: ValueId,
element_type: Type,
) -> ValueId {
self.insert_array_get_with_offset(array, index, ArrayGetOffset::None, element_type)
}

/// Insert an instruction to extract an element from an array
pub fn insert_array_get_with_offset(
&mut self,
array: ValueId,
index: ValueId,
offset: ArrayGetOffset,
element_type: Type,
) -> ValueId {
let element_type = Some(vec![element_type]);
self.insert_instruction(Instruction::ArrayGet { array, index }, element_type).first()
self.insert_instruction(Instruction::ArrayGet { array, index, offset }, element_type)
.first()
}

/// Insert an instruction to create a new array with the given index replaced with a new value
Expand Down
8 changes: 5 additions & 3 deletions compiler/noirc_evaluator/src/ssa/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
ir::{
dfg::DataFlowGraph,
function::{Function, FunctionId, RuntimeType},
instruction::{Binary, BinaryOp, Instruction, TerminatorInstruction},
instruction::{ArrayGetOffset, Binary, BinaryOp, Instruction, TerminatorInstruction},
types::Type,
value::ValueId,
},
Expand Down Expand Up @@ -443,8 +443,8 @@
self.side_effects_enabled = self.lookup_bool(*condition, "enable_side_effects")?;
Ok(())
}
Instruction::ArrayGet { array, index } => {
self.interpret_array_get(*array, *index, results[0])
Instruction::ArrayGet { array, index, offset } => {
self.interpret_array_get(*array, *index, *offset, results[0])
}
Instruction::ArraySet { array, index, value, mutable } => {
self.interpret_array_set(*array, *index, *value, *mutable, results[0])
Expand Down Expand Up @@ -736,11 +736,13 @@
&mut self,
array: ValueId,
index: ValueId,
offset: ArrayGetOffset,
result: ValueId,
) -> IResult<()> {
let element = if self.side_effects_enabled() {
let array = self.lookup_array_or_slice(array, "array get")?;
let index = self.lookup_u32(index, "array get index")?;
let index = index - offset.to_u32();
array.elements.borrow()[index as usize].clone()
} else {
let typ = self.dfg().type_of_value(result);
Expand Down Expand Up @@ -873,7 +875,7 @@
}
}

macro_rules! apply_int_binop {

Check warning on line 878 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (binop)
($lhs:expr, $rhs:expr, $binary:expr, $f:expr) => {{
use value::NumericValue::*;
match ($lhs, $rhs) {
Expand Down Expand Up @@ -904,7 +906,7 @@
}};
}

macro_rules! apply_int_binop_opt {

Check warning on line 909 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (binop)
($dfg:expr, $lhs:expr, $rhs:expr, $binary:expr, $f:expr) => {{
use value::NumericValue::*;

Expand Down Expand Up @@ -1004,7 +1006,7 @@
}));
}

// Disable this instruction if it is side-effectful and side effects are disabled.

Check warning on line 1009 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (effectful)
if !self.side_effects_enabled() && binary.requires_acir_gen_predicate(self.dfg()) {
let zero = NumericValue::zero(lhs.get_type());
return Ok(Value::Numeric(zero));
Expand All @@ -1021,13 +1023,13 @@
let dfg = self.dfg();
let result = match binary.operator {
BinaryOp::Add { unchecked: false } => {
apply_int_binop_opt!(dfg, lhs, rhs, binary, num_traits::CheckedAdd::checked_add)

Check warning on line 1026 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (binop)
}
BinaryOp::Add { unchecked: true } => {
apply_int_binop!(lhs, rhs, binary, num_traits::WrappingAdd::wrapping_add)

Check warning on line 1029 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (binop)
}
BinaryOp::Sub { unchecked: false } => {
apply_int_binop_opt!(dfg, lhs, rhs, binary, num_traits::CheckedSub::checked_sub)

Check warning on line 1032 in compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (binop)
}
BinaryOp::Sub { unchecked: true } => {
apply_int_binop!(lhs, rhs, binary, num_traits::WrappingSub::wrapping_sub)
Expand Down
15 changes: 15 additions & 0 deletions compiler/noirc_evaluator/src/ssa/interpreter/tests/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,21 @@ fn array_get() {
assert_eq!(value, from_constant(2_u32.into(), NumericType::NativeField));
}

#[test]
fn array_get_with_offset() {
let value = expect_value(
r#"
acir(inline) fn main f0 {
b0():
v0 = make_array [Field 1, Field 2] : [Field; 2]
v1 = array_get v0, index u32 4 minus 3 -> Field
return v1
}
"#,
);
assert_eq!(value, from_constant(2_u32.into(), NumericType::NativeField));
}

#[test]
fn array_get_disabled_by_enable_side_effects() {
let value = expect_value(
Expand Down
8 changes: 4 additions & 4 deletions compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::ssa::{
ir::{
basic_block::BasicBlockId,
instruction::{
Binary, BinaryOp, Instruction,
ArrayGetOffset, Binary, BinaryOp, Instruction,
binary::{truncate, truncate_field},
},
types::Type,
Expand Down Expand Up @@ -110,7 +110,7 @@ pub(crate) fn simplify(
}
}
Instruction::ConstrainNotEqual(..) => None,
Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array, index, offset: _ } => {
if let Some(index) = dfg.get_numeric_constant(*index) {
try_optimize_array_get_from_previous_set(dfg, *array, index)
} else {
Expand Down Expand Up @@ -375,8 +375,8 @@ fn try_optimize_array_set_from_previous_get(
target_value: ValueId,
) -> SimplifyResult {
let array_from_get = match dfg.get_local_or_global_instruction(target_value) {
Some(Instruction::ArrayGet { array, index }) => {
if *array == array_id && *index == target_index {
Some(Instruction::ArrayGet { array, index, offset }) => {
if *offset == ArrayGetOffset::None && *array == array_id && *index == target_index {
// If array and index match from the value, we can immediately simplify
return SimplifyResult::SimplifiedTo(array_id);
} else if *index == target_index {
Expand Down
9 changes: 6 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::ssa::{
ir::{
basic_block::BasicBlockId,
dfg::DataFlowGraph,
instruction::{Binary, BinaryOp, Endian, Hint, Instruction, Intrinsic},
instruction::{ArrayGetOffset, Binary, BinaryOp, Endian, Hint, Instruction, Intrinsic},
types::{NumericType, Type},
value::{Value, ValueId},
},
Expand Down Expand Up @@ -554,8 +554,11 @@ fn simplify_slice_pop_back(
// We must pop multiple elements in the case of a slice of tuples
// Iterating through element types in reverse here since we're popping from the end
for element_type in element_types.iter().rev() {
let get_last_elem_instr =
Instruction::ArrayGet { array: arguments[1], index: flattened_len };
let get_last_elem_instr = Instruction::ArrayGet {
array: arguments[1],
index: flattened_len,
offset: ArrayGetOffset::None,
};

let element_type = Some(vec![element_type.clone()]);
let get_last_elem = dfg
Expand Down
44 changes: 38 additions & 6 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@
EnableSideEffectsIf { condition: ValueId },

/// Retrieve a value from an array at the given index
ArrayGet { array: ValueId, index: ValueId },
/// `offset` determines whether the index has been offseted by some offset.

Check warning on line 283 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (offseted)
ArrayGet { array: ValueId, index: ValueId, offset: ArrayGetOffset },

/// Creates a new array with the new value at the given index. All other elements are identical
/// to those in the given array. This will not modify the original array unless `mutable` is
Expand Down Expand Up @@ -376,7 +377,7 @@
match self {
Instruction::Binary(binary) => binary.requires_acir_gen_predicate(dfg),

Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array, index, offset: _ } => {
// `ArrayGet`s which read from "known good" indices from an array should not need a predicate.
!dfg.is_safe_index(*index, *array)
}
Expand Down Expand Up @@ -477,8 +478,8 @@
Instruction::EnableSideEffectsIf { condition } => {
Instruction::EnableSideEffectsIf { condition: f(*condition) }
}
Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array: f(*array), index: f(*index) }
Instruction::ArrayGet { array, index, offset } => {
Instruction::ArrayGet { array: f(*array), index: f(*index), offset: *offset }
}
Instruction::ArraySet { array, index, value, mutable } => Instruction::ArraySet {
array: f(*array),
Expand Down Expand Up @@ -548,7 +549,7 @@
Instruction::EnableSideEffectsIf { condition } => {
*condition = f(*condition);
}
Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array, index, offset: _ } => {
*array = f(*array);
*index = f(*index);
}
Expand Down Expand Up @@ -614,7 +615,7 @@
f(*value);
}
Instruction::Allocate => (),
Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array, index, offset: _ } => {
f(*array);
f(*index);
}
Expand Down Expand Up @@ -647,6 +648,37 @@
}
}

/// Determines whether an ArrayGet index has been offseted by a given value.

Check warning on line 651 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (offseted)
/// Offsets are set during `crate::ssa::opt::brillig_array_gets` for brillig arrays
/// and vectors with constant indicces.

Check warning on line 653 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (indicces)
#[derive(Debug, PartialEq, Eq, Hash, Copy, Clone, Serialize, Deserialize)]
pub enum ArrayGetOffset {
None,
Array,
Slice,
}

impl ArrayGetOffset {
pub fn from_u32(value: u32) -> Option<Self> {
match value {
0 => Some(Self::None),
1 => Some(Self::Array),
3 => Some(Self::Slice),
_ => None,
}
}

pub fn to_u32(self) -> u32 {
match self {
Self::None => 0,
// Arrays in brillig are represented as [RC, ...items]
Self::Array => 1,
// Slices in brillig are represented as [RC, Size, Capacity
Self::Slice => 3,
}
}
}

impl Binary {
pub(crate) fn requires_acir_gen_predicate(&self, dfg: &DataFlowGraph) -> bool {
match self.operator {
Expand Down
14 changes: 11 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ use iter_extended::vecmap;

use crate::ssa::{
Ssa,
ir::types::{NumericType, Type},
ir::{
instruction::ArrayGetOffset,
types::{NumericType, Type},
},
};

use super::{
Expand Down Expand Up @@ -230,12 +233,17 @@ fn display_instruction_inner(
Instruction::EnableSideEffectsIf { condition } => {
writeln!(f, "enable_side_effects {}", show(*condition))
}
Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array, index, offset } => {
writeln!(
f,
"array_get {}, index {}{}",
"array_get {}, index {}{}{}",
show(*array),
show(*index),
match offset {
ArrayGetOffset::None => String::new(),
ArrayGetOffset::Array | ArrayGetOffset::Slice =>
format!(" minus {}", offset.to_u32()),
},
result_types(dfg, results)
)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/basic_conditional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ fn block_cost(block: BasicBlockId, dfg: &DataFlowGraph) -> u32 {
| Instruction::Store { .. }
| Instruction::ArraySet { .. } => return u32::MAX,

Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array, index, offset: _ } => {
// A get can fail because of out-of-bound index
let mut in_bound = false;
// check if index is in bound
Expand Down
21 changes: 11 additions & 10 deletions compiler/noirc_evaluator/src/ssa/opt/brillig_array_gets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
Ssa,
ir::{
function::Function,
instruction::Instruction,
instruction::{ArrayGetOffset, Instruction},
types::{NumericType, Type},
},
},
Expand All @@ -39,10 +39,13 @@ impl Function {
pub(super) fn brillig_array_gets(&mut self) {
self.simple_reachable_blocks_optimization(|context| {
let instruction = context.instruction();
let Instruction::ArrayGet { array, index } = instruction else {
let Instruction::ArrayGet { array, index, offset } = instruction else {
return;
};

// This pass should run at most once
assert!(*offset == ArrayGetOffset::None);

let array = *array;
let index = *index;
if !context.dfg.is_constant(index) {
Expand All @@ -52,17 +55,15 @@ impl Function {
let index_constant =
context.dfg.get_numeric_constant(index).expect("ICE: Expected constant index");
let offset = if matches!(context.dfg.type_of_value(array), Type::Array(..)) {
// Brillig arrays are [RC, ...items]
1u128
ArrayGetOffset::Array
} else {
// Brillig vectors are [RC, Size, Capacity, ...items]
3u128
ArrayGetOffset::Slice
};
let index = context.dfg.make_constant(
index_constant + offset.into(),
index_constant + offset.to_u32().into(),
NumericType::unsigned(BRILLIG_MEMORY_ADDRESSING_BIT_SIZE),
);
let new_instruction = Instruction::ArrayGet { array, index };
let new_instruction = Instruction::ArrayGet { array, index, offset };
context.replace_current_instruction_with(new_instruction);
});
}
Expand Down Expand Up @@ -90,7 +91,7 @@ mod tests {
assert_ssa_snapshot!(ssa, @r"
brillig(inline) fn main f0 {
b0(v0: [Field; 3]):
v2 = array_get v0, index u32 1 -> Field
v2 = array_get v0, index u32 1 minus 1 -> Field
return v2
}
");
Expand Down Expand Up @@ -142,7 +143,7 @@ mod tests {
assert_ssa_snapshot!(ssa, @r"
brillig(inline) fn main f0 {
b0(v0: [Field]):
v2 = array_get v0, index u32 3 -> Field
v2 = array_get v0, index u32 3 minus 3 -> Field
return v2
}
");
Expand Down
Loading
Loading