[PATCH] Fix ix86_split_long_move collision handling with TLS (PR target/66470)

[Uros Bizjak]

On Tue, Jun 9, 2015 at 1:57 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> As mentioned in the PR, when trying to split:
> (insn 7 15 13 2 (set (reg:DI 0 ax [92])
>         (mem:DI (plus:SI (plus:SI (mult:SI (reg/v:SI 1 dx [orig:89 b ] [89])
>                         (const_int 8 [0x8]))
>                     (unspec:SI [
>                             (const_int 0 [0])
>                         ] UNSPEC_TP))
>                 (reg:SI 0 ax [91])) [1 a S8 A64])) rh1212265.i:2 85 {*movdi_internal}
>      (nil))
> which has collisions == 2 (both ax and dx used on lhs and both
> ax and dx used in the memory address), we generate invalid insn
> - lea with %gs: or %fs: in it.  This patch fixes it by using
> normal lea instead (so remove the unspec UNSPEC_TP from the address
> for lea) and duplicating the unspec UNSPEC_TP to all the memory loads.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk/5/4.9/4.8?
>
> 2015-06-09  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/66470
>         * config/i386/i386.c (ix86_split_long_move): For collisions
>         involving direct tls segment refs, move the UNSPEC_TP out of
>         the address for lea, to each of the memory loads.
>
>         * gcc.dg/tls/pr66470.c: New test.
>
> --- gcc/config/i386/i386.c.jj   2015-06-08 15:41:19.000000000 +0200
> +++ gcc/config/i386/i386.c      2015-06-09 11:50:29.960859723 +0200
> @@ -22866,7 +22866,7 @@ ix86_split_long_move (rtx operands[])
>          Do an lea to the last part and use only one colliding move.  */
>        else if (collisions > 1)
>         {
> -         rtx base;
> +         rtx base, addr, tls_base = NULL_RTX;
>
>           collisions = 1;
>
> @@ -22877,10 +22877,45 @@ ix86_split_long_move (rtx operands[])
>           if (GET_MODE (base) != Pmode)
>             base = gen_rtx_REG (Pmode, REGNO (base));
>
> -         emit_insn (gen_rtx_SET (base, XEXP (part[1][0], 0)));
> +         addr = XEXP (part[1][0], 0);
> +         if (TARGET_TLS_DIRECT_SEG_REFS)
> +           {
> +             struct ix86_address parts;
> +             int ok = ix86_decompose_address (addr, &parts);
> +             gcc_assert (ok);
> +             if (parts.seg == DEFAULT_TLS_SEG_REG)
> +               {
> +                 /* It is not valid to use %gs: or %fs: in
> +                    lea though, so we need to remove it from the
> +                    address used for lea and add it to each individual
> +                    memory loads instead.  */
> +                 addr = copy_rtx (addr);
> +                 rtx *x = &addr;
> +                 while (GET_CODE (*x) == PLUS)

Why not use RTX iterators here? IMO, it would be much more readable.

Uros.

> +                   {
> +                     for (i = 0; i < 2; i++)
> +                       if (GET_CODE (XEXP (*x, i)) == UNSPEC
> +                           && XINT (XEXP (*x, i), 1) == UNSPEC_TP)
> +                         {
> +                           tls_base = XEXP (*x, i);
> +                           *x = XEXP (*x, 1 - i);
> +                           break;
> +                         }
> +                     if (tls_base)
> +                       break;
> +                     x = &XEXP (*x, 0);
> +                   }
> +                 gcc_assert (tls_base);
> +               }
> +           }
> +         emit_insn (gen_rtx_SET (base, addr));
> +         if (tls_base)
> +           base = gen_rtx_PLUS (GET_MODE (base), base, tls_base);
>           part[1][0] = replace_equiv_address (part[1][0], base);
>           for (i = 1; i < nparts; i++)
>             {
> +             if (tls_base)
> +               base = copy_rtx (base);
>               tmp = plus_constant (Pmode, base, UNITS_PER_WORD * i);
>               part[1][i] = replace_equiv_address (part[1][i], tmp);
>             }
> --- gcc/testsuite/gcc.dg/tls/pr66470.c.jj       2015-06-09 11:59:05.543954781 +0200
> +++ gcc/testsuite/gcc.dg/tls/pr66470.c  2015-06-09 11:58:43.000000000 +0200
> @@ -0,0 +1,29 @@
> +/* PR target/66470 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-require-effective-target tls } */
> +
> +extern __thread unsigned long long a[10];
> +extern __thread struct S { int a, b; } b[10];
> +
> +unsigned long long
> +foo (long x)
> +{
> +  return a[x];
> +}
> +
> +struct S
> +bar (long x)
> +{
> +  return b[x];
> +}
> +
> +#ifdef __SIZEOF_INT128__
> +extern __thread unsigned __int128 c[10];
> +
> +unsigned __int128
> +baz (long x)
> +{
> +  return c[x];
> +}
> +#endif
>
>         Jakub