[6/9] ifcvt: Extract cc comparison from jump.

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

Robin Dapp <rdapp@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