[PATCH] Fix x86 abs<mode>2 expander for ia32 (PR target/93110)

[Uros Bizjak]

On Fri, Jan 3, 2020 at 9:23 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> The newly added absdi2 expander doesn't work well on ia32, because it
> requires a xordi3 pattern, which is available even for !TARGET_64BIT,
> but only if TARGET_STV && TARGET_SSE2.
>
> The following patch just uses expand_simple_binop which is able to handle
> the splitting of it into two xorsi3 patterns if needed etc., plus is shorter
> in the expander.  Also, there is no need to build a DImode shift amount and
> then convert_modes it.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, on the
> testcase we now get identical code to what GCC 9 emitted in all of -m32
> -mno-stv -mno-sse, -m32 -msse2 -mstv and -m64 modes, ok for trunk?
>
> 2020-01-03  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/93110
>         * config/i386/i386.md (abs<mode>2): Use expand_simple_binop instead of
>         emitting ASHIFTRT, XOR and MINUS by hand.  Use gen_int_mode with QImode
>         instead of gen_int_shift_amount + convert_modes.
>
>         * gcc.dg/torture/pr93110.c: New test.

OK.

Thanks,
Uros.

> --- gcc/config/i386/i386.md.jj  2020-01-01 12:16:10.071276261 +0100
> +++ gcc/config/i386/i386.md     2020-01-02 10:47:26.638399487 +0100
> @@ -9711,30 +9711,16 @@ (define_expand "abs<mode>2"
>
>      /* Generate rtx abs using abs (x) = (((signed) x >> (W-1)) ^ x) -
>         ((signed) x >> (W-1)) */
> -    rtx shift_amount = gen_int_shift_amount (mode,
> -                                      GET_MODE_PRECISION (mode)
> -                                      - 1);
> -    shift_amount = convert_modes (E_QImode, GET_MODE (shift_amount),
> -                           shift_amount, 1);
> -    rtx shift_dst = gen_reg_rtx (mode);
> -    rtx shift_op = gen_rtx_SET (shift_dst,
> -                         gen_rtx_fmt_ee (ASHIFTRT, mode,
> -                                         operands[1], shift_amount));
> -    rtx clobber = gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (CCmode,
> -                                                   FLAGS_REG));
> -    emit_insn (gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, shift_op,
> -                                               clobber)));
> -
> -    rtx xor_op = gen_rtx_SET (operands[0],
> -                       gen_rtx_fmt_ee (XOR, mode, shift_dst,
> -                                       operands[1]));
> -    emit_insn (gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, xor_op, clobber)));
> -
> -    rtx minus_op = gen_rtx_SET (operands[0],
> -                         gen_rtx_fmt_ee (MINUS, mode,
> -                                         operands[0], shift_dst));
> -    emit_insn (gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, minus_op,
> -                                               clobber)));
> +    rtx shift_amount = gen_int_mode (GET_MODE_PRECISION (mode) - 1, QImode);
> +    rtx shift_dst = expand_simple_binop (mode, ASHIFTRT, operands[1],
> +                                        shift_amount, NULL_RTX,
> +                                        0, OPTAB_DIRECT);
> +    rtx xor_dst = expand_simple_binop (mode, XOR, shift_dst, operands[1],
> +                                      operands[0], 0, OPTAB_DIRECT);
> +    rtx minus_dst = expand_simple_binop (mode, MINUS, xor_dst, shift_dst,
> +                                        operands[0], 0, OPTAB_DIRECT);
> +    if (!rtx_equal_p (minus_dst, operands[0]))
> +      emit_move_insn (operands[0], minus_dst);
>      DONE;
>    })
>
> --- gcc/testsuite/gcc.dg/torture/pr93110.c.jj   2020-01-02 10:50:10.335895988 +0100
> +++ gcc/testsuite/gcc.dg/torture/pr93110.c      2020-01-02 10:49:54.248142020 +0100
> @@ -0,0 +1,9 @@
> +/* PR target/93110 */
> +/* { dg-do compile } */
> +/* { dg-additional-options "-mtune=core2 -mno-stv" { target { i?86-*-* x86_64-*-* } } } */
> +
> +long long
> +foo (long long a)
> +{
> +  return a > 0 ? a : -a;
> +}
>
>         Jakub
>