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
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);
}
83 changes: 55 additions & 28 deletions crates/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,15 @@ impl Context {
// assert_eq!(result_ids.len(), outputs.len());

for (result, output) in result_ids.iter().zip(outputs) {
// when the intrinsic returns an array, we initialize it
if let AcirValue::Array(elements) = &output {
guipublic marked this conversation as resolved.
Show resolved Hide resolved
let block = self.block_id(result);
let mut values = Vec::new();
for element in elements {
values.push(element.clone());
}
jfecher marked this conversation as resolved.
Show resolved Hide resolved
self.initialize_array(block, elements.len(), Some(&values))?;
}
self.ssa_values.insert(*result, output);
}
}
Expand Down Expand Up @@ -530,6 +539,10 @@ impl Context {
last_array_uses: &HashMap<ValueId, InstructionId>,
) -> Result<(), RuntimeError> {
let index_const = dfg.get_numeric_constant(index);
// we use predicate*index instead of index if there are side effects, in order to avoid index-out-of-bound with false predicates.
guipublic marked this conversation as resolved.
Show resolved Hide resolved
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)?;

match self.convert_value(array, dfg) {
AcirValue::Var(acir_var, _) => {
Expand All @@ -554,7 +567,7 @@ impl Context {
}
};
if index >= array_size {
// Ignore the error if side effects are disabled.
// Report the error if side effects are disabled.
guipublic marked this conversation as resolved.
Show resolved Hide resolved
if self.acir_context.is_constant_one(&self.current_side_effects_enabled_var)
{
let call_stack = self.acir_context.get_call_stack();
Expand All @@ -564,33 +577,50 @@ impl Context {
call_stack,
});
}
let result_type =
dfg.type_of_value(dfg.instruction_results(instruction)[0]);
let value = self.create_default_value(&result_type)?;
} 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 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(_) => (),
}
let resolved_array = dfg.resolve(array);
let map_array = last_array_uses.get(&resolved_array) == Some(&instruction);
if let Some(store) = store_value {
self.array_set(instruction, array, index, store, dfg, map_array)?;
// In case of side effects:
// - we replace the index by predicate*index
// - we replace the value by predicate*value + (1-predicate)*dummy, where dummy is the value of the array at the requested index
guipublic marked this conversation as resolved.
Show resolved Hide resolved
let store_var = self.convert_value(store, dfg).into_var()?;
let new_value =
if !self.acir_context.is_constant_one(&self.current_side_effects_enabled_var) {
// array value at the index
let dummy = self.array_get(instruction, array, predicate_index, dfg)?;

// true_pred = predicate*value
let true_pred = self
.acir_context
.mul_var(store_var, 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)?;
// false_pred = (1-predicate)*dummy
let false_pred = self.acir_context.mul_var(not_pred, dummy)?;
self.acir_context.add_var(true_pred, false_pred)?
guipublic marked this conversation as resolved.
Show resolved Hide resolved
} else {
store_var
};
self.array_set(instruction, array, predicate_index, new_value, dfg, map_array)?;
} else {
self.array_get(instruction, array, index, dfg)?;
self.array_get(instruction, array, predicate_index, dfg)?;
}

Ok(())
Expand All @@ -601,9 +631,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 @@ -617,13 +647,12 @@ impl Context {
return Err(RuntimeError::UnInitialized {
name: "array".to_string(),
call_stack: self.acir_context.get_call_stack(),
})
});
}
}
}

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 @@ -637,7 +666,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 @@ -647,8 +676,8 @@ impl Context {
&mut self,
instruction: InstructionId,
array: ValueId,
index: ValueId,
store_value: ValueId,
var_index: AcirVar,
store_value: AcirVar,
dfg: &DataFlowGraph,
map_array: bool,
) -> Result<(), InternalError> {
Expand Down Expand Up @@ -711,9 +740,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