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: support for conditional stores #2553

Merged
merged 13 commits into from
Sep 13, 2023
104 changes: 70 additions & 34 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,37 @@ impl Context {
}
};

// We compute some AcirVars:
// - index_var is the index of the array
// - predicate_index is 0, or the index if the predicate is true
// - new_value is the optional value when the operation is an array_set
// When there is a predicate, it is predicate*value + (1-predicate)*dummy, where dummy is the value of the array at the requested index.
// It is a dummy value because in the case of a false predicate, the value stored at the requested index will be itself.
let index_const = dfg.get_numeric_constant(index);
let index_var = self.convert_numeric_value(index, dfg)?;
let predicate_index =
self.acir_context.mul_var(index_var, self.current_side_effects_enabled_var)?;
let new_value = if let Some(store) = store_value {
let store_var = Some(self.convert_value(store, dfg).into_var()?);
if self.acir_context.is_constant_one(&self.current_side_effects_enabled_var) {
store_var
} else {
let dummy = self.array_get(instruction, array, predicate_index, dfg)?;
let true_pred = self
.acir_context
.mul_var(store_var.unwrap(), self.current_side_effects_enabled_var)?;
let one = self.acir_context.add_constant(FieldElement::one());
let not_pred =
self.acir_context.sub_var(one, self.current_side_effects_enabled_var)?;
let false_pred = self.acir_context.mul_var(not_pred, dummy)?;
// predicate*value + (1-predicate)*dummy
Some(self.acir_context.add_var(true_pred, false_pred)?)
}
} else {
None
};

// Handle constant index: if there is no predicate and we have the array values, we can perform the operation directly on the array
match dfg.type_of_value(array) {
Type::Array(_, _) => {
match self.convert_value(array, dfg) {
Expand All @@ -571,36 +600,36 @@ impl Context {
});
}
};
if index >= array_size {
// Ignore the error if side effects are disabled.
if self
.acir_context
.is_constant_one(&self.current_side_effects_enabled_var)
{
if self
.acir_context
.is_constant_one(&self.current_side_effects_enabled_var)
{
// Report the error if side effects are enabled.
if index >= array_size {
let call_stack = self.acir_context.get_call_stack();
return Err(RuntimeError::IndexOutOfBounds {
index,
array_size,
call_stack,
});
} else {
let value = match store_value {
Some(store_value) => {
let store_value = self.convert_value(store_value, dfg);
AcirValue::Array(array.update(index, store_value))
}
None => array[index].clone(),
};

self.define_result(dfg, instruction, value);
return Ok(());
}
let result_type =
dfg.type_of_value(dfg.instruction_results(instruction)[0]);
let value = self.create_default_value(&result_type)?;
self.define_result(dfg, instruction, value);
}
// If there is a predicate and the index is not out of range, we can directly perform the read
else if index < array_size && store_value.is_none() {
self.define_result(dfg, instruction, array[index].clone());
return Ok(());
}

let value = match store_value {
Some(store_value) => {
let store_value = self.convert_value(store_value, dfg);
AcirValue::Array(array.update(index, store_value))
}
None => array[index].clone(),
};

self.define_result(dfg, instruction, value);
return Ok(());
}
}
AcirValue::DynamicArray(_) => (),
Expand All @@ -612,12 +641,20 @@ impl Context {
_ => unreachable!("ICE: expected array or slice type"),
}

let new_index = if self.acir_context.is_constant_one(&self.current_side_effects_enabled_var)
{
index_var
} else {
predicate_index
};

let resolved_array = dfg.resolve(array);
let map_array = last_array_uses.get(&resolved_array) == Some(&instruction);
if store_value.is_some() {
self.array_set(instruction, dfg, map_array)?;

if let Some(new_value) = new_value {
self.array_set(instruction, new_index, new_value, dfg, map_array)?;
} else {
self.array_get(instruction, array, index, dfg)?;
self.array_get(instruction, array, new_index, dfg)?;
}

Ok(())
Expand All @@ -628,9 +665,9 @@ impl Context {
&mut self,
instruction: InstructionId,
array: ValueId,
index: ValueId,
var_index: AcirVar,
dfg: &DataFlowGraph,
) -> Result<(), RuntimeError> {
) -> Result<AcirVar, RuntimeError> {
let array = dfg.resolve(array);
let block_id = self.block_id(&array);
if !self.initialized_arrays.contains(&block_id) {
Expand All @@ -649,8 +686,7 @@ impl Context {
}
}

let index_var = self.convert_value(index, dfg).into_var()?;
let read = self.acir_context.read_from_memory(block_id, &index_var)?;
let read = self.acir_context.read_from_memory(block_id, &var_index)?;
let typ = match dfg.type_of_value(array) {
Type::Array(typ, _) => {
if typ.len() != 1 {
Expand All @@ -674,7 +710,7 @@ impl Context {
};
let typ = AcirType::from(typ);
self.define_result(dfg, instruction, AcirValue::Var(read, typ));
Ok(())
Ok(read)
}

/// Copy the array and generates a write opcode on the new array
Expand All @@ -683,12 +719,14 @@ impl Context {
fn array_set(
&mut self,
instruction: InstructionId,
var_index: AcirVar,
store_value: AcirVar,
dfg: &DataFlowGraph,
map_array: bool,
) -> Result<(), InternalError> {
// Pass the instruction between array methods rather than the internal fields themselves
let (array, index, store_value, length) = match dfg[instruction] {
Instruction::ArraySet { array, index, value, length } => (array, index, value, length),
let (array, length) = match dfg[instruction] {
Instruction::ArraySet { array, length, .. } => (array, length),
_ => {
return Err(InternalError::UnExpected {
expected: "Instruction should be an ArraySet".to_owned(),
Expand Down Expand Up @@ -766,9 +804,7 @@ impl Context {
}

// Write the new value into the new array at the specified index
let index_var = self.convert_value(index, dfg).into_var()?;
let value_var = self.convert_value(store_value, dfg).into_var()?;
self.acir_context.write_to_memory(result_block_id, &index_var, &value_var)?;
self.acir_context.write_to_memory(result_block_id, &var_index, &store_value)?;

let result_value =
AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, len });
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_mem_op_predicate"
type = "bin"
authors = [""]
compiler_version = "0.1"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = [4,3,1,5,111]
idx = 12
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main(mut x: [u32; 5], idx: Field) {
// We should not hit out of bounds here as we have a predicate
// that should not be hit
if idx as u32 < 3 {
x[idx] = 10;
}
assert(x[4] == 111);
}
Loading