Robin Dapp <rd...@linux.ibm.com> writes: > This patch extracts a cc comparison from the initial compare/jump > insn and allows it to be passed to noce_emit_cmove and > emit_conditional_move. > --- > gcc/ifcvt.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++---- > gcc/optabs.c | 7 ++++-- > gcc/optabs.h | 2 +- > 3 files changed, 69 insertions(+), 8 deletions(-) > > diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c > index 99716e5f63c..3db707e1fd1 100644 > --- a/gcc/ifcvt.c > +++ b/gcc/ifcvt.c > @@ -86,6 +86,7 @@ static rtx_insn *find_active_insn_after (basic_block, > rtx_insn *); > static basic_block block_fallthru (basic_block); > static rtx cond_exec_get_condition (rtx_insn *); > static rtx noce_get_condition (rtx_insn *, rtx_insn **, bool); > +static rtx noce_get_compare (rtx_insn *, bool); > static int noce_operand_ok (const_rtx); > static void merge_if_block (ce_if_block *); > static int find_cond_trap (basic_block, edge, edge); > @@ -775,7 +776,7 @@ static int noce_try_addcc (struct noce_if_info *); > static int noce_try_store_flag_constants (struct noce_if_info *); > static int noce_try_store_flag_mask (struct noce_if_info *); > static rtx noce_emit_cmove (struct noce_if_info *, rtx, enum rtx_code, rtx, > - rtx, rtx, rtx); > + rtx, rtx, rtx, rtx = NULL); > static int noce_try_cmove (struct noce_if_info *); > static int noce_try_cmove_arith (struct noce_if_info *); > static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **); > @@ -1658,7 +1659,7 @@ noce_try_store_flag_mask (struct noce_if_info *if_info) > > static rtx > noce_emit_cmove (struct noce_if_info *if_info, rtx x, enum rtx_code code, > - rtx cmp_a, rtx cmp_b, rtx vfalse, rtx vtrue) > + rtx cmp_a, rtx cmp_b, rtx vfalse, rtx vtrue, rtx cc_cmp) > { > rtx target ATTRIBUTE_UNUSED; > int unsignedp ATTRIBUTE_UNUSED; > @@ -1706,7 +1707,7 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx x, > enum rtx_code code, > > target = emit_conditional_move (x, code, cmp_a, cmp_b, VOIDmode, > vtrue, vfalse, GET_MODE (x), > - unsignedp); > + unsignedp, cc_cmp); > if (target) > return target; > > @@ -2970,6 +2971,60 @@ noce_get_condition (rtx_insn *jump, rtx_insn > **earliest, bool then_else_reversed > return tmp; > } > > +/* Get the comparison from the insn. */ > +static rtx > +noce_get_compare (rtx_insn *jump, bool then_else_reversed) > +{ > + enum rtx_code code; > + const_rtx set; > + rtx tem; > + rtx op0, op1; > + > + if (!have_cbranchcc4) > + return 0;
Why do we need to check this? > + > + if (! any_condjump_p (jump)) > + return NULL_RTX; There's a mixture of "return 0" and "return NULL_RTX"; think the latter is preferred. > + > + set = pc_set (jump); > + > + /* If this branches to JUMP_LABEL when the condition is false, > + reverse the condition. */ > + bool reverse = (GET_CODE (XEXP (SET_SRC (set), 2)) == LABEL_REF > + && label_ref_label (XEXP (SET_SRC (set), 2)) == JUMP_LABEL (jump)); > + > + /* We may have to reverse because the caller's if block is not canonical, > + i.e. the THEN block isn't the fallthrough block for the TEST block > + (see find_if_header). */ > + if (then_else_reversed) > + reverse = !reverse; > + > + rtx cond = XEXP (SET_SRC (set), 0); > + > + code = GET_CODE (cond); > + op0 = XEXP (cond, 0); > + op1 = XEXP (cond, 1); > + > + if (reverse) > + code = reversed_comparison_code (cond, jump); > + if (code == UNKNOWN) > + return 0; > + > + /* If constant is first, put it last. */ > + if (CONSTANT_P (op0)) > + code = swap_condition (code), tem = op0, op0 = op1, op1 = tem; Can use std::swap here. But this should never happen: we're looking at the operands to a condition in an existing instruction, which should always be canonical. > + > + /* Never return CC0; return zero instead. */ > + if (CC0_P (op0)) > + return 0; > + > + /* We promised to return a comparison. */ > + rtx ret = gen_rtx_fmt_ee (code, VOIDmode, op0, op1); > + if (COMPARISON_P (ret)) > + return ret; > + return 0; > +} Without the unnecessary swapping, it looks like the only difference between this routine and cond_exec_get_condition is the initial have_cbranchcc4 test and the final COMPARISON_P (ret) test. The COMPARISON_P test shouldn't really be needed, since I think all jump (if_then_else ...)s are supposed to have comparisons, and targets can have no complaints if an if_then_else condition from a jump gets carried over to a conditional move. So I think we can probably drop that too. I didn't understand why have_cbranchcc4 was needed so can't comment on that. But assuming that that can be dropped, it looks like we could rename cond_exec_get_condition to something more generic and use it here too. > + > /* Return true if OP is ok for if-then-else processing. */ > > static int > @@ -3140,6 +3195,8 @@ noce_convert_multiple_sets (struct noce_if_info > *if_info) > rtx y = XEXP (cond, 1); > rtx_code cond_code = GET_CODE (cond); > > + rtx cc_cmp = noce_get_compare (jump, false); > + > /* The true targets for a conditional move. */ > auto_vec<rtx> targets; > /* The temporaries introduced to allow us to not consider register > @@ -3151,7 +3208,8 @@ noce_convert_multiple_sets (struct noce_if_info > *if_info) > > hash_map<rtx, bool> need_temps; > > - check_need_temps (then_bb, &need_temps, cond); > + if (!cc_cmp) > + check_need_temps (then_bb, &need_temps, cond); > > hash_map<rtx, bool> temps_created; > > @@ -3242,7 +3300,7 @@ noce_convert_multiple_sets (struct noce_if_info > *if_info) > > /* Actually emit the conditional move. */ > rtx temp_dest = noce_emit_cmove (if_info, temp, cond_code, > - x, y, new_val, old_val); > + x, y, new_val, old_val, cc_cmp); > > /* If we failed to expand the conditional move, drop out and don't > try to continue. */ > diff --git a/gcc/optabs.c b/gcc/optabs.c > index 06bcaab1f55..3ce6f8cdd30 100644 > --- a/gcc/optabs.c > +++ b/gcc/optabs.c > @@ -4325,7 +4325,7 @@ emit_indirect_jump (rtx loc) > rtx > emit_conditional_move (rtx target, enum rtx_code code, rtx op0, rtx op1, > machine_mode cmode, rtx op2, rtx op3, > - machine_mode mode, int unsignedp) > + machine_mode mode, int unsignedp, rtx cc_cmp) > { > rtx comparison; > rtx_insn *last; > @@ -4408,7 +4408,10 @@ emit_conditional_move (rtx target, enum rtx_code code, > rtx op0, rtx op1, > class expand_operand ops[4]; > > create_output_operand (&ops[0], target, mode); > - create_fixed_operand (&ops[1], comparison); > + if (cc_cmp) > + create_fixed_operand (&ops[1], cc_cmp); > + else > + create_fixed_operand (&ops[1], comparison); > create_input_operand (&ops[2], op2, mode); > create_input_operand (&ops[3], op3, mode); > if (maybe_expand_insn (icode, 4, ops)) Most of what emit_conditional_move does is redundant if we discard "comparison" here. So I think instead we should split this expand_operand code out into a new overload of emit_conditional_move that replaces code, op0 and op1, cmode and unsignedp with "comparison". We can then call it here and in ifcvt.c. Thanks, Richard