> On Mar 3, 2015, at 7:19 PM, Wilco Dijkstra <wdijk...@arm.com> wrote: > > Expand abs into a compare and conditional negate. This is the most obvious > expansion, enables > merging of the comparison into ALU instructions and is faster on all > implementations.
> Bootstrapped & > regression tested. > > int f(int x) { return abs (x + 1); } > > Before: > add w0, w0, 1 > sxtw x0, w0 > eor x1, x0, x0, asr 63 > sub x1, x1, x0, asr 63 > mov x0, x1 > ret > > After: > adds w0, w0, 1 > csneg w0, w0, w0, pl > ret > > ChangeLog: > > 2015-03-03 Wilco Dijkstra <wdijk...@arm.com> > > * gcc/config/aarch64/aarch64.md (absdi2): optimize abs expansion. > (csneg3<mode>_insn): enable expansion of pattern. > * gcc/testsuite/gcc.target/aarch64/abs_1.c (abs64): update test > for new abs expansion. (abs64_in_dreg): likewise. > > --- > gcc/config/aarch64/aarch64.md | 33 +++++++------------------------- > gcc/testsuite/gcc.target/aarch64/abs_1.c | 5 ++--- > 2 files changed, 9 insertions(+), 29 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md > index 1f4169e..46b7a63 100644 > --- a/gcc/config/aarch64/aarch64.md > +++ b/gcc/config/aarch64/aarch64.md > @@ -2172,35 +2172,16 @@ > [(set_attr "type" "alu_ext")] > ) > > -(define_insn_and_split "absdi2" > - [(set (match_operand:DI 0 "register_operand" "=&r,w") > - (abs:DI (match_operand:DI 1 "register_operand" "r,w")))] > +(define_expand "abs<mode>2" > + [(match_operand:GPI 0 "register_operand" "") > + (match_operand:GPI 1 "register_operand" "")] > "" > - "@ > - # > - abs\\t%d0, %d1" You are removing the 2nd alternative that generates "abs" with your patch. While I agree that using "csneg" is faster on all implementations, can you say the same for "abs"? Especially given the fact that csneg requires 4 operands instead of abs'es 2? Wouldn't it be better to have (define_expand "abs<mode>2") that would expand into either csneg3<mode> or second alternative of current absdi2? Other than this, the patch looks OK. Thank you, -- Maxim Kuvyrkov www.linaro.org