[PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov

Andrew Pinski pinskia@gmail.com
Mon Jun 23 07:12:00 GMT 2014


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" } } */



More information about the Gcc-patches mailing list