On Tue, Jun 9, 2015 at 1:57 PM, Jakub Jelinek <ja...@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 <ja...@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
