Skip to content

Commit 0524be5

Browse files
TomAFrenchTomAFrench
authored
Merge d6c0198 into f291e37 
2 parents f291e37 + d6c0198 commit 0524be5

File tree

6 files changed

+57
-110
lines changed

6 files changed

+57
-110
lines changed

‎compiler/noirc_evaluator/src/ssa/ir/instruction.rs

+8-22
Original file line numberDiff line numberDiff line change
@@ -273,15 +273,7 @@ pub(crate) enum Instruction {
273273
/// else_value
274274
/// }
275275
/// ```
276-
///
277-
/// Where we save the result of !then_condition so that we have the same
278-
/// ValueId for it each time.
279-
IfElse {
280-
then_condition: ValueId,
281-
then_value: ValueId,
282-
else_condition: ValueId,
283-
else_value: ValueId,
284-
},
276+
IfElse { then_condition: ValueId, then_value: ValueId, else_value: ValueId },
285277

286278
/// Creates a new array or slice.
287279
///
@@ -536,14 +528,11 @@ impl Instruction {
536528
assert_message: assert_message.clone(),
537529
}
538530
}
539-
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
540-
Instruction::IfElse {
541-
then_condition: f(*then_condition),
542-
then_value: f(*then_value),
543-
else_condition: f(*else_condition),
544-
else_value: f(*else_value),
545-
}
546-
}
531+
Instruction::IfElse { then_condition, then_value, else_value } => Instruction::IfElse {
532+
then_condition: f(*then_condition),
533+
then_value: f(*then_value),
534+
else_value: f(*else_value),
535+
},
547536
Instruction::MakeArray { elements, typ } => Instruction::MakeArray {
548537
elements: elements.iter().copied().map(f).collect(),
549538
typ: typ.clone(),
@@ -602,10 +591,9 @@ impl Instruction {
602591
| Instruction::RangeCheck { value, .. } => {
603592
f(*value);
604593
}
605-
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
594+
Instruction::IfElse { then_condition, then_value, else_value } => {
606595
f(*then_condition);
607596
f(*then_value);
608-
f(*else_condition);
609597
f(*else_value);
610598
}
611599
Instruction::MakeArray { elements, typ: _ } => {
@@ -768,7 +756,7 @@ impl Instruction {
768756
None
769757
}
770758
}
771-
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
759+
Instruction::IfElse { then_condition, then_value, else_value } => {
772760
let typ = dfg.type_of_value(*then_value);
773761

774762
if let Some(constant) = dfg.get_numeric_constant(*then_condition) {
@@ -787,13 +775,11 @@ impl Instruction {
787775

788776
if matches!(&typ, Type::Numeric(_)) {
789777
let then_condition = *then_condition;
790-
let else_condition = *else_condition;
791778

792779
let result = ValueMerger::merge_numeric_values(
793780
dfg,
794781
block,
795782
then_condition,
796-
else_condition,
797783
then_value,
798784
else_value,
799785
);

‎compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -443,12 +443,8 @@ fn simplify_slice_push_back(
443443
let mut value_merger =
444444
ValueMerger::new(dfg, block, &mut slice_sizes, unknown, None, call_stack);
445445

446-
let new_slice = value_merger.merge_values(
447-
len_not_equals_capacity,
448-
len_equals_capacity,
449-
set_last_slice_value,
450-
new_slice,
451-
);
446+
let new_slice =
447+
value_merger.merge_values(len_not_equals_capacity, set_last_slice_value, new_slice);
452448

453449
SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice])
454450
}

‎compiler/noirc_evaluator/src/ssa/ir/printer.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -210,15 +210,11 @@ fn display_instruction_inner(
210210
Instruction::RangeCheck { value, max_bit_size, .. } => {
211211
writeln!(f, "range_check {} to {} bits", show(*value), *max_bit_size,)
212212
}
213-
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
213+
Instruction::IfElse { then_condition, then_value, else_value } => {
214214
let then_condition = show(*then_condition);
215215
let then_value = show(*then_value);
216-
let else_condition = show(*else_condition);
217216
let else_value = show(*else_value);
218-
writeln!(
219-
f,
220-
"if {then_condition} then {then_value} else if {else_condition} then {else_value}"
221-
)
217+
writeln!(f, "if {then_condition} then {then_value} else {else_value}")
222218
}
223219
Instruction::MakeArray { elements, typ } => {
224220
write!(f, "make_array [")?;

‎compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs

+36-43
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,6 @@ impl<'f> Context<'f> {
519519
let instruction = Instruction::IfElse {
520520
then_condition: cond_context.then_branch.condition,
521521
then_value: then_arg,
522-
else_condition: cond_context.else_branch.as_ref().unwrap().condition,
523522
else_value: else_arg,
524523
};
525524
let call_stack = cond_context.call_stack.clone();
@@ -667,13 +666,10 @@ impl<'f> Context<'f> {
667666
)
668667
.first();
669668

670-
let not = Instruction::Not(condition);
671-
let else_condition = self.insert_instruction(not, call_stack.clone());
672-
673669
let instruction = Instruction::IfElse {
674670
then_condition: condition,
675671
then_value: value,
676-
else_condition,
672+
677673
else_value: previous_value,
678674
};
679675

@@ -907,13 +903,12 @@ mod test {
907903
b0(v0: u1, v1: &mut Field):
908904
enable_side_effects v0
909905
v2 = load v1 -> Field
910-
v3 = not v0
911-
v4 = cast v0 as Field
912-
v6 = sub Field 5, v2
913-
v7 = mul v4, v6
914-
v8 = add v2, v7
915-
store v8 at v1
916-
v9 = not v0
906+
v3 = cast v0 as Field
907+
v5 = sub Field 5, v2
908+
v6 = mul v3, v5
909+
v7 = add v2, v6
910+
store v7 at v1
911+
v8 = not v0
917912
enable_side_effects u1 1
918913
return
919914
}
@@ -945,20 +940,19 @@ mod test {
945940
b0(v0: u1, v1: &mut Field):
946941
enable_side_effects v0
947942
v2 = load v1 -> Field
948-
v3 = not v0
949-
v4 = cast v0 as Field
950-
v6 = sub Field 5, v2
951-
v7 = mul v4, v6
952-
v8 = add v2, v7
953-
store v8 at v1
954-
v9 = not v0
955-
enable_side_effects v9
956-
v10 = load v1 -> Field
957-
v11 = cast v9 as Field
958-
v13 = sub Field 6, v10
959-
v14 = mul v11, v13
960-
v15 = add v10, v14
961-
store v15 at v1
943+
v3 = cast v0 as Field
944+
v5 = sub Field 5, v2
945+
v6 = mul v3, v5
946+
v7 = add v2, v6
947+
store v7 at v1
948+
v8 = not v0
949+
enable_side_effects v8
950+
v9 = load v1 -> Field
951+
v10 = cast v8 as Field
952+
v12 = sub Field 6, v9
953+
v13 = mul v10, v12
954+
v14 = add v9, v13
955+
store v14 at v1
962956
enable_side_effects u1 1
963957
return
964958
}
@@ -1306,24 +1300,23 @@ mod test {
13061300
v9 = add v7, Field 1
13071301
v10 = cast v9 as u8
13081302
v11 = load v6 -> u8
1309-
v12 = not v5
1310-
v13 = cast v4 as Field
1311-
v14 = cast v11 as Field
1312-
v15 = sub v9, v14
1313-
v16 = mul v13, v15
1314-
v17 = add v14, v16
1315-
v18 = cast v17 as u8
1316-
store v18 at v6
1317-
v19 = not v5
1318-
enable_side_effects v19
1319-
v20 = load v6 -> u8
1303+
v12 = cast v4 as Field
1304+
v13 = cast v11 as Field
1305+
v14 = sub v9, v13
1306+
v15 = mul v12, v14
1307+
v16 = add v13, v15
1308+
v17 = cast v16 as u8
1309+
store v17 at v6
1310+
v18 = not v5
1311+
enable_side_effects v18
1312+
v19 = load v6 -> u8
1313+
v20 = cast v18 as Field
13201314
v21 = cast v19 as Field
1321-
v22 = cast v20 as Field
1322-
v24 = sub Field 0, v22
1323-
v25 = mul v21, v24
1324-
v26 = add v22, v25
1325-
v27 = cast v26 as u8
1326-
store v27 at v6
1315+
v23 = sub Field 0, v21
1316+
v24 = mul v20, v23
1317+
v25 = add v21, v24
1318+
v26 = cast v25 as u8
1319+
store v26 at v6
13271320
enable_side_effects u1 1
13281321
constrain v5 == u1 1
13291322
return

‎compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs

+7-25
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ impl<'a> ValueMerger<'a> {
4545

4646
/// Merge two values a and b from separate basic blocks to a single value.
4747
/// If these two values are numeric, the result will be
48-
/// `then_condition * then_value + else_condition * else_value`.
48+
/// `then_condition * (then_value - else_value) + else_value`.
4949
/// Otherwise, if the values being merged are arrays, a new array will be made
5050
/// recursively from combining each element of both input arrays.
5151
///
@@ -54,7 +54,6 @@ impl<'a> ValueMerger<'a> {
5454
pub(crate) fn merge_values(
5555
&mut self,
5656
then_condition: ValueId,
57-
else_condition: ValueId,
5857
then_value: ValueId,
5958
else_value: ValueId,
6059
) -> ValueId {
@@ -70,28 +69,26 @@ impl<'a> ValueMerger<'a> {
7069
self.dfg,
7170
self.block,
7271
then_condition,
73-
else_condition,
7472
then_value,
7573
else_value,
7674
),
7775
typ @ Type::Array(_, _) => {
78-
self.merge_array_values(typ, then_condition, else_condition, then_value, else_value)
76+
self.merge_array_values(typ, then_condition, then_value, else_value)
7977
}
8078
typ @ Type::Slice(_) => {
81-
self.merge_slice_values(typ, then_condition, else_condition, then_value, else_value)
79+
self.merge_slice_values(typ, then_condition, then_value, else_value)
8280
}
8381
Type::Reference(_) => panic!("Cannot return references from an if expression"),
8482
Type::Function => panic!("Cannot return functions from an if expression"),
8583
}
8684
}
8785

8886
/// Merge two numeric values a and b from separate basic blocks to a single value. This
89-
/// function would return the result of `if c { a } else { b }` as `c*a + (!c)*b`.
87+
/// function would return the result of `if c { a } else { b }` as `c * (a-b) + b`.
9088
pub(crate) fn merge_numeric_values(
9189
dfg: &mut DataFlowGraph,
9290
block: BasicBlockId,
9391
then_condition: ValueId,
94-
_else_condition: ValueId,
9592
then_value: ValueId,
9693
else_value: ValueId,
9794
) -> ValueId {
@@ -155,7 +152,6 @@ impl<'a> ValueMerger<'a> {
155152
&mut self,
156153
typ: Type,
157154
then_condition: ValueId,
158-
else_condition: ValueId,
159155
then_value: ValueId,
160156
else_value: ValueId,
161157
) -> ValueId {
@@ -170,7 +166,6 @@ impl<'a> ValueMerger<'a> {
170166

171167
if let Some(result) = self.try_merge_only_changed_indices(
172168
then_condition,
173-
else_condition,
174169
then_value,
175170
else_value,
176171
actual_length,
@@ -200,12 +195,7 @@ impl<'a> ValueMerger<'a> {
200195
let then_element = get_element(then_value, typevars.clone());
201196
let else_element = get_element(else_value, typevars);
202197

203-
merged.push_back(self.merge_values(
204-
then_condition,
205-
else_condition,
206-
then_element,
207-
else_element,
208-
));
198+
merged.push_back(self.merge_values(then_condition, then_element, else_element));
209199
}
210200
}
211201

@@ -218,7 +208,6 @@ impl<'a> ValueMerger<'a> {
218208
&mut self,
219209
typ: Type,
220210
then_condition: ValueId,
221-
else_condition: ValueId,
222211
then_value_id: ValueId,
223212
else_value_id: ValueId,
224213
) -> ValueId {
@@ -276,12 +265,7 @@ impl<'a> ValueMerger<'a> {
276265
let else_element =
277266
get_element(else_value_id, typevars, else_len * element_types.len());
278267

279-
merged.push_back(self.merge_values(
280-
then_condition,
281-
else_condition,
282-
then_element,
283-
else_element,
284-
));
268+
merged.push_back(self.merge_values(then_condition, then_element, else_element));
285269
}
286270
}
287271

@@ -330,7 +314,6 @@ impl<'a> ValueMerger<'a> {
330314
fn try_merge_only_changed_indices(
331315
&mut self,
332316
then_condition: ValueId,
333-
else_condition: ValueId,
334317
then_value: ValueId,
335318
else_value: ValueId,
336319
array_length: usize,
@@ -414,8 +397,7 @@ impl<'a> ValueMerger<'a> {
414397
let then_element = get_element(then_value, typevars.clone());
415398
let else_element = get_element(else_value, typevars);
416399

417-
let value =
418-
self.merge_values(then_condition, else_condition, then_element, else_element);
400+
let value = self.merge_values(then_condition, then_element, else_element);
419401

420402
array = self.insert_array_set(array, index, value, Some(condition)).first();
421403
}

‎compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs

+2-8
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,9 @@ impl Context {
6666

6767
for instruction in instructions {
6868
match &function.dfg[instruction] {
69-
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
69+
Instruction::IfElse { then_condition, then_value, else_value } => {
7070
let then_condition = *then_condition;
7171
let then_value = *then_value;
72-
let else_condition = *else_condition;
7372
let else_value = *else_value;
7473

7574
let typ = function.dfg.type_of_value(then_value);
@@ -85,12 +84,7 @@ impl Context {
8584
call_stack,
8685
);
8786

88-
let value = value_merger.merge_values(
89-
then_condition,
90-
else_condition,
91-
then_value,
92-
else_value,
93-
);
87+
let value = value_merger.merge_values(then_condition, then_value, else_value);
9488

9589
let _typ = function.dfg.type_of_value(value);
9690
let results = function.dfg.instruction_results(instruction);

0 commit comments

Comments
 (0)