public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov
@ 2014-06-23  7:02 Zhenqiang Chen
  2014-06-23  7:09 ` Andrew Pinski
  0 siblings, 1 reply; 6+ messages in thread
From: Zhenqiang Chen @ 2014-06-23  7:02 UTC (permalink / raw)
  To: gcc-patches

Hi,

The patch enhances ifcvt to handle conditional compare instruction
(ccmp) to make it work with cmov. For ccmp, ALLOW_CC_MODE is set to
TRUE when calling canonicalize_condition. And the backend does not
need to generate additional "compare (CC, 0)" for it.

Bootstrap and no check regression on qemu.

OK for trunk?

Thanks!
-Zhenqiang

ChangeLog:
2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>

        * config/aarch64/aarch64.md (mov<mode>cc): Handle ccmp_cc.
        * ifcvt.c: #include "ccmp.h".
        (struct noce_if_info): Add a new field ccmp_p.
        (noce_emit_cmove): Allow ccmp condition.
        (noce_get_alt_condition): Call canonicalize_condition with ccmp_p.
        (noce_get_condition): Set ALLOW_CC_MODE to TRUE for ccmp.
        (noce_process_if_block): Set ccmp_p for ccmp.

testsuite/ChangeLog:
2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>

        * gcc.target/aarch64/ccmn-csel-1.c: New testcase.
        * gcc.target/aarch64/ccmn-csel-2.c: New testcase.
        * gcc.target/aarch64/ccmn-csel-3.c: New testcase.
        * gcc.target/aarch64/ccmp-csel-1.c: New testcase.
        * gcc.target/aarch64/ccmp-csel-2.c: New testcase.
        * gcc.target/aarch64/ccmp-csel-3.c: New testcase.

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index fcc5559..82cc561 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -2459,15 +2459,19 @@
                           (match_operand:ALLI 3 "register_operand" "")))]
   ""
   {
-    rtx ccreg;
     enum rtx_code code = GET_CODE (operands[1]);

     if (code == UNEQ || code == LTGT)
       FAIL;

-    ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
-                                 XEXP (operands[1], 1));
-    operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
+    if (!ccmp_cc_register (XEXP (operands[1], 0),
+                          GET_MODE (XEXP (operands[1], 0))))
+      {
+       rtx ccreg;
+       ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
+                                        XEXP (operands[1], 1));
+       operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
+      }
   }
 )

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 2ca2278..8ee1266 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -43,6 +43,7 @@
 #include "vec.h"
 #include "pointer-set.h"
 #include "dbgcnt.h"
+#include "ccmp.h"

 #ifndef HAVE_conditional_move
 #define HAVE_conditional_move 0
@@ -786,6 +787,9 @@ struct noce_if_info

   /* Estimated cost of the particular branch instruction.  */
   int branch_cost;
+
+  /* The COND is a conditional compare or not.  */
+  bool ccmp_p;
 };

 static rtx noce_emit_store_flag (struct noce_if_info *, rtx, int, int);
@@ -1407,9 +1411,16 @@ noce_emit_cmove (struct noce_if_info *if_info,
rtx x, enum rtx_code code,
       end_sequence ();
     }

-  /* Don't even try if the comparison operands are weird.  */
-  if (! general_operand (cmp_a, GET_MODE (cmp_a))
-      || ! general_operand (cmp_b, GET_MODE (cmp_b)))
+  /* Don't even try if the comparison operands are weird
+     except conditional compare.  */
+  if (if_info->ccmp_p)
+    {
+      if (!(GET_MODE_CLASS (GET_MODE (cmp_a)) == MODE_CC
+           || GET_MODE_CLASS (GET_MODE (cmp_b)) == MODE_CC))
+       return NULL_RTX;
+    }
+  else if (! general_operand (cmp_a, GET_MODE (cmp_a))
+          || ! general_operand (cmp_b, GET_MODE (cmp_b)))
     return NULL_RTX;

 #if HAVE_conditional_move
@@ -1849,7 +1860,7 @@ noce_get_alt_condition (struct noce_if_info
*if_info, rtx target,
     }

   cond = canonicalize_condition (if_info->jump, cond, reverse,
-                                earliest, target, false, true);
+                                earliest, target, if_info->ccmp_p, true);
   if (! cond || ! reg_mentioned_p (target, cond))
     return NULL;

@@ -2300,6 +2311,7 @@ noce_get_condition (rtx jump, rtx *earliest,
bool then_else_reversed)
 {
   rtx cond, set, tmp;
   bool reverse;
+  int allow_cc_mode = false;

   if (! any_condjump_p (jump))
     return NULL_RTX;
@@ -2333,10 +2345,21 @@ noce_get_condition (rtx jump, rtx *earliest,
bool then_else_reversed)
       return cond;
     }

+  /* For conditional compare, set ALLOW_CC_MODE to TRUE.  */
+  if (targetm.gen_ccmp_first)
+    {
+      rtx prev = prev_nonnote_nondebug_insn (jump);
+      if (prev
+         && NONJUMP_INSN_P (prev)
+         && BLOCK_FOR_INSN (prev) == BLOCK_FOR_INSN (jump)
+         && ccmp_insn_p (prev))
+       allow_cc_mode = true;
+    }
+
   /* Otherwise, fall back on canonicalize_condition to do the dirty
      work of manipulating MODE_CC values and COMPARE rtx codes.  */
   tmp = canonicalize_condition (jump, cond, reverse, earliest,
-                               NULL_RTX, false, true);
+                               NULL_RTX, allow_cc_mode, true);

   /* We don't handle side-effects in the condition, like handling
      REG_INC notes and making sure no duplicate conditions are emitted.  */
@@ -2577,6 +2600,11 @@ noce_process_if_block (struct noce_if_info *if_info)
   if_info->a = a;
   if_info->b = b;

+  if (targetm.gen_ccmp_first)
+    if (GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 0))) == MODE_CC
+       || GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 1))) == MODE_CC)
+      if_info->ccmp_p = true;
+
   /* Try optimizations in some approximation of a useful order.  */
   /* ??? Should first look to see if X is live incoming at all.  If it
      isn't, we don't need anything but an unconditional set.  */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
new file mode 100644
index 0000000..472c271
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a < b && c > -5 ? d : 7;
+}
+
+/* { dg-final { scan-assembler "ccmn" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
new file mode 100644
index 0000000..c7d41d5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a < b && c > -9 ? d : c;
+}
+
+/* { dg-final { scan-assembler "ccmn" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
new file mode 100644
index 0000000..9350e5d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a > b && c <= -2 ? 9 : 7;
+}
+
+/* { dg-final { scan-assembler "ccmn" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
new file mode 100644
index 0000000..2d0929f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a < b && c <= d ? d : 7;
+}
+
+/* { dg-final { scan-assembler "ccmp" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
new file mode 100644
index 0000000..978aa64
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a < b && c > d ? d : c;
+}
+
+/* { dg-final { scan-assembler "ccmp" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
new file mode 100644
index 0000000..7db1cd5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a > b && c <= d ? 9 : 7;
+}
+
+/* { dg-final { scan-assembler "ccmp" } } */
+/* { dg-final { scan-assembler "csel" } } */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov
  2014-06-23  7:02 [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov Zhenqiang Chen
@ 2014-06-23  7:09 ` Andrew Pinski
  2014-06-23  7:12   ` Andrew Pinski
  2014-06-23  7:37   ` Zhenqiang Chen
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Pinski @ 2014-06-23  7:09 UTC (permalink / raw)
  To: Zhenqiang Chen; +Cc: gcc-patches

On Mon, Jun 23, 2014 at 12:01 AM, Zhenqiang Chen
<zhenqiang.chen@linaro.org> wrote:
> Hi,
>
> The patch enhances ifcvt to handle conditional compare instruction
> (ccmp) to make it work with cmov. For ccmp, ALLOW_CC_MODE is set to
> TRUE when calling canonicalize_condition. And the backend does not
> need to generate additional "compare (CC, 0)" for it.
>
> Bootstrap and no check regression on qemu.
>
> OK for trunk?
>
> Thanks!
> -Zhenqiang
>
> ChangeLog:
> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>
>         * config/aarch64/aarch64.md (mov<mode>cc): Handle ccmp_cc.
>         * ifcvt.c: #include "ccmp.h".
>         (struct noce_if_info): Add a new field ccmp_p.
>         (noce_emit_cmove): Allow ccmp condition.
>         (noce_get_alt_condition): Call canonicalize_condition with ccmp_p.
>         (noce_get_condition): Set ALLOW_CC_MODE to TRUE for ccmp.
>         (noce_process_if_block): Set ccmp_p for ccmp.
>
> testsuite/ChangeLog:
> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>
>         * gcc.target/aarch64/ccmn-csel-1.c: New testcase.
>         * gcc.target/aarch64/ccmn-csel-2.c: New testcase.
>         * gcc.target/aarch64/ccmn-csel-3.c: New testcase.
>         * gcc.target/aarch64/ccmp-csel-1.c: New testcase.
>         * gcc.target/aarch64/ccmp-csel-2.c: New testcase.
>         * gcc.target/aarch64/ccmp-csel-3.c: New testcase.
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index fcc5559..82cc561 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -2459,15 +2459,19 @@
>                            (match_operand:ALLI 3 "register_operand" "")))]
>    ""
>    {
> -    rtx ccreg;
>      enum rtx_code code = GET_CODE (operands[1]);
>
>      if (code == UNEQ || code == LTGT)
>        FAIL;
>
> -    ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
> -                                 XEXP (operands[1], 1));
> -    operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
> +    if (!ccmp_cc_register (XEXP (operands[1], 0),
> +                          GET_MODE (XEXP (operands[1], 0))))
> +      {
> +       rtx ccreg;
> +       ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
> +                                        XEXP (operands[1], 1));
> +       operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
> +      }
>    }
>  )
>


You should do the same thing for the FP one.  The change to aarch64.md
is exactly the same patch which I came up with.

For the rest I actually I have a late phi-opt pass which does the
conversion into COND_EXPR.  That is I don't change ifcvt at all.

And then I needed two more patches after that to get conditional
compares to work with cmov's.
The following patch which fixes up expand_cond_expr_using_cmove to
handle CCmode correctly:
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -7989,7 +7989,9 @@ expand_cond_expr_using_cmove (tree treeop0
ATTRIBUTE_UNUSED,
       op00 = expand_normal (treeop0);
       op01 = const0_rtx;
       comparison_code = NE;
-      comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
+      comparison_mode = GET_MODE (op00);
+      if (comparison_mode == VOIDmode)
+       comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
     }

   if (GET_MODE (op1) != mode)


--- CUT ---
And then this one to have ccmp to be expanded from the tree level:
index cfc4a16..056e9b0 100644 (file)
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -9300,26 +9300,36 @@ used_in_cond_stmt_p (tree exp)
   imm_use_iterator ui;
   gimple use_stmt;
   FOR_EACH_IMM_USE_STMT (use_stmt, ui, exp)
-    if (gimple_code (use_stmt) == GIMPLE_COND)
-      {
-       tree op1 = gimple_cond_rhs (use_stmt);
-       /* TBD: If we can convert all
-           _Bool t;
+    {
+      if (gimple_code (use_stmt) == GIMPLE_COND)
+       {
+         tree op1 = gimple_cond_rhs (use_stmt);
+         /* TBD: If we can convert all
+             _Bool t;

-           if (t == 1)
-             goto <bb 3>;
-           else
-             goto <bb 4>;
-          to
-           if (t != 0)
-             goto <bb 3>;
-           else
-             goto <bb 4>;
-          we can remove the following check.  */
-       if (integer_zerop (op1))
-         expand_cond = true;
-       BREAK_FROM_IMM_USE_STMT (ui);
-      }
+             if (t == 1)
+               goto <bb 3>;
+             else
+               goto <bb 4>;
+            to
+             if (t != 0)
+               goto <bb 3>;
+             else
+               goto <bb 4>;
+            we can remove the following check.  */
+         if (integer_zerop (op1))
+           expand_cond = true;
+         BREAK_FROM_IMM_USE_STMT (ui);
+       }
+      /* a = EXP ? b : c is also an use in conditional
+         statement. */
+      else if (gimple_code (use_stmt) == GIMPLE_ASSIGN
+              && gimple_expr_code (use_stmt) == COND_EXPR)
+       {
+         if (gimple_assign_rhs1 (use_stmt) == exp)
+           expand_cond = true;
+       }
+    }
   return expand_cond;
 }

Thanks,
Andrew Pinski


> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index 2ca2278..8ee1266 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -43,6 +43,7 @@
>  #include "vec.h"
>  #include "pointer-set.h"
>  #include "dbgcnt.h"
> +#include "ccmp.h"
>
>  #ifndef HAVE_conditional_move
>  #define HAVE_conditional_move 0
> @@ -786,6 +787,9 @@ struct noce_if_info
>
>    /* Estimated cost of the particular branch instruction.  */
>    int branch_cost;
> +
> +  /* The COND is a conditional compare or not.  */
> +  bool ccmp_p;
>  };
>
>  static rtx noce_emit_store_flag (struct noce_if_info *, rtx, int, int);
> @@ -1407,9 +1411,16 @@ noce_emit_cmove (struct noce_if_info *if_info,
> rtx x, enum rtx_code code,
>        end_sequence ();
>      }
>
> -  /* Don't even try if the comparison operands are weird.  */
> -  if (! general_operand (cmp_a, GET_MODE (cmp_a))
> -      || ! general_operand (cmp_b, GET_MODE (cmp_b)))
> +  /* Don't even try if the comparison operands are weird
> +     except conditional compare.  */
> +  if (if_info->ccmp_p)
> +    {
> +      if (!(GET_MODE_CLASS (GET_MODE (cmp_a)) == MODE_CC
> +           || GET_MODE_CLASS (GET_MODE (cmp_b)) == MODE_CC))
> +       return NULL_RTX;
> +    }
> +  else if (! general_operand (cmp_a, GET_MODE (cmp_a))
> +          || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>      return NULL_RTX;
>
>  #if HAVE_conditional_move
> @@ -1849,7 +1860,7 @@ noce_get_alt_condition (struct noce_if_info
> *if_info, rtx target,
>      }
>
>    cond = canonicalize_condition (if_info->jump, cond, reverse,
> -                                earliest, target, false, true);
> +                                earliest, target, if_info->ccmp_p, true);
>    if (! cond || ! reg_mentioned_p (target, cond))
>      return NULL;
>
> @@ -2300,6 +2311,7 @@ noce_get_condition (rtx jump, rtx *earliest,
> bool then_else_reversed)
>  {
>    rtx cond, set, tmp;
>    bool reverse;
> +  int allow_cc_mode = false;
>
>    if (! any_condjump_p (jump))
>      return NULL_RTX;
> @@ -2333,10 +2345,21 @@ noce_get_condition (rtx jump, rtx *earliest,
> bool then_else_reversed)
>        return cond;
>      }
>
> +  /* For conditional compare, set ALLOW_CC_MODE to TRUE.  */
> +  if (targetm.gen_ccmp_first)
> +    {
> +      rtx prev = prev_nonnote_nondebug_insn (jump);
> +      if (prev
> +         && NONJUMP_INSN_P (prev)
> +         && BLOCK_FOR_INSN (prev) == BLOCK_FOR_INSN (jump)
> +         && ccmp_insn_p (prev))
> +       allow_cc_mode = true;
> +    }
> +
>    /* Otherwise, fall back on canonicalize_condition to do the dirty
>       work of manipulating MODE_CC values and COMPARE rtx codes.  */
>    tmp = canonicalize_condition (jump, cond, reverse, earliest,
> -                               NULL_RTX, false, true);
> +                               NULL_RTX, allow_cc_mode, true);
>
>    /* We don't handle side-effects in the condition, like handling
>       REG_INC notes and making sure no duplicate conditions are emitted.  */
> @@ -2577,6 +2600,11 @@ noce_process_if_block (struct noce_if_info *if_info)
>    if_info->a = a;
>    if_info->b = b;
>
> +  if (targetm.gen_ccmp_first)
> +    if (GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 0))) == MODE_CC
> +       || GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 1))) == MODE_CC)
> +      if_info->ccmp_p = true;
> +
>    /* Try optimizations in some approximation of a useful order.  */
>    /* ??? Should first look to see if X is live incoming at all.  If it
>       isn't, we don't need anything but an unconditional set.  */
> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
> new file mode 100644
> index 0000000..472c271
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options " -O2 " } */
> +
> +int foo (int a, int b, int c, int d)
> +{
> +  return a < b && c > -5 ? d : 7;
> +}
> +
> +/* { dg-final { scan-assembler "ccmn" } } */
> +/* { dg-final { scan-assembler "csel" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
> new file mode 100644
> index 0000000..c7d41d5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options " -O2 " } */
> +
> +int foo (int a, int b, int c, int d)
> +{
> +  return a < b && c > -9 ? d : c;
> +}
> +
> +/* { dg-final { scan-assembler "ccmn" } } */
> +/* { dg-final { scan-assembler "csel" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
> new file mode 100644
> index 0000000..9350e5d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options " -O2 " } */
> +
> +int foo (int a, int b, int c, int d)
> +{
> +  return a > b && c <= -2 ? 9 : 7;
> +}
> +
> +/* { dg-final { scan-assembler "ccmn" } } */
> +/* { dg-final { scan-assembler "csel" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
> new file mode 100644
> index 0000000..2d0929f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options " -O2 " } */
> +
> +int foo (int a, int b, int c, int d)
> +{
> +  return a < b && c <= d ? d : 7;
> +}
> +
> +/* { dg-final { scan-assembler "ccmp" } } */
> +/* { dg-final { scan-assembler "csel" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
> new file mode 100644
> index 0000000..978aa64
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options " -O2 " } */
> +
> +int foo (int a, int b, int c, int d)
> +{
> +  return a < b && c > d ? d : c;
> +}
> +
> +/* { dg-final { scan-assembler "ccmp" } } */
> +/* { dg-final { scan-assembler "csel" } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
> new file mode 100644
> index 0000000..7db1cd5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options " -O2 " } */
> +
> +int foo (int a, int b, int c, int d)
> +{
> +  return a > b && c <= d ? 9 : 7;
> +}
> +
> +/* { dg-final { scan-assembler "ccmp" } } */
> +/* { dg-final { scan-assembler "csel" } } */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov
  2014-06-23  7:09 ` Andrew Pinski
@ 2014-06-23  7:12   ` Andrew Pinski
  2014-06-23  7:37   ` Zhenqiang Chen
  1 sibling, 0 replies; 6+ messages in thread
From: Andrew Pinski @ 2014-06-23  7:12 UTC (permalink / raw)
  To: Zhenqiang Chen; +Cc: gcc-patches

On Mon, Jun 23, 2014 at 12:09 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Mon, Jun 23, 2014 at 12:01 AM, Zhenqiang Chen
> <zhenqiang.chen@linaro.org> wrote:
>> Hi,
>>
>> The patch enhances ifcvt to handle conditional compare instruction
>> (ccmp) to make it work with cmov. For ccmp, ALLOW_CC_MODE is set to
>> TRUE when calling canonicalize_condition. And the backend does not
>> need to generate additional "compare (CC, 0)" for it.
>>
>> Bootstrap and no check regression on qemu.
>>
>> OK for trunk?
>>
>> Thanks!
>> -Zhenqiang
>>
>> ChangeLog:
>> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>>
>>         * config/aarch64/aarch64.md (mov<mode>cc): Handle ccmp_cc.
>>         * ifcvt.c: #include "ccmp.h".
>>         (struct noce_if_info): Add a new field ccmp_p.
>>         (noce_emit_cmove): Allow ccmp condition.
>>         (noce_get_alt_condition): Call canonicalize_condition with ccmp_p.
>>         (noce_get_condition): Set ALLOW_CC_MODE to TRUE for ccmp.
>>         (noce_process_if_block): Set ccmp_p for ccmp.
>>
>> testsuite/ChangeLog:
>> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>>
>>         * gcc.target/aarch64/ccmn-csel-1.c: New testcase.
>>         * gcc.target/aarch64/ccmn-csel-2.c: New testcase.
>>         * gcc.target/aarch64/ccmn-csel-3.c: New testcase.
>>         * gcc.target/aarch64/ccmp-csel-1.c: New testcase.
>>         * gcc.target/aarch64/ccmp-csel-2.c: New testcase.
>>         * gcc.target/aarch64/ccmp-csel-3.c: New testcase.


I forgot to make a mention that the following code does not catch:
char foo_c (char a, signed char b)
{
  if (a > 9 && b > -20)
    return 0;
  else
    return 1;
}

Where you need to define a cstorecc4.  I can submit the patch which
adds this pattern if you want.
Note with the cstorecc4 defined you will need the following patch too
to fix up return statements which are not expecting a different mode:
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 789c8b6..cb8b922 100644 (file)
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -3079,6 +3079,11 @@ expand_value_return (rtx val)
       else
         mode = promote_function_mode (type, old_mode, &unsignedp, funtype, 1);

+      /* If the mode of val is not VOID mode (that is val is not a constant),
+         use it for the old mode.  */
+      if (mode != BLKmode && GET_MODE (val) != VOIDmode)
+       old_mode = GET_MODE(val);
+
       if (mode != old_mode)
        val = convert_modes (mode, old_mode, val, unsignedp);


Thanks,
Andrew


>>
>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index fcc5559..82cc561 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -2459,15 +2459,19 @@
>>                            (match_operand:ALLI 3 "register_operand" "")))]
>>    ""
>>    {
>> -    rtx ccreg;
>>      enum rtx_code code = GET_CODE (operands[1]);
>>
>>      if (code == UNEQ || code == LTGT)
>>        FAIL;
>>
>> -    ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
>> -                                 XEXP (operands[1], 1));
>> -    operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
>> +    if (!ccmp_cc_register (XEXP (operands[1], 0),
>> +                          GET_MODE (XEXP (operands[1], 0))))
>> +      {
>> +       rtx ccreg;
>> +       ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
>> +                                        XEXP (operands[1], 1));
>> +       operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
>> +      }
>>    }
>>  )
>>
>
>
> You should do the same thing for the FP one.  The change to aarch64.md
> is exactly the same patch which I came up with.
>
> For the rest I actually I have a late phi-opt pass which does the
> conversion into COND_EXPR.  That is I don't change ifcvt at all.
>
> And then I needed two more patches after that to get conditional
> compares to work with cmov's.
> The following patch which fixes up expand_cond_expr_using_cmove to
> handle CCmode correctly:
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -7989,7 +7989,9 @@ expand_cond_expr_using_cmove (tree treeop0
> ATTRIBUTE_UNUSED,
>        op00 = expand_normal (treeop0);
>        op01 = const0_rtx;
>        comparison_code = NE;
> -      comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
> +      comparison_mode = GET_MODE (op00);
> +      if (comparison_mode == VOIDmode)
> +       comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
>      }
>
>    if (GET_MODE (op1) != mode)
>
>
> --- CUT ---
> And then this one to have ccmp to be expanded from the tree level:
> index cfc4a16..056e9b0 100644 (file)
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -9300,26 +9300,36 @@ used_in_cond_stmt_p (tree exp)
>    imm_use_iterator ui;
>    gimple use_stmt;
>    FOR_EACH_IMM_USE_STMT (use_stmt, ui, exp)
> -    if (gimple_code (use_stmt) == GIMPLE_COND)
> -      {
> -       tree op1 = gimple_cond_rhs (use_stmt);
> -       /* TBD: If we can convert all
> -           _Bool t;
> +    {
> +      if (gimple_code (use_stmt) == GIMPLE_COND)
> +       {
> +         tree op1 = gimple_cond_rhs (use_stmt);
> +         /* TBD: If we can convert all
> +             _Bool t;
>
> -           if (t == 1)
> -             goto <bb 3>;
> -           else
> -             goto <bb 4>;
> -          to
> -           if (t != 0)
> -             goto <bb 3>;
> -           else
> -             goto <bb 4>;
> -          we can remove the following check.  */
> -       if (integer_zerop (op1))
> -         expand_cond = true;
> -       BREAK_FROM_IMM_USE_STMT (ui);
> -      }
> +             if (t == 1)
> +               goto <bb 3>;
> +             else
> +               goto <bb 4>;
> +            to
> +             if (t != 0)
> +               goto <bb 3>;
> +             else
> +               goto <bb 4>;
> +            we can remove the following check.  */
> +         if (integer_zerop (op1))
> +           expand_cond = true;
> +         BREAK_FROM_IMM_USE_STMT (ui);
> +       }
> +      /* a = EXP ? b : c is also an use in conditional
> +         statement. */
> +      else if (gimple_code (use_stmt) == GIMPLE_ASSIGN
> +              && gimple_expr_code (use_stmt) == COND_EXPR)
> +       {
> +         if (gimple_assign_rhs1 (use_stmt) == exp)
> +           expand_cond = true;
> +       }
> +    }
>    return expand_cond;
>  }
>
> Thanks,
> Andrew Pinski
>
>
>> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
>> index 2ca2278..8ee1266 100644
>> --- a/gcc/ifcvt.c
>> +++ b/gcc/ifcvt.c
>> @@ -43,6 +43,7 @@
>>  #include "vec.h"
>>  #include "pointer-set.h"
>>  #include "dbgcnt.h"
>> +#include "ccmp.h"
>>
>>  #ifndef HAVE_conditional_move
>>  #define HAVE_conditional_move 0
>> @@ -786,6 +787,9 @@ struct noce_if_info
>>
>>    /* Estimated cost of the particular branch instruction.  */
>>    int branch_cost;
>> +
>> +  /* The COND is a conditional compare or not.  */
>> +  bool ccmp_p;
>>  };
>>
>>  static rtx noce_emit_store_flag (struct noce_if_info *, rtx, int, int);
>> @@ -1407,9 +1411,16 @@ noce_emit_cmove (struct noce_if_info *if_info,
>> rtx x, enum rtx_code code,
>>        end_sequence ();
>>      }
>>
>> -  /* Don't even try if the comparison operands are weird.  */
>> -  if (! general_operand (cmp_a, GET_MODE (cmp_a))
>> -      || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>> +  /* Don't even try if the comparison operands are weird
>> +     except conditional compare.  */
>> +  if (if_info->ccmp_p)
>> +    {
>> +      if (!(GET_MODE_CLASS (GET_MODE (cmp_a)) == MODE_CC
>> +           || GET_MODE_CLASS (GET_MODE (cmp_b)) == MODE_CC))
>> +       return NULL_RTX;
>> +    }
>> +  else if (! general_operand (cmp_a, GET_MODE (cmp_a))
>> +          || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>>      return NULL_RTX;
>>
>>  #if HAVE_conditional_move
>> @@ -1849,7 +1860,7 @@ noce_get_alt_condition (struct noce_if_info
>> *if_info, rtx target,
>>      }
>>
>>    cond = canonicalize_condition (if_info->jump, cond, reverse,
>> -                                earliest, target, false, true);
>> +                                earliest, target, if_info->ccmp_p, true);
>>    if (! cond || ! reg_mentioned_p (target, cond))
>>      return NULL;
>>
>> @@ -2300,6 +2311,7 @@ noce_get_condition (rtx jump, rtx *earliest,
>> bool then_else_reversed)
>>  {
>>    rtx cond, set, tmp;
>>    bool reverse;
>> +  int allow_cc_mode = false;
>>
>>    if (! any_condjump_p (jump))
>>      return NULL_RTX;
>> @@ -2333,10 +2345,21 @@ noce_get_condition (rtx jump, rtx *earliest,
>> bool then_else_reversed)
>>        return cond;
>>      }
>>
>> +  /* For conditional compare, set ALLOW_CC_MODE to TRUE.  */
>> +  if (targetm.gen_ccmp_first)
>> +    {
>> +      rtx prev = prev_nonnote_nondebug_insn (jump);
>> +      if (prev
>> +         && NONJUMP_INSN_P (prev)
>> +         && BLOCK_FOR_INSN (prev) == BLOCK_FOR_INSN (jump)
>> +         && ccmp_insn_p (prev))
>> +       allow_cc_mode = true;
>> +    }
>> +
>>    /* Otherwise, fall back on canonicalize_condition to do the dirty
>>       work of manipulating MODE_CC values and COMPARE rtx codes.  */
>>    tmp = canonicalize_condition (jump, cond, reverse, earliest,
>> -                               NULL_RTX, false, true);
>> +                               NULL_RTX, allow_cc_mode, true);
>>
>>    /* We don't handle side-effects in the condition, like handling
>>       REG_INC notes and making sure no duplicate conditions are emitted.  */
>> @@ -2577,6 +2600,11 @@ noce_process_if_block (struct noce_if_info *if_info)
>>    if_info->a = a;
>>    if_info->b = b;
>>
>> +  if (targetm.gen_ccmp_first)
>> +    if (GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 0))) == MODE_CC
>> +       || GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 1))) == MODE_CC)
>> +      if_info->ccmp_p = true;
>> +
>>    /* Try optimizations in some approximation of a useful order.  */
>>    /* ??? Should first look to see if X is live incoming at all.  If it
>>       isn't, we don't need anything but an unconditional set.  */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>> new file mode 100644
>> index 0000000..472c271
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a < b && c > -5 ? d : 7;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmn" } } */
>> +/* { dg-final { scan-assembler "csel" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>> new file mode 100644
>> index 0000000..c7d41d5
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a < b && c > -9 ? d : c;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmn" } } */
>> +/* { dg-final { scan-assembler "csel" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>> new file mode 100644
>> index 0000000..9350e5d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a > b && c <= -2 ? 9 : 7;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmn" } } */
>> +/* { dg-final { scan-assembler "csel" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>> new file mode 100644
>> index 0000000..2d0929f
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a < b && c <= d ? d : 7;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmp" } } */
>> +/* { dg-final { scan-assembler "csel" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>> new file mode 100644
>> index 0000000..978aa64
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a < b && c > d ? d : c;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmp" } } */
>> +/* { dg-final { scan-assembler "csel" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>> new file mode 100644
>> index 0000000..7db1cd5
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a > b && c <= d ? 9 : 7;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmp" } } */
>> +/* { dg-final { scan-assembler "csel" } } */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov
  2014-06-23  7:09 ` Andrew Pinski
  2014-06-23  7:12   ` Andrew Pinski
@ 2014-06-23  7:37   ` Zhenqiang Chen
  2014-06-23  7:51     ` pinskia
  1 sibling, 1 reply; 6+ messages in thread
From: Zhenqiang Chen @ 2014-06-23  7:37 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

On 23 June 2014 15:09, Andrew Pinski <pinskia@gmail.com> wrote:
> On Mon, Jun 23, 2014 at 12:01 AM, Zhenqiang Chen
> <zhenqiang.chen@linaro.org> wrote:
>> Hi,
>>
>> The patch enhances ifcvt to handle conditional compare instruction
>> (ccmp) to make it work with cmov. For ccmp, ALLOW_CC_MODE is set to
>> TRUE when calling canonicalize_condition. And the backend does not
>> need to generate additional "compare (CC, 0)" for it.
>>
>> Bootstrap and no check regression on qemu.
>>
>> OK for trunk?
>>
>> Thanks!
>> -Zhenqiang
>>
>> ChangeLog:
>> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>>
>>         * config/aarch64/aarch64.md (mov<mode>cc): Handle ccmp_cc.
>>         * ifcvt.c: #include "ccmp.h".
>>         (struct noce_if_info): Add a new field ccmp_p.
>>         (noce_emit_cmove): Allow ccmp condition.
>>         (noce_get_alt_condition): Call canonicalize_condition with ccmp_p.
>>         (noce_get_condition): Set ALLOW_CC_MODE to TRUE for ccmp.
>>         (noce_process_if_block): Set ccmp_p for ccmp.
>>
>> testsuite/ChangeLog:
>> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>>
>>         * gcc.target/aarch64/ccmn-csel-1.c: New testcase.
>>         * gcc.target/aarch64/ccmn-csel-2.c: New testcase.
>>         * gcc.target/aarch64/ccmn-csel-3.c: New testcase.
>>         * gcc.target/aarch64/ccmp-csel-1.c: New testcase.
>>         * gcc.target/aarch64/ccmp-csel-2.c: New testcase.
>>         * gcc.target/aarch64/ccmp-csel-3.c: New testcase.
>>
>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index fcc5559..82cc561 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -2459,15 +2459,19 @@
>>                            (match_operand:ALLI 3 "register_operand" "")))]
>>    ""
>>    {
>> -    rtx ccreg;
>>      enum rtx_code code = GET_CODE (operands[1]);
>>
>>      if (code == UNEQ || code == LTGT)
>>        FAIL;
>>
>> -    ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
>> -                                 XEXP (operands[1], 1));
>> -    operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
>> +    if (!ccmp_cc_register (XEXP (operands[1], 0),
>> +                          GET_MODE (XEXP (operands[1], 0))))
>> +      {
>> +       rtx ccreg;
>> +       ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
>> +                                        XEXP (operands[1], 1));
>> +       operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
>> +      }
>>    }
>>  )
>>
>
>
> You should do the same thing for the FP one.  The change to aarch64.md
> is exactly the same patch which I came up with.

Thanks for the comments.

For AARCH64, we can mix INT and FP compares. But FP compare would be
slower than INT compare.

CMP
FCCMP

FCMP
CCMP

FCMP
FCCMP

I have no enough resource to collect benchmark results to approve them
valuable. So the patches did not handle FP at all. If you had approved
CCMP for FP valuable, I will work out a separate patch to support it.
Or you can share your patches.

Thanks!
-Zhenqiang

> For the rest I actually I have a late phi-opt pass which does the
> conversion into COND_EXPR.  That is I don't change ifcvt at all.
>
> And then I needed two more patches after that to get conditional
> compares to work with cmov's.

Thanks. Any patch to improve ccmp is welcome.

-Zhenqiang

> The following patch which fixes up expand_cond_expr_using_cmove to
> handle CCmode correctly:
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -7989,7 +7989,9 @@ expand_cond_expr_using_cmove (tree treeop0
> ATTRIBUTE_UNUSED,
>        op00 = expand_normal (treeop0);
>        op01 = const0_rtx;
>        comparison_code = NE;
> -      comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
> +      comparison_mode = GET_MODE (op00);
> +      if (comparison_mode == VOIDmode)
> +       comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
>      }
>
>    if (GET_MODE (op1) != mode)
>
>
> --- CUT ---
> And then this one to have ccmp to be expanded from the tree level:
> index cfc4a16..056e9b0 100644 (file)
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -9300,26 +9300,36 @@ used_in_cond_stmt_p (tree exp)
>    imm_use_iterator ui;
>    gimple use_stmt;
>    FOR_EACH_IMM_USE_STMT (use_stmt, ui, exp)
> -    if (gimple_code (use_stmt) == GIMPLE_COND)
> -      {
> -       tree op1 = gimple_cond_rhs (use_stmt);
> -       /* TBD: If we can convert all
> -           _Bool t;
> +    {
> +      if (gimple_code (use_stmt) == GIMPLE_COND)
> +       {
> +         tree op1 = gimple_cond_rhs (use_stmt);
> +         /* TBD: If we can convert all
> +             _Bool t;
>
> -           if (t == 1)
> -             goto <bb 3>;
> -           else
> -             goto <bb 4>;
> -          to
> -           if (t != 0)
> -             goto <bb 3>;
> -           else
> -             goto <bb 4>;
> -          we can remove the following check.  */
> -       if (integer_zerop (op1))
> -         expand_cond = true;
> -       BREAK_FROM_IMM_USE_STMT (ui);
> -      }
> +             if (t == 1)
> +               goto <bb 3>;
> +             else
> +               goto <bb 4>;
> +            to
> +             if (t != 0)
> +               goto <bb 3>;
> +             else
> +               goto <bb 4>;
> +            we can remove the following check.  */
> +         if (integer_zerop (op1))
> +           expand_cond = true;
> +         BREAK_FROM_IMM_USE_STMT (ui);
> +       }
> +      /* a = EXP ? b : c is also an use in conditional
> +         statement. */
> +      else if (gimple_code (use_stmt) == GIMPLE_ASSIGN
> +              && gimple_expr_code (use_stmt) == COND_EXPR)
> +       {
> +         if (gimple_assign_rhs1 (use_stmt) == exp)
> +           expand_cond = true;
> +       }
> +    }
>    return expand_cond;
>  }
>
> Thanks,
> Andrew Pinski
>
>
>> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
>> index 2ca2278..8ee1266 100644
>> --- a/gcc/ifcvt.c
>> +++ b/gcc/ifcvt.c
>> @@ -43,6 +43,7 @@
>>  #include "vec.h"
>>  #include "pointer-set.h"
>>  #include "dbgcnt.h"
>> +#include "ccmp.h"
>>
>>  #ifndef HAVE_conditional_move
>>  #define HAVE_conditional_move 0
>> @@ -786,6 +787,9 @@ struct noce_if_info
>>
>>    /* Estimated cost of the particular branch instruction.  */
>>    int branch_cost;
>> +
>> +  /* The COND is a conditional compare or not.  */
>> +  bool ccmp_p;
>>  };
>>
>>  static rtx noce_emit_store_flag (struct noce_if_info *, rtx, int, int);
>> @@ -1407,9 +1411,16 @@ noce_emit_cmove (struct noce_if_info *if_info,
>> rtx x, enum rtx_code code,
>>        end_sequence ();
>>      }
>>
>> -  /* Don't even try if the comparison operands are weird.  */
>> -  if (! general_operand (cmp_a, GET_MODE (cmp_a))
>> -      || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>> +  /* Don't even try if the comparison operands are weird
>> +     except conditional compare.  */
>> +  if (if_info->ccmp_p)
>> +    {
>> +      if (!(GET_MODE_CLASS (GET_MODE (cmp_a)) == MODE_CC
>> +           || GET_MODE_CLASS (GET_MODE (cmp_b)) == MODE_CC))
>> +       return NULL_RTX;
>> +    }
>> +  else if (! general_operand (cmp_a, GET_MODE (cmp_a))
>> +          || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>>      return NULL_RTX;
>>
>>  #if HAVE_conditional_move
>> @@ -1849,7 +1860,7 @@ noce_get_alt_condition (struct noce_if_info
>> *if_info, rtx target,
>>      }
>>
>>    cond = canonicalize_condition (if_info->jump, cond, reverse,
>> -                                earliest, target, false, true);
>> +                                earliest, target, if_info->ccmp_p, true);
>>    if (! cond || ! reg_mentioned_p (target, cond))
>>      return NULL;
>>
>> @@ -2300,6 +2311,7 @@ noce_get_condition (rtx jump, rtx *earliest,
>> bool then_else_reversed)
>>  {
>>    rtx cond, set, tmp;
>>    bool reverse;
>> +  int allow_cc_mode = false;
>>
>>    if (! any_condjump_p (jump))
>>      return NULL_RTX;
>> @@ -2333,10 +2345,21 @@ noce_get_condition (rtx jump, rtx *earliest,
>> bool then_else_reversed)
>>        return cond;
>>      }
>>
>> +  /* For conditional compare, set ALLOW_CC_MODE to TRUE.  */
>> +  if (targetm.gen_ccmp_first)
>> +    {
>> +      rtx prev = prev_nonnote_nondebug_insn (jump);
>> +      if (prev
>> +         && NONJUMP_INSN_P (prev)
>> +         && BLOCK_FOR_INSN (prev) == BLOCK_FOR_INSN (jump)
>> +         && ccmp_insn_p (prev))
>> +       allow_cc_mode = true;
>> +    }
>> +
>>    /* Otherwise, fall back on canonicalize_condition to do the dirty
>>       work of manipulating MODE_CC values and COMPARE rtx codes.  */
>>    tmp = canonicalize_condition (jump, cond, reverse, earliest,
>> -                               NULL_RTX, false, true);
>> +                               NULL_RTX, allow_cc_mode, true);
>>
>>    /* We don't handle side-effects in the condition, like handling
>>       REG_INC notes and making sure no duplicate conditions are emitted.  */
>> @@ -2577,6 +2600,11 @@ noce_process_if_block (struct noce_if_info *if_info)
>>    if_info->a = a;
>>    if_info->b = b;
>>
>> +  if (targetm.gen_ccmp_first)
>> +    if (GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 0))) == MODE_CC
>> +       || GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 1))) == MODE_CC)
>> +      if_info->ccmp_p = true;
>> +
>>    /* Try optimizations in some approximation of a useful order.  */
>>    /* ??? Should first look to see if X is live incoming at all.  If it
>>       isn't, we don't need anything but an unconditional set.  */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>> new file mode 100644
>> index 0000000..472c271
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a < b && c > -5 ? d : 7;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmn" } } */
>> +/* { dg-final { scan-assembler "csel" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>> new file mode 100644
>> index 0000000..c7d41d5
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a < b && c > -9 ? d : c;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmn" } } */
>> +/* { dg-final { scan-assembler "csel" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>> new file mode 100644
>> index 0000000..9350e5d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a > b && c <= -2 ? 9 : 7;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmn" } } */
>> +/* { dg-final { scan-assembler "csel" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>> new file mode 100644
>> index 0000000..2d0929f
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a < b && c <= d ? d : 7;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmp" } } */
>> +/* { dg-final { scan-assembler "csel" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>> new file mode 100644
>> index 0000000..978aa64
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a < b && c > d ? d : c;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmp" } } */
>> +/* { dg-final { scan-assembler "csel" } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>> new file mode 100644
>> index 0000000..7db1cd5
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile } */
>> +/* { dg-options " -O2 " } */
>> +
>> +int foo (int a, int b, int c, int d)
>> +{
>> +  return a > b && c <= d ? 9 : 7;
>> +}
>> +
>> +/* { dg-final { scan-assembler "ccmp" } } */
>> +/* { dg-final { scan-assembler "csel" } } */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov
  2014-06-23  7:37   ` Zhenqiang Chen
@ 2014-06-23  7:51     ` pinskia
  2014-07-04  6:54       ` Zhenqiang Chen
  0 siblings, 1 reply; 6+ messages in thread
From: pinskia @ 2014-06-23  7:51 UTC (permalink / raw)
  To: Zhenqiang Chen; +Cc: gcc-patches



> On Jun 23, 2014, at 12:37 AM, Zhenqiang Chen <zhenqiang.chen@linaro.org> wrote:
> 
>> On 23 June 2014 15:09, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Mon, Jun 23, 2014 at 12:01 AM, Zhenqiang Chen
>> <zhenqiang.chen@linaro.org> wrote:
>>> Hi,
>>> 
>>> The patch enhances ifcvt to handle conditional compare instruction
>>> (ccmp) to make it work with cmov. For ccmp, ALLOW_CC_MODE is set to
>>> TRUE when calling canonicalize_condition. And the backend does not
>>> need to generate additional "compare (CC, 0)" for it.
>>> 
>>> Bootstrap and no check regression on qemu.
>>> 
>>> OK for trunk?
>>> 
>>> Thanks!
>>> -Zhenqiang
>>> 
>>> ChangeLog:
>>> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>>> 
>>>        * config/aarch64/aarch64.md (mov<mode>cc): Handle ccmp_cc.
>>>        * ifcvt.c: #include "ccmp.h".
>>>        (struct noce_if_info): Add a new field ccmp_p.
>>>        (noce_emit_cmove): Allow ccmp condition.
>>>        (noce_get_alt_condition): Call canonicalize_condition with ccmp_p.
>>>        (noce_get_condition): Set ALLOW_CC_MODE to TRUE for ccmp.
>>>        (noce_process_if_block): Set ccmp_p for ccmp.
>>> 
>>> testsuite/ChangeLog:
>>> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>>> 
>>>        * gcc.target/aarch64/ccmn-csel-1.c: New testcase.
>>>        * gcc.target/aarch64/ccmn-csel-2.c: New testcase.
>>>        * gcc.target/aarch64/ccmn-csel-3.c: New testcase.
>>>        * gcc.target/aarch64/ccmp-csel-1.c: New testcase.
>>>        * gcc.target/aarch64/ccmp-csel-2.c: New testcase.
>>>        * gcc.target/aarch64/ccmp-csel-3.c: New testcase.
>>> 
>>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>>> index fcc5559..82cc561 100644
>>> --- a/gcc/config/aarch64/aarch64.md
>>> +++ b/gcc/config/aarch64/aarch64.md
>>> @@ -2459,15 +2459,19 @@
>>>                           (match_operand:ALLI 3 "register_operand" "")))]
>>>   ""
>>>   {
>>> -    rtx ccreg;
>>>     enum rtx_code code = GET_CODE (operands[1]);
>>> 
>>>     if (code == UNEQ || code == LTGT)
>>>       FAIL;
>>> 
>>> -    ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
>>> -                                 XEXP (operands[1], 1));
>>> -    operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
>>> +    if (!ccmp_cc_register (XEXP (operands[1], 0),
>>> +                          GET_MODE (XEXP (operands[1], 0))))
>>> +      {
>>> +       rtx ccreg;
>>> +       ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
>>> +                                        XEXP (operands[1], 1));
>>> +       operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
>>> +      }
>>>   }
>>> )
>>> 
>> 
>> 
>> You should do the same thing for the FP one.  The change to aarch64.md
>> is exactly the same patch which I came up with.
> 
> Thanks for the comments.
> 
> For AARCH64, we can mix INT and FP compares. But FP compare would be
> slower than INT compare.

One point is that this is not about fp compares but rather moving of fp register. The fp pattern is used for that.  So something like this would fail/ice:
double f(double a, double b, int c, int d)
{
  return c>10&&d>20?a:b;
}

Thanks,
Andrew



> 
> CMP
> FCCMP
> 
> FCMP
> CCMP
> 
> FCMP
> FCCMP
> 
> I have no enough resource to collect benchmark results to approve them
> valuable. So the patches did not handle FP at all. If you had approved
> CCMP for FP valuable, I will work out a separate patch to support it.
> Or you can share your patches.

I need to l

> 
> Thanks!
> -Zhenqiang
> 
>> For the rest I actually I have a late phi-opt pass which does the
>> conversion into COND_EXPR.  That is I don't change ifcvt at all.
>> 
>> And then I needed two more patches after that to get conditional
>> compares to work with cmov's.
> 
> Thanks. Any patch to improve ccmp is welcome.
> 
> -Zhenqiang
> 
>> The following patch which fixes up expand_cond_expr_using_cmove to
>> handle CCmode correctly:
>> --- a/gcc/expr.c
>> +++ b/gcc/expr.c
>> @@ -7989,7 +7989,9 @@ expand_cond_expr_using_cmove (tree treeop0
>> ATTRIBUTE_UNUSED,
>>       op00 = expand_normal (treeop0);
>>       op01 = const0_rtx;
>>       comparison_code = NE;
>> -      comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
>> +      comparison_mode = GET_MODE (op00);
>> +      if (comparison_mode == VOIDmode)
>> +       comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
>>     }
>> 
>>   if (GET_MODE (op1) != mode)
>> 
>> 
>> --- CUT ---
>> And then this one to have ccmp to be expanded from the tree level:
>> index cfc4a16..056e9b0 100644 (file)
>> --- a/gcc/expr.c
>> +++ b/gcc/expr.c
>> @@ -9300,26 +9300,36 @@ used_in_cond_stmt_p (tree exp)
>>   imm_use_iterator ui;
>>   gimple use_stmt;
>>   FOR_EACH_IMM_USE_STMT (use_stmt, ui, exp)
>> -    if (gimple_code (use_stmt) == GIMPLE_COND)
>> -      {
>> -       tree op1 = gimple_cond_rhs (use_stmt);
>> -       /* TBD: If we can convert all
>> -           _Bool t;
>> +    {
>> +      if (gimple_code (use_stmt) == GIMPLE_COND)
>> +       {
>> +         tree op1 = gimple_cond_rhs (use_stmt);
>> +         /* TBD: If we can convert all
>> +             _Bool t;
>> 
>> -           if (t == 1)
>> -             goto <bb 3>;
>> -           else
>> -             goto <bb 4>;
>> -          to
>> -           if (t != 0)
>> -             goto <bb 3>;
>> -           else
>> -             goto <bb 4>;
>> -          we can remove the following check.  */
>> -       if (integer_zerop (op1))
>> -         expand_cond = true;
>> -       BREAK_FROM_IMM_USE_STMT (ui);
>> -      }
>> +             if (t == 1)
>> +               goto <bb 3>;
>> +             else
>> +               goto <bb 4>;
>> +            to
>> +             if (t != 0)
>> +               goto <bb 3>;
>> +             else
>> +               goto <bb 4>;
>> +            we can remove the following check.  */
>> +         if (integer_zerop (op1))
>> +           expand_cond = true;
>> +         BREAK_FROM_IMM_USE_STMT (ui);
>> +       }
>> +      /* a = EXP ? b : c is also an use in conditional
>> +         statement. */
>> +      else if (gimple_code (use_stmt) == GIMPLE_ASSIGN
>> +              && gimple_expr_code (use_stmt) == COND_EXPR)
>> +       {
>> +         if (gimple_assign_rhs1 (use_stmt) == exp)
>> +           expand_cond = true;
>> +       }
>> +    }
>>   return expand_cond;
>> }
>> 
>> Thanks,
>> Andrew Pinski
>> 
>> 
>>> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
>>> index 2ca2278..8ee1266 100644
>>> --- a/gcc/ifcvt.c
>>> +++ b/gcc/ifcvt.c
>>> @@ -43,6 +43,7 @@
>>> #include "vec.h"
>>> #include "pointer-set.h"
>>> #include "dbgcnt.h"
>>> +#include "ccmp.h"
>>> 
>>> #ifndef HAVE_conditional_move
>>> #define HAVE_conditional_move 0
>>> @@ -786,6 +787,9 @@ struct noce_if_info
>>> 
>>>   /* Estimated cost of the particular branch instruction.  */
>>>   int branch_cost;
>>> +
>>> +  /* The COND is a conditional compare or not.  */
>>> +  bool ccmp_p;
>>> };
>>> 
>>> static rtx noce_emit_store_flag (struct noce_if_info *, rtx, int, int);
>>> @@ -1407,9 +1411,16 @@ noce_emit_cmove (struct noce_if_info *if_info,
>>> rtx x, enum rtx_code code,
>>>       end_sequence ();
>>>     }
>>> 
>>> -  /* Don't even try if the comparison operands are weird.  */
>>> -  if (! general_operand (cmp_a, GET_MODE (cmp_a))
>>> -      || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>>> +  /* Don't even try if the comparison operands are weird
>>> +     except conditional compare.  */
>>> +  if (if_info->ccmp_p)
>>> +    {
>>> +      if (!(GET_MODE_CLASS (GET_MODE (cmp_a)) == MODE_CC
>>> +           || GET_MODE_CLASS (GET_MODE (cmp_b)) == MODE_CC))
>>> +       return NULL_RTX;
>>> +    }
>>> +  else if (! general_operand (cmp_a, GET_MODE (cmp_a))
>>> +          || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>>>     return NULL_RTX;
>>> 
>>> #if HAVE_conditional_move
>>> @@ -1849,7 +1860,7 @@ noce_get_alt_condition (struct noce_if_info
>>> *if_info, rtx target,
>>>     }
>>> 
>>>   cond = canonicalize_condition (if_info->jump, cond, reverse,
>>> -                                earliest, target, false, true);
>>> +                                earliest, target, if_info->ccmp_p, true);
>>>   if (! cond || ! reg_mentioned_p (target, cond))
>>>     return NULL;
>>> 
>>> @@ -2300,6 +2311,7 @@ noce_get_condition (rtx jump, rtx *earliest,
>>> bool then_else_reversed)
>>> {
>>>   rtx cond, set, tmp;
>>>   bool reverse;
>>> +  int allow_cc_mode = false;
>>> 
>>>   if (! any_condjump_p (jump))
>>>     return NULL_RTX;
>>> @@ -2333,10 +2345,21 @@ noce_get_condition (rtx jump, rtx *earliest,
>>> bool then_else_reversed)
>>>       return cond;
>>>     }
>>> 
>>> +  /* For conditional compare, set ALLOW_CC_MODE to TRUE.  */
>>> +  if (targetm.gen_ccmp_first)
>>> +    {
>>> +      rtx prev = prev_nonnote_nondebug_insn (jump);
>>> +      if (prev
>>> +         && NONJUMP_INSN_P (prev)
>>> +         && BLOCK_FOR_INSN (prev) == BLOCK_FOR_INSN (jump)
>>> +         && ccmp_insn_p (prev))
>>> +       allow_cc_mode = true;
>>> +    }
>>> +
>>>   /* Otherwise, fall back on canonicalize_condition to do the dirty
>>>      work of manipulating MODE_CC values and COMPARE rtx codes.  */
>>>   tmp = canonicalize_condition (jump, cond, reverse, earliest,
>>> -                               NULL_RTX, false, true);
>>> +                               NULL_RTX, allow_cc_mode, true);
>>> 
>>>   /* We don't handle side-effects in the condition, like handling
>>>      REG_INC notes and making sure no duplicate conditions are emitted.  */
>>> @@ -2577,6 +2600,11 @@ noce_process_if_block (struct noce_if_info *if_info)
>>>   if_info->a = a;
>>>   if_info->b = b;
>>> 
>>> +  if (targetm.gen_ccmp_first)
>>> +    if (GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 0))) == MODE_CC
>>> +       || GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 1))) == MODE_CC)
>>> +      if_info->ccmp_p = true;
>>> +
>>>   /* Try optimizations in some approximation of a useful order.  */
>>>   /* ??? Should first look to see if X is live incoming at all.  If it
>>>      isn't, we don't need anything but an unconditional set.  */
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>>> new file mode 100644
>>> index 0000000..472c271
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>>> @@ -0,0 +1,10 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options " -O2 " } */
>>> +
>>> +int foo (int a, int b, int c, int d)
>>> +{
>>> +  return a < b && c > -5 ? d : 7;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler "ccmn" } } */
>>> +/* { dg-final { scan-assembler "csel" } } */
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>>> new file mode 100644
>>> index 0000000..c7d41d5
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>>> @@ -0,0 +1,10 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options " -O2 " } */
>>> +
>>> +int foo (int a, int b, int c, int d)
>>> +{
>>> +  return a < b && c > -9 ? d : c;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler "ccmn" } } */
>>> +/* { dg-final { scan-assembler "csel" } } */
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>>> new file mode 100644
>>> index 0000000..9350e5d
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>>> @@ -0,0 +1,10 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options " -O2 " } */
>>> +
>>> +int foo (int a, int b, int c, int d)
>>> +{
>>> +  return a > b && c <= -2 ? 9 : 7;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler "ccmn" } } */
>>> +/* { dg-final { scan-assembler "csel" } } */
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>>> new file mode 100644
>>> index 0000000..2d0929f
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>>> @@ -0,0 +1,10 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options " -O2 " } */
>>> +
>>> +int foo (int a, int b, int c, int d)
>>> +{
>>> +  return a < b && c <= d ? d : 7;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler "ccmp" } } */
>>> +/* { dg-final { scan-assembler "csel" } } */
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>>> new file mode 100644
>>> index 0000000..978aa64
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>>> @@ -0,0 +1,10 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options " -O2 " } */
>>> +
>>> +int foo (int a, int b, int c, int d)
>>> +{
>>> +  return a < b && c > d ? d : c;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler "ccmp" } } */
>>> +/* { dg-final { scan-assembler "csel" } } */
>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>>> new file mode 100644
>>> index 0000000..7db1cd5
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>>> @@ -0,0 +1,10 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options " -O2 " } */
>>> +
>>> +int foo (int a, int b, int c, int d)
>>> +{
>>> +  return a > b && c <= d ? 9 : 7;
>>> +}
>>> +
>>> +/* { dg-final { scan-assembler "ccmp" } } */
>>> +/* { dg-final { scan-assembler "csel" } } */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov
  2014-06-23  7:51     ` pinskia
@ 2014-07-04  6:54       ` Zhenqiang Chen
  0 siblings, 0 replies; 6+ messages in thread
From: Zhenqiang Chen @ 2014-07-04  6:54 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

Andrew,

Thanks for the input. Patch is updated with your codes. In addition,
add "cstorecc4" and 4 test cases. So it can optimized the following
example

int foo (int a, int b)
{
   return a > 0 && b < 7;
}
to:
    cmp    w0, wzr
    ccmp    w1, #6, #9, gt
    cset    w0, le

Bootstrap and no make check regression on qemu.

OK for trunk?

Thanks!
-Zhenqiang

2014-07-04  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
            Andrew Pinski  <apinski@cavium.com>

        * config/aarch64/aarch64.md (cstorecc4): New.
        (mov<mode>cc): Handle ccmp_cc.
        * ccmp.c (used_in_cond_stmt_p): Hande ? expr.
        (expand_ccmp_expr): Set mode to the MODE of gimple_assign_lhs (g).
        * expr.c (expand_cond_expr_using_cmove): Handle CCmode.
        * ifcvt.c: #include "ccmp.h".
        (struct noce_if_info): Add a new field ccmp_p.
        (noce_emit_cmove): Allow ccmp condition.
        (noce_get_alt_condition): Call canonicalize_condition with ccmp_p.
        (noce_get_condition): Set ALLOW_CC_MODE to TRUE for ccmp.
        (noce_process_if_block): Set ccmp_p for ccmp.

testsuite/ChangeLog:
2014-07-04  Zhenqiang Chen  <zhenqiang.chen@linaro.org>

        * gcc.target/aarch64/ccmn-csel-1.c: New testcase.
        * gcc.target/aarch64/ccmn-csel-2.c: New testcase.
        * gcc.target/aarch64/ccmn-csel-3.c: New testcase.
        * gcc.target/aarch64/ccmp-csel-1.c: New testcase.
        * gcc.target/aarch64/ccmp-csel-2.c: New testcase.
        * gcc.target/aarch64/ccmp-csel-3.c: New testcase.
        * gcc.target/aarch64/ccmp-csel-4.c: New testcase.
        * gcc.target/aarch64/ccmp-csel-5.c: New testcase.
        * gcc.target/aarch64/ccmp-cset-1.c: New testcase.
        * gcc.target/aarch64/ccmp-cset-2.c: New testcase.

diff --git a/gcc/ccmp.c b/gcc/ccmp.c
index e0670f1..6382974 100644
--- a/gcc/ccmp.c
+++ b/gcc/ccmp.c
@@ -25,6 +25,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tm_p.h"
 #include "tree.h"
 #include "stringpool.h"
+#include "stor-layout.h"
 #include "regs.h"
 #include "expr.h"
 #include "optabs.h"
@@ -133,6 +134,13 @@ used_in_cond_stmt_p (tree exp)
          expand_cond = true;
        BREAK_FROM_IMM_USE_STMT (ui);
       }
+    else if (gimple_code (use_stmt) == GIMPLE_ASSIGN
+            && gimple_expr_code (use_stmt) == COND_EXPR)
+      {
+       if (gimple_assign_rhs1 (use_stmt) == exp)
+         expand_cond = true;
+      }
+
   return expand_cond;
 }

@@ -274,9 +282,11 @@ expand_ccmp_expr (gimple g)
       icode = optab_handler (cstore_optab, cc_mode);
       if (icode != CODE_FOR_nothing)
        {
-         rtx target = gen_reg_rtx (word_mode);
-         tmp = emit_cstore (target, icode, NE, CCmode, CCmode,
-                            0, tmp, const0_rtx, 1, word_mode);
+         tree lhs = gimple_assign_lhs(g);
+         enum machine_mode mode = TYPE_MODE (TREE_TYPE (lhs));
+         rtx target = gen_reg_rtx (mode);
+         tmp = emit_cstore (target, icode, NE, cc_mode, cc_mode,
+                            0, tmp, const0_rtx, 1, mode);
          if (tmp)
            return tmp;
        }
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index c25d940..88733f4 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -2337,6 +2337,19 @@
   "
 )

+(define_expand "cstorecc4"
+  [(set (match_operand:SI 0 "register_operand")
+       (match_operator 1 "aarch64_comparison_operator"
+        [(match_operand 2 "ccmp_cc_register")
+         (match_operand 3 "aarch64_plus_operand")]))]
+  ""
+"{
+  operands[3] = const0_rtx;
+  emit_insn (gen_rtx_SET (SImode, operands[0], operands[1]));
+  DONE;
+}")
+
+
 (define_expand "cstore<mode>4"
   [(set (match_operand:SI 0 "register_operand" "")
        (match_operator:SI 1 "aarch64_comparison_operator"
@@ -2485,15 +2498,19 @@
                           (match_operand:ALLI 3 "register_operand" "")))]
   ""
   {
-    rtx ccreg;
     enum rtx_code code = GET_CODE (operands[1]);

     if (code == UNEQ || code == LTGT)
       FAIL;

-    ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
-                                 XEXP (operands[1], 1));
-    operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
+    if (!ccmp_cc_register (XEXP (operands[1], 0),
+                          GET_MODE (XEXP (operands[1], 0))))
+      {
+       rtx ccreg;
+       ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
+                                        XEXP (operands[1], 1));
+       operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
+      }
   }
 )
diff --git a/gcc/expr.c b/gcc/expr.c
index 4c31521..88928aa 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -7986,7 +7986,9 @@ expand_cond_expr_using_cmove (tree treeop0
ATTRIBUTE_UNUSED,
       op00 = expand_normal (treeop0);
       op01 = const0_rtx;
       comparison_code = NE;
-      comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
+      comparison_mode = GET_MODE (op00);
+      if (comparison_mode == VOIDmode)
+       comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
     }

   if (GET_MODE (op1) != mode)
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 2ca2278..733d583 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -43,6 +43,7 @@
 #include "vec.h"
 #include "pointer-set.h"
 #include "dbgcnt.h"
+#include "ccmp.h"

 #ifndef HAVE_conditional_move
 #define HAVE_conditional_move 0
@@ -786,6 +787,9 @@ struct noce_if_info

   /* Estimated cost of the particular branch instruction.  */
   int branch_cost;
+
+  /* The COND is a conditional compare or not.  */
+  bool ccmp_p;
 };

 static rtx noce_emit_store_flag (struct noce_if_info *, rtx, int, int);
@@ -1407,9 +1411,16 @@ noce_emit_cmove (struct noce_if_info *if_info,
rtx x, enum rtx_code code,
       end_sequence ();
     }

-  /* Don't even try if the comparison operands are weird.  */
-  if (! general_operand (cmp_a, GET_MODE (cmp_a))
-      || ! general_operand (cmp_b, GET_MODE (cmp_b)))
+  /* Don't even try if the comparison operands are weird
+     except conditional compare.  */
+  if (if_info->ccmp_p)
+    {
+      if (!(GET_MODE_CLASS (GET_MODE (cmp_a)) == MODE_CC
+           || GET_MODE_CLASS (GET_MODE (cmp_b)) == MODE_CC))
+       return NULL_RTX;
+    }
+  else if (! general_operand (cmp_a, GET_MODE (cmp_a))
+          || ! general_operand (cmp_b, GET_MODE (cmp_b)))
     return NULL_RTX;

 #if HAVE_conditional_move
@@ -1849,7 +1860,7 @@ noce_get_alt_condition (struct noce_if_info
*if_info, rtx target,
     }

   cond = canonicalize_condition (if_info->jump, cond, reverse,
-                                earliest, target, false, true);
+                                earliest, target, if_info->ccmp_p, true);
   if (! cond || ! reg_mentioned_p (target, cond))
     return NULL;

@@ -2300,6 +2311,7 @@ noce_get_condition (rtx jump, rtx *earliest,
bool then_else_reversed)
 {
   rtx cond, set, tmp;
   bool reverse;
+  int allow_cc_mode = false;

   if (! any_condjump_p (jump))
     return NULL_RTX;
@@ -2333,10 +2345,21 @@ noce_get_condition (rtx jump, rtx *earliest,
bool then_else_reversed)
       return cond;
     }

+  /* For conditional compare, set ALLOW_CC_MODE to TRUE.  */
+  if (targetm.gen_ccmp_first)
+    {
+      rtx prev = prev_nonnote_nondebug_insn (jump);
+      if (prev
+         && NONJUMP_INSN_P (prev)
+         && BLOCK_FOR_INSN (prev) == BLOCK_FOR_INSN (jump)
+         && ccmp_insn_p (prev))
+       allow_cc_mode = true;
+    }
+
   /* Otherwise, fall back on canonicalize_condition to do the dirty
      work of manipulating MODE_CC values and COMPARE rtx codes.  */
   tmp = canonicalize_condition (jump, cond, reverse, earliest,
-                               NULL_RTX, false, true);
+                               NULL_RTX, allow_cc_mode, true);

   /* We don't handle side-effects in the condition, like handling
      REG_INC notes and making sure no duplicate conditions are emitted.  */
@@ -2577,6 +2600,11 @@ noce_process_if_block (struct noce_if_info *if_info)
   if_info->a = a;
   if_info->b = b;

+  if (targetm.gen_ccmp_first)
+    if (GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 0))) == MODE_CC
+       || GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 1))) == MODE_CC)
+      if_info->ccmp_p = true;
+
   /* Try optimizations in some approximation of a useful order.  */
   /* ??? Should first look to see if X is live incoming at all.  If it
      isn't, we don't need anything but an unconditional set.  */

diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
new file mode 100644
index 0000000..472c271
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a < b && c > -5 ? d : 7;
+}
+
+/* { dg-final { scan-assembler "ccmn" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
new file mode 100644
index 0000000..c7d41d5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a < b && c > -9 ? d : c;
+}
+
+/* { dg-final { scan-assembler "ccmn" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
new file mode 100644
index 0000000..9350e5d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a > b && c <= -2 ? 9 : 7;
+}
+
+/* { dg-final { scan-assembler "ccmn" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
new file mode 100644
index 0000000..2d0929f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a < b && c <= d ? d : 7;
+}
+
+/* { dg-final { scan-assembler "ccmp" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
new file mode 100644
index 0000000..978aa64
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a < b && c > d ? d : c;
+}
+
+/* { dg-final { scan-assembler "ccmp" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
new file mode 100644
index 0000000..7db1cd5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return a > b && c <= d ? 9 : 7;
+}
+
+/* { dg-final { scan-assembler "ccmp" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-4.c
b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-4.c
new file mode 100644
index 0000000..51b1ce8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-4.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b, int c, int d)
+{
+  return (a > 0 && b < 7) ? c : d;
+}
+
+/* { dg-final { scan-assembler "ccmp" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-5.c
b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-5.c
new file mode 100644
index 0000000..04d27fa
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-5.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+_Bool foo (int a, int b, _Bool c, _Bool d)
+{
+  return (a > 0 && b < 7) ? c : d;
+}
+
+/* { dg-final { scan-assembler "ccmp" } } */
+/* { dg-final { scan-assembler "csel" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-cset-1.c
b/gcc/testsuite/gcc.target/aarch64/ccmp-cset-1.c
new file mode 100644
index 0000000..fa3dd5b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmp-cset-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+int foo (int a, int b)
+{
+  return a > 0 && b < 7;
+}
+
+/* { dg-final { scan-assembler "ccmp" } } */
+/* { dg-final { scan-assembler "cset" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-cset-2.c
b/gcc/testsuite/gcc.target/aarch64/ccmp-cset-2.c
new file mode 100644
index 0000000..afcd4d0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ccmp-cset-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+
+_Bool foo (int a, int b)
+{
+  return a > 0 && b < 7;
+}
+
+/* { dg-final { scan-assembler "ccmp" } } */
+/* { dg-final { scan-assembler "cset" } } */

On 23 June 2014 15:51,  <pinskia@gmail.com> wrote:
>
>
>> On Jun 23, 2014, at 12:37 AM, Zhenqiang Chen <zhenqiang.chen@linaro.org> wrote:
>>
>>> On 23 June 2014 15:09, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Mon, Jun 23, 2014 at 12:01 AM, Zhenqiang Chen
>>> <zhenqiang.chen@linaro.org> wrote:
>>>> Hi,
>>>>
>>>> The patch enhances ifcvt to handle conditional compare instruction
>>>> (ccmp) to make it work with cmov. For ccmp, ALLOW_CC_MODE is set to
>>>> TRUE when calling canonicalize_condition. And the backend does not
>>>> need to generate additional "compare (CC, 0)" for it.
>>>>
>>>> Bootstrap and no check regression on qemu.
>>>>
>>>> OK for trunk?
>>>>
>>>> Thanks!
>>>> -Zhenqiang
>>>>
>>>> ChangeLog:
>>>> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>>>>
>>>>        * config/aarch64/aarch64.md (mov<mode>cc): Handle ccmp_cc.
>>>>        * ifcvt.c: #include "ccmp.h".
>>>>        (struct noce_if_info): Add a new field ccmp_p.
>>>>        (noce_emit_cmove): Allow ccmp condition.
>>>>        (noce_get_alt_condition): Call canonicalize_condition with ccmp_p.
>>>>        (noce_get_condition): Set ALLOW_CC_MODE to TRUE for ccmp.
>>>>        (noce_process_if_block): Set ccmp_p for ccmp.
>>>>
>>>> testsuite/ChangeLog:
>>>> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
>>>>
>>>>        * gcc.target/aarch64/ccmn-csel-1.c: New testcase.
>>>>        * gcc.target/aarch64/ccmn-csel-2.c: New testcase.
>>>>        * gcc.target/aarch64/ccmn-csel-3.c: New testcase.
>>>>        * gcc.target/aarch64/ccmp-csel-1.c: New testcase.
>>>>        * gcc.target/aarch64/ccmp-csel-2.c: New testcase.
>>>>        * gcc.target/aarch64/ccmp-csel-3.c: New testcase.
>>>>
>>>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>>>> index fcc5559..82cc561 100644
>>>> --- a/gcc/config/aarch64/aarch64.md
>>>> +++ b/gcc/config/aarch64/aarch64.md
>>>> @@ -2459,15 +2459,19 @@
>>>>                           (match_operand:ALLI 3 "register_operand" "")))]
>>>>   ""
>>>>   {
>>>> -    rtx ccreg;
>>>>     enum rtx_code code = GET_CODE (operands[1]);
>>>>
>>>>     if (code == UNEQ || code == LTGT)
>>>>       FAIL;
>>>>
>>>> -    ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
>>>> -                                 XEXP (operands[1], 1));
>>>> -    operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
>>>> +    if (!ccmp_cc_register (XEXP (operands[1], 0),
>>>> +                          GET_MODE (XEXP (operands[1], 0))))
>>>> +      {
>>>> +       rtx ccreg;
>>>> +       ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
>>>> +                                        XEXP (operands[1], 1));
>>>> +       operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
>>>> +      }
>>>>   }
>>>> )
>>>>
>>>
>>>
>>> You should do the same thing for the FP one.  The change to aarch64.md
>>> is exactly the same patch which I came up with.
>>
>> Thanks for the comments.
>>
>> For AARCH64, we can mix INT and FP compares. But FP compare would be
>> slower than INT compare.
>
> One point is that this is not about fp compares but rather moving of fp register. The fp pattern is used for that.  So something like this would fail/ice:
> double f(double a, double b, int c, int d)
> {
>   return c>10&&d>20?a:b;
> }
>
> Thanks,
> Andrew
>
>
>
>>
>> CMP
>> FCCMP
>>
>> FCMP
>> CCMP
>>
>> FCMP
>> FCCMP
>>
>> I have no enough resource to collect benchmark results to approve them
>> valuable. So the patches did not handle FP at all. If you had approved
>> CCMP for FP valuable, I will work out a separate patch to support it.
>> Or you can share your patches.
>
> I need to l
>
>>
>> Thanks!
>> -Zhenqiang
>>
>>> For the rest I actually I have a late phi-opt pass which does the
>>> conversion into COND_EXPR.  That is I don't change ifcvt at all.
>>>
>>> And then I needed two more patches after that to get conditional
>>> compares to work with cmov's.
>>
>> Thanks. Any patch to improve ccmp is welcome.
>>
>> -Zhenqiang
>>
>>> The following patch which fixes up expand_cond_expr_using_cmove to
>>> handle CCmode correctly:
>>> --- a/gcc/expr.c
>>> +++ b/gcc/expr.c
>>> @@ -7989,7 +7989,9 @@ expand_cond_expr_using_cmove (tree treeop0
>>> ATTRIBUTE_UNUSED,
>>>       op00 = expand_normal (treeop0);
>>>       op01 = const0_rtx;
>>>       comparison_code = NE;
>>> -      comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
>>> +      comparison_mode = GET_MODE (op00);
>>> +      if (comparison_mode == VOIDmode)
>>> +       comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
>>>     }
>>>
>>>   if (GET_MODE (op1) != mode)
>>>
>>>
>>> --- CUT ---
>>> And then this one to have ccmp to be expanded from the tree level:
>>> index cfc4a16..056e9b0 100644 (file)
>>> --- a/gcc/expr.c
>>> +++ b/gcc/expr.c
>>> @@ -9300,26 +9300,36 @@ used_in_cond_stmt_p (tree exp)
>>>   imm_use_iterator ui;
>>>   gimple use_stmt;
>>>   FOR_EACH_IMM_USE_STMT (use_stmt, ui, exp)
>>> -    if (gimple_code (use_stmt) == GIMPLE_COND)
>>> -      {
>>> -       tree op1 = gimple_cond_rhs (use_stmt);
>>> -       /* TBD: If we can convert all
>>> -           _Bool t;
>>> +    {
>>> +      if (gimple_code (use_stmt) == GIMPLE_COND)
>>> +       {
>>> +         tree op1 = gimple_cond_rhs (use_stmt);
>>> +         /* TBD: If we can convert all
>>> +             _Bool t;
>>>
>>> -           if (t == 1)
>>> -             goto <bb 3>;
>>> -           else
>>> -             goto <bb 4>;
>>> -          to
>>> -           if (t != 0)
>>> -             goto <bb 3>;
>>> -           else
>>> -             goto <bb 4>;
>>> -          we can remove the following check.  */
>>> -       if (integer_zerop (op1))
>>> -         expand_cond = true;
>>> -       BREAK_FROM_IMM_USE_STMT (ui);
>>> -      }
>>> +             if (t == 1)
>>> +               goto <bb 3>;
>>> +             else
>>> +               goto <bb 4>;
>>> +            to
>>> +             if (t != 0)
>>> +               goto <bb 3>;
>>> +             else
>>> +               goto <bb 4>;
>>> +            we can remove the following check.  */
>>> +         if (integer_zerop (op1))
>>> +           expand_cond = true;
>>> +         BREAK_FROM_IMM_USE_STMT (ui);
>>> +       }
>>> +      /* a = EXP ? b : c is also an use in conditional
>>> +         statement. */
>>> +      else if (gimple_code (use_stmt) == GIMPLE_ASSIGN
>>> +              && gimple_expr_code (use_stmt) == COND_EXPR)
>>> +       {
>>> +         if (gimple_assign_rhs1 (use_stmt) == exp)
>>> +           expand_cond = true;
>>> +       }
>>> +    }
>>>   return expand_cond;
>>> }
>>>
>>> Thanks,
>>> Andrew Pinski
>>>
>>>
>>>> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
>>>> index 2ca2278..8ee1266 100644
>>>> --- a/gcc/ifcvt.c
>>>> +++ b/gcc/ifcvt.c
>>>> @@ -43,6 +43,7 @@
>>>> #include "vec.h"
>>>> #include "pointer-set.h"
>>>> #include "dbgcnt.h"
>>>> +#include "ccmp.h"
>>>>
>>>> #ifndef HAVE_conditional_move
>>>> #define HAVE_conditional_move 0
>>>> @@ -786,6 +787,9 @@ struct noce_if_info
>>>>
>>>>   /* Estimated cost of the particular branch instruction.  */
>>>>   int branch_cost;
>>>> +
>>>> +  /* The COND is a conditional compare or not.  */
>>>> +  bool ccmp_p;
>>>> };
>>>>
>>>> static rtx noce_emit_store_flag (struct noce_if_info *, rtx, int, int);
>>>> @@ -1407,9 +1411,16 @@ noce_emit_cmove (struct noce_if_info *if_info,
>>>> rtx x, enum rtx_code code,
>>>>       end_sequence ();
>>>>     }
>>>>
>>>> -  /* Don't even try if the comparison operands are weird.  */
>>>> -  if (! general_operand (cmp_a, GET_MODE (cmp_a))
>>>> -      || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>>>> +  /* Don't even try if the comparison operands are weird
>>>> +     except conditional compare.  */
>>>> +  if (if_info->ccmp_p)
>>>> +    {
>>>> +      if (!(GET_MODE_CLASS (GET_MODE (cmp_a)) == MODE_CC
>>>> +           || GET_MODE_CLASS (GET_MODE (cmp_b)) == MODE_CC))
>>>> +       return NULL_RTX;
>>>> +    }
>>>> +  else if (! general_operand (cmp_a, GET_MODE (cmp_a))
>>>> +          || ! general_operand (cmp_b, GET_MODE (cmp_b)))
>>>>     return NULL_RTX;
>>>>
>>>> #if HAVE_conditional_move
>>>> @@ -1849,7 +1860,7 @@ noce_get_alt_condition (struct noce_if_info
>>>> *if_info, rtx target,
>>>>     }
>>>>
>>>>   cond = canonicalize_condition (if_info->jump, cond, reverse,
>>>> -                                earliest, target, false, true);
>>>> +                                earliest, target, if_info->ccmp_p, true);
>>>>   if (! cond || ! reg_mentioned_p (target, cond))
>>>>     return NULL;
>>>>
>>>> @@ -2300,6 +2311,7 @@ noce_get_condition (rtx jump, rtx *earliest,
>>>> bool then_else_reversed)
>>>> {
>>>>   rtx cond, set, tmp;
>>>>   bool reverse;
>>>> +  int allow_cc_mode = false;
>>>>
>>>>   if (! any_condjump_p (jump))
>>>>     return NULL_RTX;
>>>> @@ -2333,10 +2345,21 @@ noce_get_condition (rtx jump, rtx *earliest,
>>>> bool then_else_reversed)
>>>>       return cond;
>>>>     }
>>>>
>>>> +  /* For conditional compare, set ALLOW_CC_MODE to TRUE.  */
>>>> +  if (targetm.gen_ccmp_first)
>>>> +    {
>>>> +      rtx prev = prev_nonnote_nondebug_insn (jump);
>>>> +      if (prev
>>>> +         && NONJUMP_INSN_P (prev)
>>>> +         && BLOCK_FOR_INSN (prev) == BLOCK_FOR_INSN (jump)
>>>> +         && ccmp_insn_p (prev))
>>>> +       allow_cc_mode = true;
>>>> +    }
>>>> +
>>>>   /* Otherwise, fall back on canonicalize_condition to do the dirty
>>>>      work of manipulating MODE_CC values and COMPARE rtx codes.  */
>>>>   tmp = canonicalize_condition (jump, cond, reverse, earliest,
>>>> -                               NULL_RTX, false, true);
>>>> +                               NULL_RTX, allow_cc_mode, true);
>>>>
>>>>   /* We don't handle side-effects in the condition, like handling
>>>>      REG_INC notes and making sure no duplicate conditions are emitted.  */
>>>> @@ -2577,6 +2600,11 @@ noce_process_if_block (struct noce_if_info *if_info)
>>>>   if_info->a = a;
>>>>   if_info->b = b;
>>>>
>>>> +  if (targetm.gen_ccmp_first)
>>>> +    if (GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 0))) == MODE_CC
>>>> +       || GET_MODE_CLASS (GET_MODE (XEXP (if_info->cond, 1))) == MODE_CC)
>>>> +      if_info->ccmp_p = true;
>>>> +
>>>>   /* Try optimizations in some approximation of a useful order.  */
>>>>   /* ??? Should first look to see if X is live incoming at all.  If it
>>>>      isn't, we don't need anything but an unconditional set.  */
>>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>>>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>>>> new file mode 100644
>>>> index 0000000..472c271
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-1.c
>>>> @@ -0,0 +1,10 @@
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options " -O2 " } */
>>>> +
>>>> +int foo (int a, int b, int c, int d)
>>>> +{
>>>> +  return a < b && c > -5 ? d : 7;
>>>> +}
>>>> +
>>>> +/* { dg-final { scan-assembler "ccmn" } } */
>>>> +/* { dg-final { scan-assembler "csel" } } */
>>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>>>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>>>> new file mode 100644
>>>> index 0000000..c7d41d5
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-2.c
>>>> @@ -0,0 +1,10 @@
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options " -O2 " } */
>>>> +
>>>> +int foo (int a, int b, int c, int d)
>>>> +{
>>>> +  return a < b && c > -9 ? d : c;
>>>> +}
>>>> +
>>>> +/* { dg-final { scan-assembler "ccmn" } } */
>>>> +/* { dg-final { scan-assembler "csel" } } */
>>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>>>> b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>>>> new file mode 100644
>>>> index 0000000..9350e5d
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmn-csel-3.c
>>>> @@ -0,0 +1,10 @@
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options " -O2 " } */
>>>> +
>>>> +int foo (int a, int b, int c, int d)
>>>> +{
>>>> +  return a > b && c <= -2 ? 9 : 7;
>>>> +}
>>>> +
>>>> +/* { dg-final { scan-assembler "ccmn" } } */
>>>> +/* { dg-final { scan-assembler "csel" } } */
>>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>>>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>>>> new file mode 100644
>>>> index 0000000..2d0929f
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-1.c
>>>> @@ -0,0 +1,10 @@
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options " -O2 " } */
>>>> +
>>>> +int foo (int a, int b, int c, int d)
>>>> +{
>>>> +  return a < b && c <= d ? d : 7;
>>>> +}
>>>> +
>>>> +/* { dg-final { scan-assembler "ccmp" } } */
>>>> +/* { dg-final { scan-assembler "csel" } } */
>>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>>>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>>>> new file mode 100644
>>>> index 0000000..978aa64
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-2.c
>>>> @@ -0,0 +1,10 @@
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options " -O2 " } */
>>>> +
>>>> +int foo (int a, int b, int c, int d)
>>>> +{
>>>> +  return a < b && c > d ? d : c;
>>>> +}
>>>> +
>>>> +/* { dg-final { scan-assembler "ccmp" } } */
>>>> +/* { dg-final { scan-assembler "csel" } } */
>>>> diff --git a/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>>>> b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>>>> new file mode 100644
>>>> index 0000000..7db1cd5
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/aarch64/ccmp-csel-3.c
>>>> @@ -0,0 +1,10 @@
>>>> +/* { dg-do compile } */
>>>> +/* { dg-options " -O2 " } */
>>>> +
>>>> +int foo (int a, int b, int c, int d)
>>>> +{
>>>> +  return a > b && c <= d ? 9 : 7;
>>>> +}
>>>> +
>>>> +/* { dg-final { scan-assembler "ccmp" } } */
>>>> +/* { dg-final { scan-assembler "csel" } } */

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-07-04  6:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-23  7:02 [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov Zhenqiang Chen
2014-06-23  7:09 ` Andrew Pinski
2014-06-23  7:12   ` Andrew Pinski
2014-06-23  7:37   ` Zhenqiang Chen
2014-06-23  7:51     ` pinskia
2014-07-04  6:54       ` Zhenqiang Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).