From e5ec7242cc0d42af77fde124d4f269c720a37533 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 28 Mar 2018 13:20:11 -0700 Subject: [PATCH] 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 | 81 ++++++++++++++++++++++++++++ lib/cretonne/src/context.rs | 2 - lib/cretonne/src/licm.rs | 45 +++++++++++----- 3 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 cranelift/filetests/licm/reject.cton diff --git a/cranelift/filetests/licm/reject.cton b/cranelift/filetests/licm/reject.cton new file mode 100644 index 0000000000..052ae2b6f2 --- /dev/null +++ b/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 + +} diff --git a/lib/cretonne/src/context.rs b/lib/cretonne/src/context.rs index 8358f21545..fba2cd2cc3 100644 --- a/lib/cretonne/src/context.rs +++ b/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(); diff --git a/lib/cretonne/src/licm.rs b/lib/cretonne/src/licm.rs index 7887a39e4e..f0327296a1 100644 --- a/lib/cretonne/src/licm.rs +++ b/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) -> 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 { let mut loop_values: HashSet = HashSet::new(); - let mut invariant_inst: Vec = Vec::new(); + let mut invariant_insts: Vec = 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() { - if pos.func.dfg.has_results(inst) && - pos.func.dfg.inst_args(inst).into_iter().all(|arg| { - !loop_values.contains(arg) - }) - { + 'next_inst: while let Some(inst) = pos.next_inst() { + if is_loop_invariant(inst, &pos.func.dfg, &loop_values) { // 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.