Skip to content

feat: no need to use dummy slice values in remove_if_else #9928

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.

Sign up for GitHub

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

Merged
asterite merged 1 commit into from
Sep 22, 2025
Merged

feat: no need to use dummy slice values in remove_if_else #9928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
68 changes: 23 additions & 45 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use acvm::{FieldElement, acir::AcirField};
use noirc_errors::call_stack::CallStackId;
use rustc_hash::FxHashMap as HashMap;

Expand Down Expand Up @@ -214,6 +213,9 @@ impl<'a> ValueMerger<'a> {

let len = then_len.max(else_len);

let flattened_then_length = then_len * element_types.len() as u32;
let flattened_else_length = else_len * element_types.len() as u32;

for i in 0..len {
for (element_index, element_type) in element_types.iter().enumerate() {
let index_u32 = i * element_types.len() as u32 + element_index as u32;
Expand All @@ -223,10 +225,8 @@ impl<'a> ValueMerger<'a> {
let typevars = Some(vec![element_type.clone()]);

let mut get_element = |array, typevars, len| {
// The smaller slice is filled with placeholder data. Codegen for slice accesses must
// include checks against the dynamic slice length so that this placeholder data is not incorrectly accessed.
if len <= index_u32 {
self.make_slice_dummy_data(element_type)
panic!("get_element invoked with an out of bounds index");
} else {
let offset = ArrayOffset::None;
let get = Instruction::ArrayGet { array, index, offset };
Expand All @@ -240,11 +240,26 @@ impl<'a> ValueMerger<'a> {
}
};

let len = then_len * element_types.len() as u32;
let then_element = get_element(then_value_id, typevars.clone(), len);
// If it's out of bounds for the "then" slice, a value in the "else" *must* exist.
// We can use that value directly as accessing it is always checked against the actual
// slice length.
if index_u32 >= flattened_then_length {
let else_element = get_element(else_value_id, typevars, flattened_else_length);
merged.push_back(else_element);
continue;
}

// Same for if it's out of bounds for the "else" slice.
if index_u32 >= flattened_else_length {
let then_element =
get_element(then_value_id, typevars.clone(), flattened_then_length);
merged.push_back(then_element);
continue;
}

let len = else_len * element_types.len() as u32;
let else_element = get_element(else_value_id, typevars, len);
let then_element =
get_element(then_value_id, typevars.clone(), flattened_then_length);
let else_element = get_element(else_value_id, typevars, flattened_else_length);

merged.push_back(self.merge_values(
then_condition,
Expand All @@ -261,41 +276,4 @@ impl<'a> ValueMerger<'a> {
self.dfg.insert_instruction_and_results(instruction, self.block, None, call_stack);
Ok(result.first())
}

/// Construct a dummy value to be attached to the smaller of two slices being merged.
/// We need to make sure we follow the internal element type structure of the slice type
/// even for dummy data to ensure that we do not have errors later in the compiler,
/// such as with dynamic indexing of non-homogenous slices.
fn make_slice_dummy_data(&mut self, typ: &Type) -> ValueId {
match typ {
Type::Numeric(numeric_type) => {
let zero = FieldElement::zero();
self.dfg.make_constant(zero, *numeric_type)
}
Type::Array(element_types, len) => {
let mut array = im::Vector::new();
for _ in 0..*len {
for typ in element_types.iter() {
array.push_back(self.make_slice_dummy_data(typ));
}
}
let instruction = Instruction::MakeArray { elements: array, typ: typ.clone() };
let call_stack = self.call_stack;
self.dfg
.insert_instruction_and_results(instruction, self.block, None, call_stack)
.first()
}
Type::Slice(_) => {
// TODO(#3188): Need to update flattening to use true user facing length of slices
// to accurately construct dummy data
unreachable!("ICE: Cannot return a slice of slices from an if expression")
}
Type::Reference(_) => {
unreachable!("ICE: Merging references is unsupported")
}
Type::Function => {
unreachable!("ICE: Merging functions is unsupported")
}
}
}
}
43 changes: 20 additions & 23 deletions compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,30 +554,27 @@ acir(inline) impure fn main f0 {
v10 = not v0
v11 = cast v0 as u32
v13 = array_get v9, index u32 0 -> Field
v14 = cast v0 as Field
v15 = cast v10 as Field
v16 = mul v14, v13
v17 = make_array [v16] : [Field]
v14 = make_array [v13] : [Field]
enable_side_effects u1 1
v20 = eq v11, u32 1
v21 = not v20
v22 = add v11, u32 1
v23 = make_array [v16, v2] : [Field]
v24 = array_set v23, index v11, value v2
v25 = array_get v24, index u32 0 -> Field
v26 = cast v21 as Field
v27 = cast v20 as Field
v28 = mul v26, v25
v29 = mul v27, v16
v30 = add v28, v29
v31 = array_get v24, index u32 1 -> Field
v32 = cast v21 as Field
v33 = cast v20 as Field
v34 = mul v32, v31
v35 = mul v33, v2
v36 = add v34, v35
v37 = make_array [v30, v36] : [Field]
constrain v30 == Field 1
v17 = eq v11, u32 1
v18 = not v17
v19 = add v11, u32 1
v20 = make_array [v13, v2] : [Field]
v21 = array_set v20, index v11, value v2
v22 = array_get v21, index u32 0 -> Field
v23 = cast v18 as Field
v24 = cast v17 as Field
v25 = mul v23, v22
v26 = mul v24, v13
v27 = add v25, v26
v28 = array_get v21, index u32 1 -> Field
v29 = cast v18 as Field
v30 = cast v17 as Field
v31 = mul v29, v28
v32 = mul v30, v2
v33 = add v31, v32
v34 = make_array [v27, v33] : [Field]
constrain v27 == Field 1
return
}
");
Expand Down
Loading