Re: [PATCH][Aarch64] Add support for overflow add and sub operations

[Christophe Lyon <christophe.lyon@linaro.org>]

Hi Michael,


On 19 May 2017 at 07:12, Michael Collison <Michael.Collison@arm.com> wrote:
> Hi,
>
> This patch improves code generations for builtin arithmetic overflow operations for the aarch64 backend. As an example for a simple test case such as:
>
> Sure for a simple test case such as:
>
> int
> f (int x, int y, int *ovf)
> {
>   int res;
>   *ovf = __builtin_sadd_overflow (x, y, &res);
>   return res;
> }
>
> Current trunk at -O2 generates
>
> f:
>         mov     w3, w0
>         mov     w4, 0
>         add     w0, w0, w1
>         tbnz    w1, #31, .L4
>         cmp     w0, w3
>         blt     .L3
> .L2:
>         str     w4, [x2]
>         ret
>         .p2align 3
> .L4:
>         cmp     w0, w3
>         ble     .L2
> .L3:
>         mov     w4, 1
>         b       .L2
>
>
> With the patch this now generates:
>
> f:
>         adds    w0, w0, w1
>         cset    w1, vs
>         str     w1, [x2]
>         ret
>
>
> Original patch from Richard Henderson:
>
> https://gcc.gnu.org/ml/gcc-patches/2016-01/msg01903.html
>
>
> Okay for trunk?
>
> 2017-05-17  Michael Collison  <michael.collison@arm.com>
>             Richard Henderson <rth@redhat.com>
>
>         * config/aarch64/aarch64-modes.def (CC_V): New.
>         * config/aarch64/aarch64-protos.h
>         (aarch64_add_128bit_scratch_regs): Declare
>         (aarch64_add_128bit_scratch_regs): Declare.
>         (aarch64_expand_subvti): Declare.
>         (aarch64_gen_unlikely_cbranch): Declare
>         * config/aarch64/aarch64.c (aarch64_select_cc_mode): Test
>         for signed overflow using CC_Vmode.
>         (aarch64_get_condition_code_1): Handle CC_Vmode.
>         (aarch64_gen_unlikely_cbranch): New function.
>         (aarch64_add_128bit_scratch_regs): New function.
>         (aarch64_subv_128bit_scratch_regs): New function.
>         (aarch64_expand_subvti): New function.
>         * config/aarch64/aarch64.md (addv<GPI>4, uaddv<GPI>4): New.
>         (addti3): Create simpler code if low part is already known to be 0.
>         (addvti4, uaddvti4): New.
>         (*add<GPI>3_compareC_cconly_imm): New.
>         (*add<GPI>3_compareC_cconly): New.
>         (*add<GPI>3_compareC_imm): New.
>         (*add<GPI>3_compareC): Rename from add<GPI>3_compare1; do not
>         handle constants within this pattern.
>         (*add<GPI>3_compareV_cconly_imm): New.
>         (*add<GPI>3_compareV_cconly): New.
>         (*add<GPI>3_compareV_imm): New.
>         (add<GPI>3_compareV): New.
>         (add<GPI>3_carryinC, add<GPI>3_carryinV): New.
>         (*add<GPI>3_carryinC_zero, *add<GPI>3_carryinV_zero): New.
>         (*add<GPI>3_carryinC, *add<GPI>3_carryinV): New.
>         (subv<GPI>4, usubv<GPI>4): New.
>         (subti): Handle op1 zero.
>         (subvti4, usub4ti4): New.
>         (*sub<GPI>3_compare1_imm): New.
>         (sub<GPI>3_carryinCV): New.
>         (*sub<GPI>3_carryinCV_z1_z2, *sub<GPI>3_carryinCV_z1): New.
>         (*sub<GPI>3_carryinCV_z2, *sub<GPI>3_carryinCV): New.
>         * testsuite/gcc.target/arm/builtin_sadd_128.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_saddl.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_saddll.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_uadd_128.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_uaddl.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_uaddll.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_ssub_128.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_ssubl.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_ssubll.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_usub_128.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_usubl.c: New testcase.
>         * testsuite/gcc.target/arm/builtin_usubll.c: New testcase.

I've tried your patch, and 2 of the new tests FAIL:
    gcc.target/aarch64/builtin_sadd_128.c scan-assembler addcs
    gcc.target/aarch64/builtin_uadd_128.c scan-assembler addcs

Am I missing something?

Thanks,

Christophe