Skip to content

Commit 2d55b39

Browse files
TomAFrenchTomAFrench
committed
chore: remove some _else_condition tech debt
1 parent a666a1f commit 2d55b39

File tree

6 files changed

+22
-69
lines changed

6 files changed

+22
-69
lines changed

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

+8-19
Original file line numberDiff line numberDiff line change
@@ -276,12 +276,7 @@ pub(crate) enum Instruction {
276276
///
277277
/// Where we save the result of !then_condition so that we have the same
278278
/// ValueId for it each time.
279-
IfElse {
280-
then_condition: ValueId,
281-
then_value: ValueId,
282-
else_condition: ValueId,
283-
else_value: ValueId,
284-
},
279+
IfElse { then_condition: ValueId, then_value: ValueId, else_value: ValueId },
285280
}
286281

287282
impl Instruction {
@@ -523,14 +518,11 @@ impl Instruction {
523518
assert_message: assert_message.clone(),
524519
}
525520
}
526-
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
527-
Instruction::IfElse {
528-
then_condition: f(*then_condition),
529-
then_value: f(*then_value),
530-
else_condition: f(*else_condition),
531-
else_value: f(*else_value),
532-
}
533-
}
521+
Instruction::IfElse { then_condition, then_value, else_value } => Instruction::IfElse {
522+
then_condition: f(*then_condition),
523+
then_value: f(*then_value),
524+
else_value: f(*else_value),
525+
},
534526
}
535527
}
536528

@@ -585,10 +577,9 @@ impl Instruction {
585577
| Instruction::RangeCheck { value, .. } => {
586578
f(*value);
587579
}
588-
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
580+
Instruction::IfElse { then_condition, then_value, else_value } => {
589581
f(*then_condition);
590582
f(*then_value);
591-
f(*else_condition);
592583
f(*else_value);
593584
}
594585
}
@@ -738,7 +729,7 @@ impl Instruction {
738729
None
739730
}
740731
}
741-
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
732+
Instruction::IfElse { then_condition, then_value, else_value } => {
742733
let typ = dfg.type_of_value(*then_value);
743734

744735
if let Some(constant) = dfg.get_numeric_constant(*then_condition) {
@@ -757,13 +748,11 @@ impl Instruction {
757748

758749
if matches!(&typ, Type::Numeric(_)) {
759750
let then_condition = *then_condition;
760-
let else_condition = *else_condition;
761751

762752
let result = ValueMerger::merge_numeric_values(
763753
dfg,
764754
block,
765755
then_condition,
766-
else_condition,
767756
then_value,
768757
else_value,
769758
);

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

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

443-
let new_slice = value_merger.merge_values(
444-
len_not_equals_capacity,
445-
len_equals_capacity,
446-
set_last_slice_value,
447-
new_slice,
448-
);
443+
let new_slice =
444+
value_merger.merge_values(len_not_equals_capacity, set_last_slice_value, new_slice);
449445

450446
SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice])
451447
}

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

+2-6
Original file line numberDiff line numberDiff line change
@@ -221,15 +221,11 @@ fn display_instruction_inner(
221221
Instruction::RangeCheck { value, max_bit_size, .. } => {
222222
writeln!(f, "range_check {} to {} bits", show(*value), *max_bit_size,)
223223
}
224-
Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
224+
Instruction::IfElse { then_condition, then_value, else_value } => {
225225
let then_condition = show(*then_condition);
226226
let then_value = show(*then_value);
227-
let else_condition = show(*else_condition);
228227
let else_value = show(*else_value);
229-
writeln!(
230-
f,
231-
"if {then_condition} then {then_value} else if {else_condition} then {else_value}"
232-
)
228+
writeln!(f, "if {then_condition} then {then_value} else {else_value}")
233229
}
234230
}
235231
}

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

+1-5
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();
@@ -669,13 +668,10 @@ impl<'f> Context<'f> {
669668
)
670669
.first();
671670

672-
let not = Instruction::Not(condition);
673-
let else_condition = self.insert_instruction(not, call_stack.clone());
674-
675671
let instruction = Instruction::IfElse {
676672
then_condition: condition,
677673
then_value: value,
678-
else_condition,
674+
679675
else_value: previous_value,
680676
};
681677

‎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

@@ -216,7 +206,6 @@ impl<'a> ValueMerger<'a> {
216206
&mut self,
217207
typ: Type,
218208
then_condition: ValueId,
219-
else_condition: ValueId,
220209
then_value_id: ValueId,
221210
else_value_id: ValueId,
222211
) -> ValueId {
@@ -274,12 +263,7 @@ impl<'a> ValueMerger<'a> {
274263
let else_element =
275264
get_element(else_value_id, typevars, else_len * element_types.len());
276265

277-
merged.push_back(self.merge_values(
278-
then_condition,
279-
else_condition,
280-
then_element,
281-
else_element,
282-
));
266+
merged.push_back(self.merge_values(then_condition, then_element, else_element));
283267
}
284268
}
285269

@@ -322,7 +306,6 @@ impl<'a> ValueMerger<'a> {
322306
fn try_merge_only_changed_indices(
323307
&mut self,
324308
then_condition: ValueId,
325-
else_condition: ValueId,
326309
then_value: ValueId,
327310
else_value: ValueId,
328311
array_length: usize,
@@ -406,8 +389,7 @@ impl<'a> ValueMerger<'a> {
406389
let then_element = get_element(then_value, typevars.clone());
407390
let else_element = get_element(else_value, typevars);
408391

409-
let value =
410-
self.merge_values(then_condition, else_condition, then_element, else_element);
392+
let value = self.merge_values(then_condition, then_element, else_element);
411393

412394
array = self.insert_array_set(array, index, value, Some(condition)).first();
413395
}

‎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)