This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov
- From: pinskia at gmail dot com
- To: Zhenqiang Chen <zhenqiang dot chen at linaro dot org>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 23 Jun 2014 00:51:45 -0700
- Subject: Re: [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov
- Authentication-results: sourceware.org; auth=none
- References: <CACgzC7AOZh5tFio6=Sx0RmHvjkY7UWCHEJfbQsHFvdHv61QXPg at mail dot gmail dot com> <CA+=Sn1mk_2eifK8-idtuA1nzoGNYahZ1N3o0yPBJZ8+X8K8ipg at mail dot gmail dot com> <CACgzC7ANx5bpjb6zaaV-4ssDQRW-TvqU25U-e0s=gGiX+TJRdg at mail dot gmail dot com>
> 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" } } */