Fix handling of value aliases, and re-enable LICM.

Value aliases aren't instructions, so they don't have a location in the
CFG, so it's not meaningful to query whether a value alias is defined
within a loop.

cranelift/filetests/licm/reject.cton

@@ -0,0 +1,81 @@
+test licm
+
+function %other_side_effects(i32) -> i32 {
+
+ebb0(v0: i32):
+    jump ebb1(v0)
+
+ebb1(v1: i32):
+    regmove.i32 v0, %10 -> %20
+; check: ebb1(v1: i32):
+; check: regmove.i32 v0, %10 -> %20
+    v2 = iconst.i32 1
+    brz v1, ebb2(v1)
+    v5 = isub v1, v2
+    jump ebb1(v5)
+
+ebb2(v6: i32):
+    return v6
+
+}
+
+function %cpu_flags(i32, i32) -> i32 {
+ebb0(v0: i32, v1: i32):
+    jump ebb1(v0, v1)
+
+ebb1(v2: i32, v3: i32):
+    v4 = ifcmp.i32 v0, v1
+    v5 = selectif.i32 eq v4, v2, v3
+; check: ebb1(v2: i32, v3: i32):
+; check: ifcmp.i32 v0, v1
+; check: v5 = selectif.i32 eq v4, v2, v3
+    v8 = iconst.i32 1
+    brz v1, ebb2(v1)
+    v9 = isub v1, v8
+    v10 = iadd v1, v8
+    jump ebb1(v9, v10)
+
+ebb2(v6: i32):
+    return v6
+}
+
+function %spill(i32, i32) -> i32 {
+ebb0(v0: i32, v1: i32):
+    v2 = spill.i32 v0
+    jump ebb1(v0, v1)
+
+ebb1(v3: i32, v4: i32):
+    v5 = spill.i32 v1
+    v6 = fill.i32 v2
+    v7 = fill.i32 v5
+; check: ebb1(v3: i32, v4: i32):
+; check: v5 = spill.i32 v1
+; check: v6 = fill.i32 v2
+; check: v7 = fill v5
+    brz v1, ebb2(v1)
+    v9 = isub v1, v4
+    jump ebb1(v9, v3)
+
+ebb2(v10: i32):
+    return v10
+}
+
+function %non_invariant_aliases(i32) -> i32 {
+
+ebb0(v0: i32):
+    jump ebb1(v0)
+
+ebb1(v1: i32):
+    v8 -> v1
+    v9 -> v1
+    v2 = iadd v8, v9
+; check: ebb1(v1: i32):
+; check: v2 = iadd v8, v9
+    brz v1, ebb2(v1)
+    v5 = isub v1, v2
+    jump ebb1(v5)
+
+ebb2(v6: i32):
+    return v6
+
+}

lib/cretonne/src/context.rs

@@ -92,10 +92,8 @@ impl Context {
         self.legalize(isa)?;
         if isa.flags().opt_level() == OptLevel::Best {
             self.compute_domtree();
-            /* TODO: Re-enable LICM.
             self.compute_loop_analysis();
             self.licm(isa)?;
-            */
             self.simple_gvn(isa)?;
         }
         self.compute_domtree();

lib/cretonne/src/licm.rs

@@ -1,7 +1,7 @@
 //! A Loop Invariant Code Motion optimization pass
 
 use cursor::{Cursor, FuncCursor};
-use ir::{Function, Ebb, Inst, Value, Type, InstBuilder, Layout};
+use ir::{Function, Ebb, Inst, Value, Type, InstBuilder, Layout, Opcode, DataFlowGraph};
 use flowgraph::ControlFlowGraph;
 use std::collections::HashSet;
 use dominator_tree::DominatorTree;
@@ -27,10 +27,10 @@ pub fn do_licm(
     for lp in loop_analysis.loops() {
         // For each loop that we want to optimize we determine the set of loop-invariant
         // instructions
-        let invariant_inst = remove_loop_invariant_instructions(lp, func, cfg, loop_analysis);
+        let invariant_insts = remove_loop_invariant_instructions(lp, func, cfg, loop_analysis);
         // Then we create the loop's pre-header and fill it with the invariant instructions
         // Then we remove the invariant instructions from the loop body
-        if !invariant_inst.is_empty() {
+        if !invariant_insts.is_empty() {
             // If the loop has a natural pre-header we use it, otherwise we create it.
             let mut pos;
             match has_pre_header(&func.layout, cfg, domtree, loop_analysis.loop_header(lp)) {
@@ -47,7 +47,7 @@ pub fn do_licm(
             };
             // The last instruction of the pre-header is the termination instruction (usually
             // a jump) so we need to insert just before this.
-            for inst in invariant_inst {
+            for inst in invariant_insts {
                 pos.insert_inst(inst);
             }
         }
@@ -131,6 +131,29 @@ fn change_branch_jump_destination(inst: Inst, new_ebb: Ebb, func: &mut Function)
     }
 }
 
+/// Test whether the given opcode is unsafe to even consider for LICM.
+fn trivially_unsafe_for_licm(opcode: Opcode) -> bool {
+    opcode.can_load() || opcode.can_store() || opcode.is_call() || opcode.is_branch() ||
+        opcode.is_terminator() || opcode.is_return() ||
+        opcode.can_trap() || opcode.other_side_effects() || opcode.writes_cpu_flags()
+}
+
+/// Test whether the given instruction is loop-invariant.
+fn is_loop_invariant(inst: Inst, dfg: &DataFlowGraph, loop_values: &HashSet<Value>) -> bool {
+    if trivially_unsafe_for_licm(dfg[inst].opcode()) {
+        return false;
+    }
+
+    let inst_args = dfg.inst_args(inst);
+    for arg in inst_args {
+        let arg = dfg.resolve_aliases(*arg);
+        if loop_values.contains(&arg) {
+            return false;
+        }
+    }
+    return true;
+}
+
 // Traverses a loop in reverse post-order from a header EBB and identify loop-invariant
 // instructions. These loop-invariant instructions are then removed from the code and returned
 // (in reverse post-order) for later use.
@@ -141,7 +164,7 @@ fn remove_loop_invariant_instructions(
     loop_analysis: &LoopAnalysis,
 ) -> Vec<Inst> {
     let mut loop_values: HashSet<Value> = HashSet::new();
-    let mut invariant_inst: Vec<Inst> = Vec::new();
+    let mut invariant_insts: Vec<Inst> = Vec::new();
     let mut pos = FuncCursor::new(func);
     // We traverse the loop EBB in reverse post-order.
     for ebb in postorder_ebbs_loop(loop_analysis, cfg, lp).iter().rev() {
@@ -151,15 +174,11 @@ fn remove_loop_invariant_instructions(
         }
         pos.goto_top(*ebb);
         #[cfg_attr(feature = "cargo-clippy", allow(block_in_if_condition_stmt))]
-        while let Some(inst) = pos.next_inst() {
+        'next_inst: while let Some(inst) = pos.next_inst() {
-            if pos.func.dfg.has_results(inst) &&
+            if is_loop_invariant(inst, &pos.func.dfg, &loop_values) {
-                pos.func.dfg.inst_args(inst).into_iter().all(|arg| {
-                    !loop_values.contains(arg)
-                })
-            {
                 // If all the instruction's argument are defined outside the loop
                 // then this instruction is loop-invariant
-                invariant_inst.push(inst);
+                invariant_insts.push(inst);
                 // We remove it from the loop
                 pos.remove_inst_and_step_back();
             } else {
@@ -171,7 +190,7 @@ fn remove_loop_invariant_instructions(
             }
         }
     }
-    invariant_inst
+    invariant_insts
 }
 
 /// Return ebbs from a loop in post-order, starting from an entry point in the block.