Fold CMP and CBR instructions (FuelLabs#2615)

sway-core/src/lib.rs

@@ -480,15 +480,28 @@ pub(crate) fn compile_ast_to_ir_to_asm(
         errors
     );
 
-    // The only other optimisation we have at the moment is constant combining.  In lieu of a
-    // forthcoming pass manager we can just call it here now.
+    // TODO: Experiment with putting combine-constants and simplify-cfg
+    // in a loop, but per function.
     check!(
         combine_constants(&mut ir, &entry_point_functions),
         return err(warnings, errors),
         warnings,
         errors
     );
-
+    check!(
+        simplify_cfg(&mut ir, &entry_point_functions),
+        return err(warnings, errors),
+        warnings,
+        errors
+    );
+    // Simplify-CFG helps combine constants.
+    check!(
+        combine_constants(&mut ir, &entry_point_functions),
+        return err(warnings, errors),
+        warnings,
+        errors
+    );
+    // And that in-turn enables more simplify-cfg.
     check!(
         simplify_cfg(&mut ir, &entry_point_functions),
         return err(warnings, errors),

sway-ir/src/block.rs

@@ -112,6 +112,18 @@ impl Block {
         }
     }
 
+    /// Remove the value in the phi instruction which correlates to `from_block`.
+    pub fn remove_phi_val_coming_from(&self, context: &mut Context, from_block: &Block) {
+        let phi_val = self.get_phi(context);
+        if let ValueDatum::Instruction(Instruction::Phi(pairs)) =
+            &mut context.values[phi_val.0].value
+        {
+            pairs.retain(|(block, _value)| block != from_block);
+        } else {
+            unreachable!("Phi value must be a PHI instruction.");
+        }
+    }
+
     /// Replace a block reference in the phi instruction.
     ///
     /// Any reference to `old_source` will be replace with `new_source` in the list of phi values.

sway-ir/src/constant.rs

@@ -145,4 +145,23 @@ impl Constant {
         ));
         Value::new_constant(context, value)
     }
+
+    /// Compare two Constant values. Can't impl PartialOrder because of context.
+    pub fn eq(&self, context: &Context, other: &Self) -> bool {
+        self.ty.eq(context, &other.ty)
+            && match (&self.value, &other.value) {
+                // Two Undefs are *NOT* equal (PartialEq allows this).
+                (ConstantValue::Undef, _) | (_, ConstantValue::Undef) => false,
+                (ConstantValue::Unit, ConstantValue::Unit) => true,
+                (ConstantValue::Bool(l0), ConstantValue::Bool(r0)) => l0 == r0,
+                (ConstantValue::Uint(l0), ConstantValue::Uint(r0)) => l0 == r0,
+                (ConstantValue::B256(l0), ConstantValue::B256(r0)) => l0 == r0,
+                (ConstantValue::String(l0), ConstantValue::String(r0)) => l0 == r0,
+                (ConstantValue::Array(l0), ConstantValue::Array(r0))
+                | (ConstantValue::Struct(l0), ConstantValue::Struct(r0)) => {
+                    l0.iter().zip(r0.iter()).all(|(l0, r0)| l0.eq(context, r0))
+                }
+                _ => false,
+            }
+    }
 }

sway-ir/src/optimize/constants.rs

@@ -11,6 +11,7 @@ use crate::{
     function::Function,
     instruction::Instruction,
     value::{Value, ValueContent, ValueDatum},
+    Predicate,
 };
 
 /// Find constant expressions which can be reduced to fewer opterations.
@@ -22,12 +23,90 @@ pub fn combine_constants(context: &mut Context, function: &Function) -> Result<b
             continue;
         }
 
+        if combine_cmp(context, function) {
+            modified = true;
+            continue;
+        }
+
+        if combine_cbr(context, function)? {
+            modified = true;
+            continue;
+        }
+
         // Other passes here... always continue to the top if pass returns true.
         break;
     }
+
     Ok(modified)
 }
 
+fn combine_cbr(context: &mut Context, function: &Function) -> Result<bool, IrError> {
+    let candidate = function
+        .instruction_iter(context)
+        .find_map(
+            |(in_block, inst_val)| match &context.values[inst_val.0].value {
+                ValueDatum::Instruction(Instruction::ConditionalBranch {
+                    cond_value,
+                    true_block,
+                    false_block,
+                }) if cond_value.is_constant(context) => {
+                    match cond_value.get_constant(context).unwrap().value {
+                        ConstantValue::Bool(true) => {
+                            Some(Ok((inst_val, in_block, *true_block, *false_block)))
+                        }
+                        ConstantValue::Bool(false) => {
+                            Some(Ok((inst_val, in_block, *false_block, *true_block)))
+                        }
+                        _ => Some(Err(IrError::VerifyConditionExprNotABool)),
+                    }
+                }
+                _ => None,
+            },
+        )
+        .transpose()?;
+
+    candidate.map_or(Ok(false), |(cbr, from_block, dest, no_more_dest)| {
+        no_more_dest.remove_phi_val_coming_from(context, &from_block);
+        cbr.replace(context, ValueDatum::Instruction(Instruction::Branch(dest)));
+        Ok(true)
+    })
+}
+
+fn combine_cmp(context: &mut Context, function: &Function) -> bool {
+    let candidate = function
+        .instruction_iter(context)
+        .find_map(
+            |(block, inst_val)| match &context.values[inst_val.0].value {
+                ValueDatum::Instruction(Instruction::Cmp(pred, val1, val2))
+                    if val1.is_constant(context) && val2.is_constant(context) =>
+                {
+                    let val1 = val1.get_constant(context).unwrap();
+                    let val2 = val2.get_constant(context).unwrap();
+                    match pred {
+                        Predicate::Equal => {
+                            if val1.eq(context, val2) {
+                                Some((inst_val, block, true))
+                            } else {
+                                Some((inst_val, block, false))
+                            }
+                        }
+                    }
+                }
+                _ => None,
+            },
+        );
+
+    candidate.map_or(false, |(inst_val, block, cn_replace)| {
+        // Replace this `cmp` instruction with a constant.
+        inst_val.replace(
+            context,
+            ValueDatum::Constant(Constant::new_bool(cn_replace)),
+        );
+        block.remove_instruction(context, inst_val);
+        true
+    })
+}
+
 fn combine_const_insert_values(context: &mut Context, function: &Function) -> bool {
     // Find a candidate `insert_value` instruction.
     let candidate = function

sway-ir/src/optimize/simplify_cfg.rs

@@ -57,6 +57,11 @@ fn remove_dead_blocks(context: &mut Context, function: &Function) -> Result<bool
     for block in function.block_iter(context) {
         if !reachable.contains(&block) {
             modified = true;
+
+            for succ in block.successors(context) {
+                succ.remove_phi_val_coming_from(context, &block);
+            }
+
             function.remove_block(context, &block)?;
         }
     }

sway-ir/src/value.rs

@@ -114,6 +114,11 @@ impl Value {
         }
     }
 
+    /// Replace this value with another one, in-place.
+    pub fn replace(&self, context: &mut Context, other: ValueDatum) {
+        context.values[self.0].value = other;
+    }
+
     pub fn get_instruction_mut<'a>(&self, context: &'a mut Context) -> Option<&'a mut Instruction> {
         if let ValueDatum::Instruction(instruction) =
             &mut context.values.get_mut(self.0).unwrap().value
@@ -124,6 +129,15 @@ impl Value {
         }
     }
 
+    /// Get reference to the Constant inside this value, if it's one.
+    pub fn get_constant<'a>(&self, context: &'a Context) -> Option<&'a Constant> {
+        if let ValueDatum::Constant(cn) = &context.values.get(self.0).unwrap().value {
+            Some(cn)
+        } else {
+            None
+        }
+    }
+
     /// Get the type for this value, if found.
     ///
     /// Arguments and constants always have a type, but only some instructions do.

sway-ir/tests/constants/cbr_cmp_fold.ir

@@ -0,0 +1,24 @@
+// regex: ID=[[:alpha:]_0-9]+
+
+script {
+    fn test1() -> u64 {
+        entry:
+        v0 = const bool false
+        v1 = const bool false
+        // not: cmp eq
+        v2 = cmp eq v0 v1
+        // not: cbr
+        // check: br $(dest=$ID)
+        cbr v2, get_0_block0, get_0_block1
+
+        // check: $dest:
+        get_0_block0:
+        // check: const u64 101
+        v5 = const u64 101
+        ret u64 v5
+
+        get_0_block1:
+        v6 = const u64 111
+        ret u64 v6
+    }
+}

sway-lib-core/src/ops.sw

@@ -353,11 +353,7 @@ impl Ord for b256 {
 // Should this be a trait eventually? Do we want to allow people to customize what `!` does?
 // Scala says yes, Rust says perhaps...
 pub fn not(a: bool) -> bool {
-    // using direct asm for perf
-    asm(r1: a, r2) {
-        eq r2 r1 zero;
-        r2: bool
-    }
+    __eq(a, false)
 }
 
 impl b256 {

test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw

@@ -3,7 +3,7 @@ use basic_storage_abi::{StoreU64, Quad};
 use std::assert::assert;
 
 fn main() -> u64 {
-    let addr = abi(StoreU64, 0x3870edc4883dd886839a8679fe6fb707336e5f4edd35cb3f70f1829af6285682);
+    let addr = abi(StoreU64, 0x57ba3df09ddb4efc3df2931eaa0b565d7100289f6317ec21277657eced7f3c39);
     let key = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff;
     let value = 4242;