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

Message ID 20190802151028.15590-7-rdapp@linux.ibm.com
State New
Headers show
Series
  • Improve icvt "convert multiple"
Related show

Commit Message

Robin Dapp Aug. 2, 2019, 3:10 p.m.
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(-)

-- 
2.17.0

Comments

Richard Sandiford Oct. 27, 2019, 2:12 p.m. | #1
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

Patch

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;
+
+  if (! any_condjump_p (jump))
+    return NULL_RTX;
+
+  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;
+
+  /* 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;
+}
+
 /* 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))
diff --git a/gcc/optabs.h b/gcc/optabs.h
index 0654107d6e3..c4540f87144 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -258,7 +258,7 @@  extern void emit_indirect_jump (rtx);
 
 /* Emit a conditional move operation.  */
 rtx emit_conditional_move (rtx, enum rtx_code, rtx, rtx, machine_mode,
-			   rtx, rtx, machine_mode, int);
+			   rtx, rtx, machine_mode, int, rtx = NULL);
 
 /* Emit a conditional negate or bitwise complement operation.  */
 rtx emit_conditional_neg_or_complement (rtx, rtx_code, machine_mode, rtx,