This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][AArch64] Improve csinc/csneg/csinv opportunities on immediates



On 10/07/15 10:00, pinskia@gmail.com wrote:



On Jul 10, 2015, at 1:47 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:

Hi Andrew,

On 10/07/15 09:40, pinskia@gmail.com wrote:



On Jul 10, 2015, at 1:34 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:

Hi all,

Currently when evaluating expressions like (a ? 24 : 25) we will move 24 and 25 into
registers and perform a csel on them.  This misses the opportunity to instead move just 24
into a register and then perform a csinc, saving us an instruction and a register use.
Similarly for csneg and csinv.

This patch implements that idea by allowing such pairs of immediates in *cmov<mode>_insn
and adding an early splitter that performs the necessary transformation.

The testcase included in the patch demonstrates the kind of opportunities that are now picked up.

With this patch I see about 9.6% more csinc instructions being generated for SPEC2006
and the generated code looks objectively better (i.e. fewer mov-immediates and slightly
lower register pressure).

Bootstrapped and tested on aarch64.

Ok for trunk?
I think this is the wrong place for this optimization. It should happen in expr.c and we should produce cond_expr on the gimple level.
I had considered it, but I wasn't sure how general the conditional increment/negate/inverse operations
are to warrant a midend implementation. Do you mean the expand_cond_expr_using_cmove function in expr.c?
Yes and we can expand it to even have a target hook on how to expand them if needed.

I played around in that part and it seems that by the time it gets to expansion the midend
doesn't have a cond_expr of the two immediates, it's a PHI node with the immediates already expanded.
I have not been able to get it to match a cond_expr of two immediates there, although that could be
because I'm unfamiliar with that part of the codebase.

Kyrill


There is already a standard pattern for condition add so the a ? Const1 : const2 can be handled in the a generic way without much troubles. We should handle it better in rtl  ifcvt too (that should be an easier patch). The neg and not cases are very target specific but can be handled by a target hook and expand it directly to it.


I have patches to do both but I have not got around to cleaning them up. If anyone wants them, I can send a link to my current gcc 5.1 sources with them included.
Any chance you can post them on gcc-patches even as a rough idea of what needs to be done?

I posted my expr patch a few years ago but I never got around to rth's comments. This was the generic increment patch. Basically aarch64 should be implementing that pattern too.


The main reason why this should be handled in gimple is that ifcvt on the rtl level is not cheap and does not catch all of the cases the simple expansion of phi-opt does. I can dig that patch up and I will be doing that next week anyways.

Thanks,
Andrew

Thanks,
Kyrill

Thanks,
Andrew

Thanks,
Kyrill

2015-07-10  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

    * config/aarch64/aarch64.md (*cmov<mode>_insn): Move stricter
    check for operands 3 and 4 to pattern predicate.  Allow immediates
    that can be expressed as csinc/csneg/csinv.  New define_split.
    (*csinv3<mode>_insn): Rename to...
    (csinv3<mode>_insn): ... This.
    * config/aarch64/aarch64.h (AARCH64_IMMS_OK_FOR_CSNEG): New macro.
    (AARCH64_IMMS_OK_FOR_CSINC): Likewise.
    (AARCH64_IMMS_OK_FOR_CSINV): Likewise.
    * config/aarch64/aarch64.c (aarch64_imms_ok_for_cond_op_1):
    New function.
    (aarch64_imms_ok_for_cond_op): Likewise.
    * config/aarch64/aarch64-protos.h (aarch64_imms_ok_for_cond_op_1):
    Declare prototype.
    (aarch64_imms_ok_for_cond_op): Likewise.

2015-07-10  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

    * gcc.target/aarch64/cond-op-imm_1.c: New test.
<aarch64-csinc-imms.patch>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]