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

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

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?

>   
>
> 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?

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>