On 10/07/15 14:45, Kyrill Tkachov wrote:
On 10/07/15 10:00, pins...@gmail.com wrote:
On Jul 10, 2015, at 1:47 AM, Kyrill Tkachov <kyrylo.tkac...@arm.com> wrote:
Hi Andrew,
On 10/07/15 09:40, pins...@gmail.com wrote:
On Jul 10, 2015, at 1:34 AM, Kyrill Tkachov <kyrylo.tkac...@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.
So by the time we reach expansion time we don't have a COND_EXPR of two
immediates, so I tried getting
the code in expr.c to do the right thing, but it didn't work out.
This patch catches this opportunity at the RTL level and could catch such cases
if they were to be
generated by any of the pre-combine RTL passes. Or do you reckon looking for
these patterns in RTL
ifcvt is the way to go? I think it would be somewhat messy to express the
CSNEG, CSINV opportunities
there as we don't have optabs for conditional negate and invert, but
conditional increment would work,
though in the aarch64 case we can only do a conditional by 1 rather than a
general conditional add.
Kyrill
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.tkac...@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.tkac...@arm.com>
* gcc.target/aarch64/cond-op-imm_1.c: New test.
<aarch64-csinc-imms.patch>
