Skip to content
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
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),
Comment thread
jfecher marked this conversation as resolved.
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::registers::RegisterAllocator;
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 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
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 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
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
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 @@ use super::{
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 @@ impl<'ssa> Interpreter<'ssa> {
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 @@ impl<'ssa> Interpreter<'ssa> {
&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();
Comment thread
jfecher marked this conversation as resolved.
array.elements.borrow()[index as usize].clone()
} else {
let typ = self.dfg().type_of_value(result);
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 @@ pub enum Instruction {
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.
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 @@ impl Instruction {
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 @@ impl Instruction {
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 @@ impl Instruction {
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 @@ impl Instruction {
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 @@ impl Instruction {
}
}

/// Determines whether an ArrayGet index has been offseted by a given value.
/// Offsets are set during `crate::ssa::opt::brillig_array_gets` for brillig arrays
/// and vectors with constant 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, ...items]
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