| Message ID | 20200119135815.GA5275@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2 | expand |
On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > GNU2 TLS address computation must be done in ptr_mode to support > -maddress-mode=long. Also drop the "q" suffix from lea to support > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". Please use "lea%z0" instead. > Tested on Linux/x86-64. OK for master? > > Thanks. > > H.J. > --- > gcc/ > > PR target/93319 > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. > (*tls_dynamic_gnu2_lea_64): Renamed to ... > (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. > Remove the {q} suffix from lea. > (*tls_dynamic_gnu2_call_64): Renamed to ... > (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. > (*tls_dynamic_gnu2_combine_64): Renamed to ... > (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. > Pass Pmode to gen_tls_dynamic_gnu2_64. > > gcc/testsuite/ > > PR target/93319 > * gcc.target/i386/pr93319-1a.c: New test. > * gcc.target/i386/pr93319-1b.c: Likewise. > * gcc.target/i386/pr93319-1c.c: Likewise. > * gcc.target/i386/pr93319-1d.c: Likewise. > --- > gcc/config/i386/i386.c | 31 +++++++++++-- > gcc/config/i386/i386.md | 54 +++++++++++----------- > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > 6 files changed, 99 insertions(+), 31 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 2c087a4a3e0..8c437dbe1f3 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > if (TARGET_GNU2_TLS) > { > if (TARGET_64BIT) > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > else > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > tp = get_thread_pointer (Pmode, true); > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > + > + /* NB: Since thread pointer is in ptr_mode, make sure that > + PLUS is done in ptr_mode. */ > + if (Pmode != ptr_mode) > + { > + tp = lowpart_subreg (ptr_mode, tp, Pmode); > + dest = lowpart_subreg (ptr_mode, dest, Pmode); > + dest = gen_rtx_PLUS (ptr_mode, tp, dest); > + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); > + } > + else > + dest = gen_rtx_PLUS (Pmode, tp, dest); > + dest = force_reg (Pmode, dest); Sholdn't we use tp = get_thread_pointer (ptr_mode, true); then? If Pmode == ptr_mode, then we have the same functionality, otherwise we don't have to subreg tp to ptr_mode. Can we use the same approach as in ix86_zero_extend_to_Pmode here, like: dest = gen_rtx_PLUS (ptr_mode, tp, dest); dest = force_reg (Pmode, convert_to_mode (Pmode, dest, 1)); Uros. > > if (GET_MODE (x) != Pmode) > x = gen_rtx_ZERO_EXTEND (Pmode, x); > @@ -10821,7 +10833,7 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > rtx tmp = ix86_tls_module_base (); > > if (TARGET_64BIT) > - emit_insn (gen_tls_dynamic_gnu2_64 (base, tmp)); > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, base, tmp)); > else > emit_insn (gen_tls_dynamic_gnu2_32 (base, tmp, pic)); > > @@ -10864,7 +10876,18 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > > if (TARGET_GNU2_TLS) > { > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, dest, tp)); > + /* NB: Since thread pointer is in ptr_mode, make sure that > + PLUS is done in ptr_mode. */ > + if (Pmode != ptr_mode) > + { > + tp = lowpart_subreg (ptr_mode, tp, Pmode); > + dest = lowpart_subreg (ptr_mode, dest, Pmode); > + dest = gen_rtx_PLUS (ptr_mode, tp, dest); > + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); > + } > + else > + dest = gen_rtx_PLUS (Pmode, tp, dest); > + dest = force_reg (Pmode, dest); > > if (GET_MODE (x) != Pmode) > x = gen_rtx_ZERO_EXTEND (Pmode, x); > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index c9d2f338fe9..d53684096c4 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -15185,14 +15185,14 @@ (define_insn_and_split "*tls_dynamic_gnu2_combine_32" > emit_insn (gen_tls_dynamic_gnu2_32 (operands[5], operands[1], operands[2])); > }) > > -(define_expand "tls_dynamic_gnu2_64" > +(define_expand "@tls_dynamic_gnu2_64_<mode>" > [(set (match_dup 2) > - (unspec:DI [(match_operand 1 "tls_symbolic_operand")] > - UNSPEC_TLSDESC)) > + (unspec:P [(match_operand 1 "tls_symbolic_operand")] > + UNSPEC_TLSDESC)) > (parallel > - [(set (match_operand:DI 0 "register_operand") > - (unspec:DI [(match_dup 1) (match_dup 2) (reg:DI SP_REG)] > - UNSPEC_TLSDESC)) > + [(set (match_operand:P 0 "register_operand") > + (unspec:P [(match_dup 1) (match_dup 2) (reg:P SP_REG)] > + UNSPEC_TLSDESC)) > (clobber (reg:CC FLAGS_REG))])] > "TARGET_64BIT && TARGET_GNU2_TLS" > { > @@ -15200,23 +15200,23 @@ (define_expand "tls_dynamic_gnu2_64" > ix86_tls_descriptor_calls_expanded_in_cfun = true; > }) > > -(define_insn "*tls_dynamic_gnu2_lea_64" > - [(set (match_operand:DI 0 "register_operand" "=r") > - (unspec:DI [(match_operand 1 "tls_symbolic_operand")] > - UNSPEC_TLSDESC))] > +(define_insn "*tls_dynamic_gnu2_lea_64_<mode>" > + [(set (match_operand:P 0 "register_operand" "=r") > + (unspec:P [(match_operand 1 "tls_symbolic_operand")] > + UNSPEC_TLSDESC))] > "TARGET_64BIT && TARGET_GNU2_TLS" > - "lea{q}\t{%E1@TLSDESC(%%rip), %0|%0, %E1@TLSDESC[rip]}" > + "lea\t{%E1@TLSDESC(%%rip), %0|%0, %E1@TLSDESC[rip]}" > [(set_attr "type" "lea") > - (set_attr "mode" "DI") > + (set_attr "mode" "<MODE>") > (set_attr "length" "7") > (set_attr "length_address" "4")]) > > -(define_insn "*tls_dynamic_gnu2_call_64" > - [(set (match_operand:DI 0 "register_operand" "=a") > - (unspec:DI [(match_operand 1 "tls_symbolic_operand") > - (match_operand:DI 2 "register_operand" "0") > - (reg:DI SP_REG)] > - UNSPEC_TLSDESC)) > +(define_insn "*tls_dynamic_gnu2_call_64_<mode>" > + [(set (match_operand:P 0 "register_operand" "=a") > + (unspec:P [(match_operand 1 "tls_symbolic_operand") > + (match_operand:P 2 "register_operand" "0") > + (reg:P SP_REG)] > + UNSPEC_TLSDESC)) > (clobber (reg:CC FLAGS_REG))] > "TARGET_64BIT && TARGET_GNU2_TLS" > "call\t{*%a1@TLSCALL(%2)|[QWORD PTR [%2+%a1@TLSCALL]]}" > @@ -15224,14 +15224,14 @@ (define_insn "*tls_dynamic_gnu2_call_64" > (set_attr "length" "2") > (set_attr "length_address" "0")]) > > -(define_insn_and_split "*tls_dynamic_gnu2_combine_64" > - [(set (match_operand:DI 0 "register_operand" "=&a") > - (plus:DI > - (unspec:DI [(match_operand 2 "tls_modbase_operand") > - (match_operand:DI 3) > - (reg:DI SP_REG)] > - UNSPEC_TLSDESC) > - (const:DI (unspec:DI > +(define_insn_and_split "*tls_dynamic_gnu2_combine_64_<mode>" > + [(set (match_operand:P 0 "register_operand" "=&a") > + (plus:P > + (unspec:P [(match_operand 2 "tls_modbase_operand") > + (match_operand:P 3) > + (reg:P SP_REG)] > + UNSPEC_TLSDESC) > + (const:P (unspec:P > [(match_operand 1 "tls_symbolic_operand")] > UNSPEC_DTPOFF)))) > (clobber (reg:CC FLAGS_REG))] > @@ -15241,7 +15241,7 @@ (define_insn_and_split "*tls_dynamic_gnu2_combine_64" > [(set (match_dup 0) (match_dup 4))] > { > operands[4] = can_create_pseudo_p () ? gen_reg_rtx (Pmode) : operands[0]; > - emit_insn (gen_tls_dynamic_gnu2_64 (operands[4], operands[1])); > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, operands[4], operands[1])); > }) > > (define_split > diff --git a/gcc/testsuite/gcc.target/i386/pr93319-1a.c b/gcc/testsuite/gcc.target/i386/pr93319-1a.c > new file mode 100644 > index 00000000000..ba9d2b618c5 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr93319-1a.c > @@ -0,0 +1,24 @@ > +/* { dg-do compile { target { ! ia32 } } } */ > +/* { dg-require-effective-target maybe_x32 } */ > +/* { dg-require-effective-target fpic } */ > +/* { dg-require-effective-target tls_native } */ > +/* { dg-options "-mx32 -fPIC -mtls-dialect=gnu2" } */ > + > +#include <stdio.h> > + > +extern __thread int bar; > +static __thread int foo = 30; > + > +int * > +test1 (void) > +{ > + printf ("foo: %d\n", foo); > + return &foo; > +} > + > +int * > +test2 (void) > +{ > + printf ("bar: %d\n", bar); > + return &bar; > +} > diff --git a/gcc/testsuite/gcc.target/i386/pr93319-1b.c b/gcc/testsuite/gcc.target/i386/pr93319-1b.c > new file mode 100644 > index 00000000000..788796a9a94 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr93319-1b.c > @@ -0,0 +1,7 @@ > +/* { dg-do compile { target { ! ia32 } } } */ > +/* { dg-require-effective-target maybe_x32 } */ > +/* { dg-require-effective-target fpic } */ > +/* { dg-require-effective-target tls_native } */ > +/* { dg-options "-mx32 -O2 -fPIC -mtls-dialect=gnu2" } */ > + > +#include "pr93319-1a.c" > diff --git a/gcc/testsuite/gcc.target/i386/pr93319-1c.c b/gcc/testsuite/gcc.target/i386/pr93319-1c.c > new file mode 100644 > index 00000000000..8c462e904bd > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr93319-1c.c > @@ -0,0 +1,7 @@ > +/* { dg-do compile { target { ! ia32 } } } */ > +/* { dg-require-effective-target maybe_x32 } */ > +/* { dg-require-effective-target fpic } */ > +/* { dg-require-effective-target tls_native } */ > +/* { dg-options "-mx32 -O2 -fPIC -mtls-dialect=gnu2 -maddress-mode=long" } */ > + > +#include "pr93319-1a.c" > diff --git a/gcc/testsuite/gcc.target/i386/pr93319-1d.c b/gcc/testsuite/gcc.target/i386/pr93319-1d.c > new file mode 100644 > index 00000000000..7989e1c9fd4 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr93319-1d.c > @@ -0,0 +1,7 @@ > +/* { dg-do compile { target { ! ia32 } } } */ > +/* { dg-require-effective-target maybe_x32 } */ > +/* { dg-require-effective-target fpic } */ > +/* { dg-require-effective-target tls_native } */ > +/* { dg-options "-mx32 -fPIC -mtls-dialect=gnu2 -maddress-mode=long" } */ > + > +#include "pr93319-1a.c" > -- > 2.24.1 >
On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > GNU2 TLS address computation must be done in ptr_mode to support > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > Please use "lea%z0" instead. > > > Tested on Linux/x86-64. OK for master? > > > > Thanks. > > > > H.J. > > --- > > gcc/ > > > > PR target/93319 > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. > > Remove the {q} suffix from lea. > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > gcc/testsuite/ > > > > PR target/93319 > > * gcc.target/i386/pr93319-1a.c: New test. > > * gcc.target/i386/pr93319-1b.c: Likewise. > > * gcc.target/i386/pr93319-1c.c: Likewise. > > * gcc.target/i386/pr93319-1d.c: Likewise. > > --- > > gcc/config/i386/i386.c | 31 +++++++++++-- > > gcc/config/i386/i386.md | 54 +++++++++++----------- > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > 6 files changed, 99 insertions(+), 31 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > index 2c087a4a3e0..8c437dbe1f3 100644 > > --- a/gcc/config/i386/i386.c > > +++ b/gcc/config/i386/i386.c > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > > if (TARGET_GNU2_TLS) > > { > > if (TARGET_64BIT) > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > else > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > tp = get_thread_pointer (Pmode, true); > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > + > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > + PLUS is done in ptr_mode. */ Actually, thread_pointer is in Pmode, see the line just above your change. Also, dest is in Pmode, so why do we need all this subreg dance? Uros. > > + if (Pmode != ptr_mode) > > + { > > + tp = lowpart_subreg (ptr_mode, tp, Pmode); > > + dest = lowpart_subreg (ptr_mode, dest, Pmode); > > + dest = gen_rtx_PLUS (ptr_mode, tp, dest); > > + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); > > + } > > + else > > + dest = gen_rtx_PLUS (Pmode, tp, dest); > > + dest = force_reg (Pmode, dest); > > Sholdn't we use > > tp = get_thread_pointer (ptr_mode, true); > > then? If Pmode == ptr_mode, then we have the same functionality, > otherwise we don't have to subreg tp to ptr_mode. > > Can we use the same approach as in ix86_zero_extend_to_Pmode here, like: > > dest = gen_rtx_PLUS (ptr_mode, tp, dest); > dest = force_reg (Pmode, convert_to_mode (Pmode, dest, 1)); > > Uros.
On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > GNU2 TLS address computation must be done in ptr_mode to support > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > Please use "lea%z0" instead. > > > > > Tested on Linux/x86-64. OK for master? > > > > > > Thanks. > > > > > > H.J. > > > --- > > > gcc/ > > > > > > PR target/93319 > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. > > > Remove the {q} suffix from lea. > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > gcc/testsuite/ > > > > > > PR target/93319 > > > * gcc.target/i386/pr93319-1a.c: New test. > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > --- > > > gcc/config/i386/i386.c | 31 +++++++++++-- > > > gcc/config/i386/i386.md | 54 +++++++++++----------- > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > --- a/gcc/config/i386/i386.c > > > +++ b/gcc/config/i386/i386.c > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > > > if (TARGET_GNU2_TLS) > > > { > > > if (TARGET_64BIT) > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > else > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > tp = get_thread_pointer (Pmode, true); > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > + > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > + PLUS is done in ptr_mode. */ > > Actually, thread_pointer is in Pmode, see the line just above your > change. Also, dest is in Pmode, so why do we need all this subreg > dance? dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by call *foo@TLSCALL(%rax) (gdb) bt #0 test () at lib.s:20 #1 0x00401075 in main () at main.c:13 (gdb) f 0 #0 test () at lib.s:20 20 addq %rax, %r12 (gdb) disass Dump of assembler code for function test: 0xf7fca120 <+0>: push %r12 0xf7fca122 <+2>: lea 0x2ef7(%rip),%rax # 0xf7fcd020 <foo@got.plt> 0xf7fca129 <+9>: lea 0xed0(%rip),%rdi # 0xf7fcb000 0xf7fca130 <+16>: mov %fs:0x0,%r12d 0xf7fca139 <+25>: callq *(%rax) => 0xf7fca13b <+27>: add %rax,%r12 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Wrong address in R12. 0xf7fca13e <+30>: xor %eax,%eax 0xf7fca140 <+32>: mov (%r12),%esi 0xf7fca144 <+36>: callq 0xf7fca030 <printf@plt> 0xf7fca149 <+41>: mov %r12,%rax 0xf7fca14c <+44>: pop %r12 0xf7fca14e <+46>: retq End of assembler dump. (gdb) p/x $rax $6 = 0xfffffffc (gdb) c Continuing. Program received signal SIGSEGV, Segmentation fault. test () at lib.s:22 22 movl (%r12), %esi (gdb) p/x $r12 $7 = 0x1f7df96fc (gdb) > Uros. > > > > + if (Pmode != ptr_mode) > > > + { > > > + tp = lowpart_subreg (ptr_mode, tp, Pmode); > > > + dest = lowpart_subreg (ptr_mode, dest, Pmode); > > > + dest = gen_rtx_PLUS (ptr_mode, tp, dest); > > > + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); > > > + } > > > + else > > > + dest = gen_rtx_PLUS (Pmode, tp, dest); > > > + dest = force_reg (Pmode, dest); > > > > Sholdn't we use > > > > tp = get_thread_pointer (ptr_mode, true); > > > > then? If Pmode == ptr_mode, then we have the same functionality, > > otherwise we don't have to subreg tp to ptr_mode. > > > > Can we use the same approach as in ix86_zero_extend_to_Pmode here, like: > > > > dest = gen_rtx_PLUS (ptr_mode, tp, dest); > > dest = force_reg (Pmode, convert_to_mode (Pmode, dest, 1)); > > > > Uros.
On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > Please use "lea%z0" instead. > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > Thanks. > > > > > > > > H.J. > > > > --- > > > > gcc/ > > > > > > > > PR target/93319 > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > > (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. > > > > Remove the {q} suffix from lea. > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > gcc/testsuite/ > > > > > > > > PR target/93319 > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > --- > > > > gcc/config/i386/i386.c | 31 +++++++++++-- > > > > gcc/config/i386/i386.md | 54 +++++++++++----------- > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > --- a/gcc/config/i386/i386.c > > > > +++ b/gcc/config/i386/i386.c > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > > > > if (TARGET_GNU2_TLS) > > > > { > > > > if (TARGET_64BIT) > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > else > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > > + > > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > > + PLUS is done in ptr_mode. */ > > > > Actually, thread_pointer is in Pmode, see the line just above your > > change. Also, dest is in Pmode, so why do we need all this subreg > > dance? > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > call *foo@TLSCALL(%rax) It looks to me that we have Pmode/ptr_mode mixup in legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we need to perform all calculations in ptr_mode, since get_thread_pointer in effect always returns ptr_mode (SImode) value, and also the above call always returns ptr_mode (SImode) value. In case of -maddress-mode=long, the value would be zero-extended to Pmode. Uros.
On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > > > Please use "lea%z0" instead. > > > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > > > Thanks. > > > > > > > > > > H.J. > > > > > --- > > > > > gcc/ > > > > > > > > > > PR target/93319 > > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > > > (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. > > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > > (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. > > > > > Remove the {q} suffix from lea. > > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > > (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. > > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > > (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. > > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > PR target/93319 > > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > > --- > > > > > gcc/config/i386/i386.c | 31 +++++++++++-- > > > > > gcc/config/i386/i386.md | 54 +++++++++++----------- > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > > --- a/gcc/config/i386/i386.c > > > > > +++ b/gcc/config/i386/i386.c > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > > > > > if (TARGET_GNU2_TLS) > > > > > { > > > > > if (TARGET_64BIT) > > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > > else > > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > > > + > > > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > > > + PLUS is done in ptr_mode. */ > > > > > > Actually, thread_pointer is in Pmode, see the line just above your > > > change. Also, dest is in Pmode, so why do we need all this subreg > > > dance? > > > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > > > call *foo@TLSCALL(%rax) > > It looks to me that we have Pmode/ptr_mode mixup in > legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we > need to perform all calculations in ptr_mode, since get_thread_pointer > in effect always returns ptr_mode (SImode) value, and also the above > call always returns ptr_mode (SImode) value. In case of > -maddress-mode=long, the value would be zero-extended to Pmode. > The problem is with tls_dynamic_gnu2_64 which generates call *foo@TLSCALL(%rax) It always returns a 32-bit index. I can change get_thread_pointer if needed. Here is the updated patch with "lea%z0" and updated comments.
On Sun, Jan 19, 2020 at 9:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > > > > > Please use "lea%z0" instead. > > > > > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > > > > > Thanks. > > > > > > > > > > > > H.J. > > > > > > --- > > > > > > gcc/ > > > > > > > > > > > > PR target/93319 > > > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > > > > (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. > > > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > > > (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. > > > > > > Remove the {q} suffix from lea. > > > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > > > (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. > > > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > > > (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. > > > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > > > PR target/93319 > > > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > > > --- > > > > > > gcc/config/i386/i386.c | 31 +++++++++++-- > > > > > > gcc/config/i386/i386.md | 54 +++++++++++----------- > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > > > --- a/gcc/config/i386/i386.c > > > > > > +++ b/gcc/config/i386/i386.c > > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > > > > > > if (TARGET_GNU2_TLS) > > > > > > { > > > > > > if (TARGET_64BIT) > > > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > > > else > > > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > > > > + > > > > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > > > > + PLUS is done in ptr_mode. */ > > > > > > > > Actually, thread_pointer is in Pmode, see the line just above your > > > > change. Also, dest is in Pmode, so why do we need all this subreg > > > > dance? > > > > > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > > > > > call *foo@TLSCALL(%rax) > > > > It looks to me that we have Pmode/ptr_mode mixup in > > legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we > > need to perform all calculations in ptr_mode, since get_thread_pointer > > in effect always returns ptr_mode (SImode) value, and also the above > > call always returns ptr_mode (SImode) value. In case of > > -maddress-mode=long, the value would be zero-extended to Pmode. > > > > The problem is with tls_dynamic_gnu2_64 which generates > > call *foo@TLSCALL(%rax) > > It always returns a 32-bit index. I can change get_thread_pointer if > needed. > > Here is the updated patch with "lea%z0" and updated comments. There is && 0 in the first if, which looks wrong. + /* NB: Since DEST set by tls_dynamic_gnu2_64 is in ptr_mode, + make sure that PLUS is done in ptr_mode. */ + if (0 && Pmode != ptr_mode) + { Uros.
On Sun, Jan 19, 2020 at 12:16 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > On Sun, Jan 19, 2020 at 9:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > > > > > > > Please use "lea%z0" instead. > > > > > > > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > > > > H.J. > > > > > > > --- > > > > > > > gcc/ > > > > > > > > > > > > > > PR target/93319 > > > > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > > > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > > > > > (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. > > > > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > > > > (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. > > > > > > > Remove the {q} suffix from lea. > > > > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > > > > (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. > > > > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > > > > (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. > > > > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > > > > > PR target/93319 > > > > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > > > > --- > > > > > > > gcc/config/i386/i386.c | 31 +++++++++++-- > > > > > > > gcc/config/i386/i386.md | 54 +++++++++++----------- > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > > > > --- a/gcc/config/i386/i386.c > > > > > > > +++ b/gcc/config/i386/i386.c > > > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > > > > > > > if (TARGET_GNU2_TLS) > > > > > > > { > > > > > > > if (TARGET_64BIT) > > > > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > > > > else > > > > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > > > > > + > > > > > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > > > > > + PLUS is done in ptr_mode. */ > > > > > > > > > > Actually, thread_pointer is in Pmode, see the line just above your > > > > > change. Also, dest is in Pmode, so why do we need all this subreg > > > > > dance? > > > > > > > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > > > > > > > call *foo@TLSCALL(%rax) > > > > > > It looks to me that we have Pmode/ptr_mode mixup in > > > legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we > > > need to perform all calculations in ptr_mode, since get_thread_pointer > > > in effect always returns ptr_mode (SImode) value, and also the above > > > call always returns ptr_mode (SImode) value. In case of > > > -maddress-mode=long, the value would be zero-extended to Pmode. > > > > > > > The problem is with tls_dynamic_gnu2_64 which generates > > > > call *foo@TLSCALL(%rax) > > > > It always returns a 32-bit index. I can change get_thread_pointer if > > needed. > > > > Here is the updated patch with "lea%z0" and updated comments. > > There is && 0 in the first if, which looks wrong. > > + /* NB: Since DEST set by tls_dynamic_gnu2_64 is in ptr_mode, > + make sure that PLUS is done in ptr_mode. */ > + if (0 && Pmode != ptr_mode) > + { > Oops. Here is the fixed patch.
On Sun, Jan 19, 2020 at 10:00 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Sun, Jan 19, 2020 at 12:16 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > On Sun, Jan 19, 2020 at 9:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > > > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > > > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > > > > > > > > > Please use "lea%z0" instead. > > > > > > > > > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > > > > > > H.J. > > > > > > > > --- > > > > > > > > gcc/ > > > > > > > > > > > > > > > > PR target/93319 > > > > > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > > > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > > > > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > > > > > > (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. > > > > > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > > > > > (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. > > > > > > > > Remove the {q} suffix from lea. > > > > > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > > > > > (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. > > > > > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > > > > > (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. > > > > > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > > > > > > > PR target/93319 > > > > > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > > > > > --- > > > > > > > > gcc/config/i386/i386.c | 31 +++++++++++-- > > > > > > > > gcc/config/i386/i386.md | 54 +++++++++++----------- > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > > > > > --- a/gcc/config/i386/i386.c > > > > > > > > +++ b/gcc/config/i386/i386.c > > > > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > > > > > > > > if (TARGET_GNU2_TLS) > > > > > > > > { > > > > > > > > if (TARGET_64BIT) > > > > > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > > > > > else > > > > > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > > > > > > + > > > > > > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > > > > > > + PLUS is done in ptr_mode. */ > > > > > > > > > > > > Actually, thread_pointer is in Pmode, see the line just above your > > > > > > change. Also, dest is in Pmode, so why do we need all this subreg > > > > > > dance? > > > > > > > > > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > > > > > > > > > call *foo@TLSCALL(%rax) > > > > > > > > It looks to me that we have Pmode/ptr_mode mixup in > > > > legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we > > > > need to perform all calculations in ptr_mode, since get_thread_pointer > > > > in effect always returns ptr_mode (SImode) value, and also the above > > > > call always returns ptr_mode (SImode) value. In case of > > > > -maddress-mode=long, the value would be zero-extended to Pmode. > > > > > > > > > > The problem is with tls_dynamic_gnu2_64 which generates > > > > > > call *foo@TLSCALL(%rax) > > > > > > It always returns a 32-bit index. I can change get_thread_pointer if > > > needed. > > > > > > Here is the updated patch with "lea%z0" and updated comments. > > > > There is && 0 in the first if, which looks wrong. > > > > + /* NB: Since DEST set by tls_dynamic_gnu2_64 is in ptr_mode, > > + make sure that PLUS is done in ptr_mode. */ > > + if (0 && Pmode != ptr_mode) > > + { > > > > Oops. Here is the fixed patch. OK. Let's go with this version, but please investigate if we need to calculate TLS address in ptr_mode instead of Pmode. Due to quite some zero-extension from ptr_mode to Pmode hacks in this area, it looks to me that the whole calculation should be performed in ptr_mode (SImode in case of x32), and the result zero-extended to Pmode in case when Pmode = DImode. Thanks, Uros.
On Sun, Jan 19, 2020 at 11:53 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > On Sun, Jan 19, 2020 at 10:00 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Sun, Jan 19, 2020 at 12:16 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > On Sun, Jan 19, 2020 at 9:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > > > > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > > > > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > > > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > > > > > > > > > > > Please use "lea%z0" instead. > > > > > > > > > > > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > > > > > > > > H.J. > > > > > > > > > --- > > > > > > > > > gcc/ > > > > > > > > > > > > > > > > > > PR target/93319 > > > > > > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > > > > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > > > > > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > > > > > > > (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. > > > > > > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > > > > > > (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. > > > > > > > > > Remove the {q} suffix from lea. > > > > > > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > > > > > > (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. > > > > > > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > > > > > > (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. > > > > > > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > > > > > > > > > PR target/93319 > > > > > > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > > > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > > > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > > > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > > > > > > --- > > > > > > > > > gcc/config/i386/i386.c | 31 +++++++++++-- > > > > > > > > > gcc/config/i386/i386.md | 54 +++++++++++----------- > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > > > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > > > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > > > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > > > > > > --- a/gcc/config/i386/i386.c > > > > > > > > > +++ b/gcc/config/i386/i386.c > > > > > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > > > > > > > > > if (TARGET_GNU2_TLS) > > > > > > > > > { > > > > > > > > > if (TARGET_64BIT) > > > > > > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > > > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > > > > > > else > > > > > > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > > > > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > > > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > > > > > > > + > > > > > > > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > > > > > > > + PLUS is done in ptr_mode. */ > > > > > > > > > > > > > > Actually, thread_pointer is in Pmode, see the line just above your > > > > > > > change. Also, dest is in Pmode, so why do we need all this subreg > > > > > > > dance? > > > > > > > > > > > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > > > > > > > > > > > call *foo@TLSCALL(%rax) > > > > > > > > > > It looks to me that we have Pmode/ptr_mode mixup in > > > > > legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we > > > > > need to perform all calculations in ptr_mode, since get_thread_pointer > > > > > in effect always returns ptr_mode (SImode) value, and also the above > > > > > call always returns ptr_mode (SImode) value. In case of > > > > > -maddress-mode=long, the value would be zero-extended to Pmode. > > > > > > > > > > > > > The problem is with tls_dynamic_gnu2_64 which generates > > > > > > > > call *foo@TLSCALL(%rax) > > > > > > > > It always returns a 32-bit index. I can change get_thread_pointer if > > > > needed. > > > > > > > > Here is the updated patch with "lea%z0" and updated comments. > > > > > > There is && 0 in the first if, which looks wrong. > > > > > > + /* NB: Since DEST set by tls_dynamic_gnu2_64 is in ptr_mode, > > > + make sure that PLUS is done in ptr_mode. */ > > > + if (0 && Pmode != ptr_mode) > > > + { > > > > > > > Oops. Here is the fixed patch. > > OK. Let's go with this version, but please investigate if we need to > calculate TLS address in ptr_mode instead of Pmode. Due to quite some > zero-extension from ptr_mode to Pmode hacks in this area, it looks to > me that the whole calculation should be performed in ptr_mode (SImode > in case of x32), and the result zero-extended to Pmode in case when > Pmode = DImode. > I checked it in. I will investigate if we can use ptr_mode for TLS. Thanks.
On Mon, Jan 20, 2020 at 5:24 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Sun, Jan 19, 2020 at 11:53 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > On Sun, Jan 19, 2020 at 10:00 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Sun, Jan 19, 2020 at 12:16 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > On Sun, Jan 19, 2020 at 9:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > > On Sun, Jan 19, 2020 at 12:01 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > > > On Sun, Jan 19, 2020 at 7:07 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > > > > > > On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > > > > > > > On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > > > > > > > > > > > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > To add x32 support to -mtls-dialect=gnu2, we need to replace DI with > > > > > > > > > > P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in > > > > > > > > > > GNU2 TLS address computation must be done in ptr_mode to support > > > > > > > > > > -maddress-mode=long. Also drop the "q" suffix from lea to support > > > > > > > > > > both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". > > > > > > > > > > > > > > > > > > Please use "lea%z0" instead. > > > > > > > > > > > > > > > > > > > Tested on Linux/x86-64. OK for master? > > > > > > > > > > > > > > > > > > > > Thanks. > > > > > > > > > > > > > > > > > > > > H.J. > > > > > > > > > > --- > > > > > > > > > > gcc/ > > > > > > > > > > > > > > > > > > > > PR target/93319 > > > > > > > > > > * config/i386/i386.c (legitimize_tls_address): Pass Pmode to > > > > > > > > > > gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. > > > > > > > > > > * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... > > > > > > > > > > (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. > > > > > > > > > > (*tls_dynamic_gnu2_lea_64): Renamed to ... > > > > > > > > > > (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. > > > > > > > > > > Remove the {q} suffix from lea. > > > > > > > > > > (*tls_dynamic_gnu2_call_64): Renamed to ... > > > > > > > > > > (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. > > > > > > > > > > (*tls_dynamic_gnu2_combine_64): Renamed to ... > > > > > > > > > > (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. > > > > > > > > > > Pass Pmode to gen_tls_dynamic_gnu2_64. > > > > > > > > > > > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > > > > > > > > > > > PR target/93319 > > > > > > > > > > * gcc.target/i386/pr93319-1a.c: New test. > > > > > > > > > > * gcc.target/i386/pr93319-1b.c: Likewise. > > > > > > > > > > * gcc.target/i386/pr93319-1c.c: Likewise. > > > > > > > > > > * gcc.target/i386/pr93319-1d.c: Likewise. > > > > > > > > > > --- > > > > > > > > > > gcc/config/i386/i386.c | 31 +++++++++++-- > > > > > > > > > > gcc/config/i386/i386.md | 54 +++++++++++----------- > > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ > > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ > > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ > > > > > > > > > > gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ > > > > > > > > > > 6 files changed, 99 insertions(+), 31 deletions(-) > > > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c > > > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c > > > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c > > > > > > > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c > > > > > > > > > > > > > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > > > > > > > > > > index 2c087a4a3e0..8c437dbe1f3 100644 > > > > > > > > > > --- a/gcc/config/i386/i386.c > > > > > > > > > > +++ b/gcc/config/i386/i386.c > > > > > > > > > > @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) > > > > > > > > > > if (TARGET_GNU2_TLS) > > > > > > > > > > { > > > > > > > > > > if (TARGET_64BIT) > > > > > > > > > > - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); > > > > > > > > > > + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); > > > > > > > > > > else > > > > > > > > > > emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); > > > > > > > > > > > > > > > > > > > > tp = get_thread_pointer (Pmode, true); > > > > > > > > > > - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); > > > > > > > > > > + > > > > > > > > > > + /* NB: Since thread pointer is in ptr_mode, make sure that > > > > > > > > > > + PLUS is done in ptr_mode. */ > > > > > > > > > > > > > > > > Actually, thread_pointer is in Pmode, see the line just above your > > > > > > > > change. Also, dest is in Pmode, so why do we need all this subreg > > > > > > > > dance? > > > > > > > > > > > > > > dest set from gen_tls_dynamic_gnu2_64 is in ptr_mode by > > > > > > > > > > > > > > call *foo@TLSCALL(%rax) > > > > > > > > > > > > It looks to me that we have Pmode/ptr_mode mixup in > > > > > > legitimize_tls_address & friends for TARGET_GNU2_TLS. I think that we > > > > > > need to perform all calculations in ptr_mode, since get_thread_pointer > > > > > > in effect always returns ptr_mode (SImode) value, and also the above > > > > > > call always returns ptr_mode (SImode) value. In case of > > > > > > -maddress-mode=long, the value would be zero-extended to Pmode. > > > > > > > > > > > > > > > > The problem is with tls_dynamic_gnu2_64 which generates > > > > > > > > > > call *foo@TLSCALL(%rax) > > > > > > > > > > It always returns a 32-bit index. I can change get_thread_pointer if > > > > > needed. > > > > > > > > > > Here is the updated patch with "lea%z0" and updated comments. > > > > > > > > There is && 0 in the first if, which looks wrong. > > > > > > > > + /* NB: Since DEST set by tls_dynamic_gnu2_64 is in ptr_mode, > > > > + make sure that PLUS is done in ptr_mode. */ > > > > + if (0 && Pmode != ptr_mode) > > > > + { > > > > > > > > > > Oops. Here is the fixed patch. > > > > OK. Let's go with this version, but please investigate if we need to > > calculate TLS address in ptr_mode instead of Pmode. Due to quite some > > zero-extension from ptr_mode to Pmode hacks in this area, it looks to > > me that the whole calculation should be performed in ptr_mode (SImode > > in case of x32), and the result zero-extended to Pmode in case when > > Pmode = DImode. > > > > I checked it in. I will investigate if we can use ptr_mode for TLS. Here is a patch to perform GNU2 TLS address computation in ptr_mode and zero-extend result to Pmode.
On Mon, Jan 20, 2020 at 10:46 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > OK. Let's go with this version, but please investigate if we need to > > > calculate TLS address in ptr_mode instead of Pmode. Due to quite some > > > zero-extension from ptr_mode to Pmode hacks in this area, it looks to > > > me that the whole calculation should be performed in ptr_mode (SImode > > > in case of x32), and the result zero-extended to Pmode in case when > > > Pmode = DImode. > > > > > > > I checked it in. I will investigate if we can use ptr_mode for TLS. > > Here is a patch to perform GNU2 TLS address computation in ptr_mode > and zero-extend result to Pmode. case TLS_MODEL_GLOBAL_DYNAMIC: - dest = gen_reg_rtx (Pmode); + dest = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode); Please put these in their respective arms of "if (TARGET_GNU2_TLS). case TLS_MODEL_LOCAL_DYNAMIC: - base = gen_reg_rtx (Pmode); + base = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode); Also here. A question: Do we need to emit the following part in Pmode? off = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, x), UNSPEC_DTPOFF); off = gen_rtx_CONST (Pmode, off); dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, base, off)); If this can operate in ptr_mode then ... - emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, base, tmp)); + { + emit_insn (gen_tls_dynamic_gnu2_64 (ptr_mode, base, tmp)); + if (GET_MODE (base) != Pmode) + base = gen_rtx_ZERO_EXTEND (Pmode, base); + } ... please keep base in ptr_mode, no need to zero_extend it. - tp = get_thread_pointer (Pmode, true); - set_unique_reg_note (get_last_insn (), REG_EQUAL, - gen_rtx_MINUS (Pmode, tmp, tp)); Just change the above to ptr_mode. There is no zero_extend needed here, because: --cut here-- off = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, x), UNSPEC_DTPOFF); off = gen_rtx_CONST (Pmode, off); dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, base, off)); if (TARGET_GNU2_TLS) { if (GET_MODE (tp) != Pmode) { dest = lowpart_subreg (ptr_mode, dest, Pmode); dest = gen_rtx_PLUS (ptr_mode, tp, dest); dest = gen_rtx_ZERO_EXTEND (Pmode, dest); } else dest = gen_rtx_PLUS (ptr_mode, tp, dest); dest = force_reg (Pmode, dest); --cut here-- all the above part should operate in ptr_mode (for TARGET_GNU2_TLS at least), followed by the same part of --cut here-- dest = gen_rtx_PLUS (ptr_mode, tp, dest); if (GET_MODE (dest) != Pmode) dest = gen_rtx_ZERO_EXTEND (Pmode, dest); dest = force_reg (Pmode, dest); if (GET_MODE (x) != Pmode) x = gen_rtx_ZERO_EXTEND (Pmode, x); set_unique_reg_note (get_last_insn (), REG_EQUAL, x); --cut here-- as is the case with TLS_MODEL_GLOBAL_DYNAMIC. Uros.
On Tue, Jan 21, 2020 at 9:47 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > On Mon, Jan 20, 2020 at 10:46 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > OK. Let's go with this version, but please investigate if we need to > > > > calculate TLS address in ptr_mode instead of Pmode. Due to quite some > > > > zero-extension from ptr_mode to Pmode hacks in this area, it looks to > > > > me that the whole calculation should be performed in ptr_mode (SImode > > > > in case of x32), and the result zero-extended to Pmode in case when > > > > Pmode = DImode. > > > > > > > > > > I checked it in. I will investigate if we can use ptr_mode for TLS. > > > > Here is a patch to perform GNU2 TLS address computation in ptr_mode > > and zero-extend result to Pmode. > > case TLS_MODEL_GLOBAL_DYNAMIC: > - dest = gen_reg_rtx (Pmode); > + dest = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode); > > Please put these in their respective arms of "if (TARGET_GNU2_TLS). > > case TLS_MODEL_LOCAL_DYNAMIC: > - base = gen_reg_rtx (Pmode); > + base = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode); > > Also here. > > A question: Do we need to emit the following part in Pmode? To answer my own question: Yes. Linker doesn't like SImode relocs fox x86_64 and for addl $foo@dtpoff, %eax errors out with: pr93319-1a.s: Assembler messages: pr93319-1a.s:20: Error: relocated field and relocation type differ in signedness So, the part below is OK, except: - tp = get_thread_pointer (Pmode, true); - set_unique_reg_note (get_last_insn (), REG_EQUAL, - gen_rtx_MINUS (Pmode, tmp, tp)); + tp = get_thread_pointer (ptr_mode, true); + tmp = gen_rtx_MINUS (ptr_mode, tmp, tp); + if (GET_MODE (tmp) != Pmode) + tmp = gen_rtx_ZERO_EXTEND (Pmode, tmp); + set_unique_reg_note (get_last_insn (), REG_EQUAL, tmp); I don't think we should attach this note to the thread pointer initialization. I have removed this part from the patch, but please review the decision. and - dest = gen_rtx_PLUS (Pmode, tp, dest); + dest = gen_rtx_PLUS (ptr_mode, tp, dest); Please leave Pmode here. ptr_mode == Pmode at this point, but Pmode better documents the mode selection logic. Also, the tests fail for me with: /usr/include/gnu/stubs.h:13:11: fatal error: gnu/stubs-x32.h: No such file or directory so either use __builtin_printf or some other approach that doesn't need to #include stdio.h. A patch that implements above changes is attached to the message. Uros. diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 0b8a4b9ee4f0..ffe60baa72ad 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -10717,7 +10717,7 @@ ix86_tls_module_base (void) if (!ix86_tls_module_base_symbol) { ix86_tls_module_base_symbol - = gen_rtx_SYMBOL_REF (Pmode, "_TLS_MODULE_BASE_"); + = gen_rtx_SYMBOL_REF (ptr_mode, "_TLS_MODULE_BASE_"); SYMBOL_REF_FLAGS (ix86_tls_module_base_symbol) |= TLS_MODEL_GLOBAL_DYNAMIC << SYMBOL_FLAG_TLS_SHIFT; @@ -10748,8 +10748,6 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) switch (model) { case TLS_MODEL_GLOBAL_DYNAMIC: - dest = gen_reg_rtx (Pmode); - if (!TARGET_64BIT) { if (flag_pic && !TARGET_PECOFF) @@ -10763,24 +10761,16 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) if (TARGET_GNU2_TLS) { + dest = gen_reg_rtx (ptr_mode); if (TARGET_64BIT) - emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); + emit_insn (gen_tls_dynamic_gnu2_64 (ptr_mode, dest, x)); else emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); - tp = get_thread_pointer (Pmode, true); - - /* NB: Since DEST set by tls_dynamic_gnu2_64 is in ptr_mode, - make sure that PLUS is done in ptr_mode. */ - if (Pmode != ptr_mode) - { - tp = lowpart_subreg (ptr_mode, tp, Pmode); - dest = lowpart_subreg (ptr_mode, dest, Pmode); - dest = gen_rtx_PLUS (ptr_mode, tp, dest); - dest = gen_rtx_ZERO_EXTEND (Pmode, dest); - } - else - dest = gen_rtx_PLUS (Pmode, tp, dest); + tp = get_thread_pointer (ptr_mode, true); + dest = gen_rtx_PLUS (ptr_mode, tp, dest); + if (GET_MODE (dest) != Pmode) + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); dest = force_reg (Pmode, dest); if (GET_MODE (x) != Pmode) @@ -10792,6 +10782,7 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) { rtx caddr = ix86_tls_get_addr (); + dest = gen_reg_rtx (Pmode); if (TARGET_64BIT) { rtx rax = gen_rtx_REG (Pmode, AX_REG); @@ -10815,8 +10806,6 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) break; case TLS_MODEL_LOCAL_DYNAMIC: - base = gen_reg_rtx (Pmode); - if (!TARGET_64BIT) { if (flag_pic) @@ -10832,19 +10821,22 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) { rtx tmp = ix86_tls_module_base (); + base = gen_reg_rtx (ptr_mode); if (TARGET_64BIT) - emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, base, tmp)); + emit_insn (gen_tls_dynamic_gnu2_64 (ptr_mode, base, tmp)); else emit_insn (gen_tls_dynamic_gnu2_32 (base, tmp, pic)); - tp = get_thread_pointer (Pmode, true); - set_unique_reg_note (get_last_insn (), REG_EQUAL, - gen_rtx_MINUS (Pmode, tmp, tp)); + tp = get_thread_pointer (ptr_mode, true); + if (GET_MODE (base) != Pmode) + base = gen_rtx_ZERO_EXTEND (Pmode, base); + base = force_reg (Pmode, base); } else { rtx caddr = ix86_tls_get_addr (); + base = gen_reg_rtx (Pmode); if (TARGET_64BIT) { rtx rax = gen_rtx_REG (Pmode, AX_REG); @@ -10876,11 +10868,8 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) if (TARGET_GNU2_TLS) { - /* NB: Since DEST set by tls_dynamic_gnu2_64 is in ptr_mode, - make sure that PLUS is done in ptr_mode. */ - if (Pmode != ptr_mode) + if (GET_MODE (tp) != Pmode) { - tp = lowpart_subreg (ptr_mode, tp, Pmode); dest = lowpart_subreg (ptr_mode, dest, Pmode); dest = gen_rtx_PLUS (ptr_mode, tp, dest); dest = gen_rtx_ZERO_EXTEND (Pmode, dest); diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index de335cb8f029..6c674aaea5bf 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -15187,23 +15187,23 @@ (define_expand "@tls_dynamic_gnu2_64_<mode>" [(set (match_dup 2) - (unspec:P [(match_operand 1 "tls_symbolic_operand")] - UNSPEC_TLSDESC)) - (parallel - [(set (match_operand:P 0 "register_operand") - (unspec:P [(match_dup 1) (match_dup 2) (reg:P SP_REG)] + (unspec:PTR [(match_operand 1 "tls_symbolic_operand")] UNSPEC_TLSDESC)) + (parallel + [(set (match_operand:PTR 0 "register_operand") + (unspec:PTR [(match_dup 1) (match_dup 2) (reg:PTR SP_REG)] + UNSPEC_TLSDESC)) (clobber (reg:CC FLAGS_REG))])] "TARGET_64BIT && TARGET_GNU2_TLS" { - operands[2] = can_create_pseudo_p () ? gen_reg_rtx (Pmode) : operands[0]; + operands[2] = can_create_pseudo_p () ? gen_reg_rtx (ptr_mode) : operands[0]; ix86_tls_descriptor_calls_expanded_in_cfun = true; }) (define_insn "*tls_dynamic_gnu2_lea_64_<mode>" - [(set (match_operand:P 0 "register_operand" "=r") - (unspec:P [(match_operand 1 "tls_symbolic_operand")] - UNSPEC_TLSDESC))] + [(set (match_operand:PTR 0 "register_operand" "=r") + (unspec:PTR [(match_operand 1 "tls_symbolic_operand")] + UNSPEC_TLSDESC))] "TARGET_64BIT && TARGET_GNU2_TLS" "lea%z0\t{%E1@TLSDESC(%%rip), %0|%0, %E1@TLSDESC[rip]}" [(set_attr "type" "lea") @@ -15212,10 +15212,10 @@ (set_attr "length_address" "4")]) (define_insn "*tls_dynamic_gnu2_call_64_<mode>" - [(set (match_operand:P 0 "register_operand" "=a") - (unspec:P [(match_operand 1 "tls_symbolic_operand") - (match_operand:P 2 "register_operand" "0") - (reg:P SP_REG)] + [(set (match_operand:PTR 0 "register_operand" "=a") + (unspec:PTR [(match_operand 1 "tls_symbolic_operand") + (match_operand:PTR 2 "register_operand" "0") + (reg:PTR SP_REG)] UNSPEC_TLSDESC)) (clobber (reg:CC FLAGS_REG))] "TARGET_64BIT && TARGET_GNU2_TLS" @@ -15225,23 +15225,23 @@ (set_attr "length_address" "0")]) (define_insn_and_split "*tls_dynamic_gnu2_combine_64_<mode>" - [(set (match_operand:P 0 "register_operand" "=&a") - (plus:P - (unspec:P [(match_operand 2 "tls_modbase_operand") - (match_operand:P 3) - (reg:P SP_REG)] - UNSPEC_TLSDESC) - (const:P (unspec:P - [(match_operand 1 "tls_symbolic_operand")] - UNSPEC_DTPOFF)))) + [(set (match_operand:PTR 0 "register_operand" "=&a") + (plus:PTR + (unspec:PTR [(match_operand 2 "tls_modbase_operand") + (match_operand:PTR 3) + (reg:PTR SP_REG)] + UNSPEC_TLSDESC) + (const:PTR (unspec:PTR + [(match_operand 1 "tls_symbolic_operand")] + UNSPEC_DTPOFF)))) (clobber (reg:CC FLAGS_REG))] "TARGET_64BIT && TARGET_GNU2_TLS" "#" "" [(set (match_dup 0) (match_dup 4))] { - operands[4] = can_create_pseudo_p () ? gen_reg_rtx (Pmode) : operands[0]; - emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, operands[4], operands[1])); + operands[4] = can_create_pseudo_p () ? gen_reg_rtx (ptr_mode) : operands[0]; + emit_insn (gen_tls_dynamic_gnu2_64 (ptr_mode, operands[4], operands[1])); }) (define_split diff --git a/gcc/testsuite/gcc.target/i386/pr93319-1a.c b/gcc/testsuite/gcc.target/i386/pr93319-1a.c index 8f6b4af3225e..122c111d0cbb 100644 --- a/gcc/testsuite/gcc.target/i386/pr93319-1a.c +++ b/gcc/testsuite/gcc.target/i386/pr93319-1a.c @@ -4,21 +4,19 @@ /* { dg-require-effective-target tls_native } */ /* { dg-options "-mx32 -fPIC -mtls-dialect=gnu2" } */ -#include <stdio.h> - extern __thread int bar; static __thread int foo = 30; int * test1 (void) { - printf ("foo: %d\n", foo); + __builtin_printf ("foo: %d\n", foo); return &foo; } int * test2 (void) { - printf ("bar: %d\n", bar); + __builtin_printf ("bar: %d\n", bar); return &bar; }
On Tue, Jan 21, 2020 at 2:29 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > On Tue, Jan 21, 2020 at 9:47 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > On Mon, Jan 20, 2020 at 10:46 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > OK. Let's go with this version, but please investigate if we need to > > > > > calculate TLS address in ptr_mode instead of Pmode. Due to quite some > > > > > zero-extension from ptr_mode to Pmode hacks in this area, it looks to > > > > > me that the whole calculation should be performed in ptr_mode (SImode > > > > > in case of x32), and the result zero-extended to Pmode in case when > > > > > Pmode = DImode. > > > > > > > > > > > > > I checked it in. I will investigate if we can use ptr_mode for TLS. > > > > > > Here is a patch to perform GNU2 TLS address computation in ptr_mode > > > and zero-extend result to Pmode. > > > > case TLS_MODEL_GLOBAL_DYNAMIC: > > - dest = gen_reg_rtx (Pmode); > > + dest = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode); > > > > Please put these in their respective arms of "if (TARGET_GNU2_TLS). > > > > case TLS_MODEL_LOCAL_DYNAMIC: > > - base = gen_reg_rtx (Pmode); > > + base = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode); > > > > Also here. > > > > A question: Do we need to emit the following part in Pmode? > > To answer my own question: Yes. Linker doesn't like SImode relocs fox > x86_64 and for > > addl $foo@dtpoff, %eax > > errors out with: > > pr93319-1a.s: Assembler messages: > pr93319-1a.s:20: Error: relocated field and relocation type differ in signedness > > So, the part below is OK, except: > > - tp = get_thread_pointer (Pmode, true); > - set_unique_reg_note (get_last_insn (), REG_EQUAL, > - gen_rtx_MINUS (Pmode, tmp, tp)); > + tp = get_thread_pointer (ptr_mode, true); > + tmp = gen_rtx_MINUS (ptr_mode, tmp, tp); > + if (GET_MODE (tmp) != Pmode) > + tmp = gen_rtx_ZERO_EXTEND (Pmode, tmp); > + set_unique_reg_note (get_last_insn (), REG_EQUAL, tmp); > > I don't think we should attach this note to the thread pointer > initialization. I have removed this part from the patch, but please > review the decision. > > and > > - dest = gen_rtx_PLUS (Pmode, tp, dest); > + dest = gen_rtx_PLUS (ptr_mode, tp, dest); > > Please leave Pmode here. ptr_mode == Pmode at this point, but Pmode > better documents the mode selection logic. > > Also, the tests fail for me with: > > /usr/include/gnu/stubs.h:13:11: fatal error: gnu/stubs-x32.h: No such > file or directory > > so either use __builtin_printf or some other approach that doesn't > need to #include stdio.h. > > A patch that implements above changes is attached to the message. > Here is the updated patch. OK for master? Thanks.
On Tue, Jan 21, 2020 at 8:16 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Jan 21, 2020 at 2:29 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > On Tue, Jan 21, 2020 at 9:47 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > On Mon, Jan 20, 2020 at 10:46 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > > > > OK. Let's go with this version, but please investigate if we need to > > > > > > calculate TLS address in ptr_mode instead of Pmode. Due to quite some > > > > > > zero-extension from ptr_mode to Pmode hacks in this area, it looks to > > > > > > me that the whole calculation should be performed in ptr_mode (SImode > > > > > > in case of x32), and the result zero-extended to Pmode in case when > > > > > > Pmode = DImode. > > > > > > > > > > > > > > > > I checked it in. I will investigate if we can use ptr_mode for TLS. > > > > > > > > Here is a patch to perform GNU2 TLS address computation in ptr_mode > > > > and zero-extend result to Pmode. > > > > > > case TLS_MODEL_GLOBAL_DYNAMIC: > > > - dest = gen_reg_rtx (Pmode); > > > + dest = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode); > > > > > > Please put these in their respective arms of "if (TARGET_GNU2_TLS). > > > > > > case TLS_MODEL_LOCAL_DYNAMIC: > > > - base = gen_reg_rtx (Pmode); > > > + base = gen_reg_rtx (TARGET_GNU2_TLS ? ptr_mode : Pmode); > > > > > > Also here. > > > > > > A question: Do we need to emit the following part in Pmode? > > > > To answer my own question: Yes. Linker doesn't like SImode relocs fox > > x86_64 and for > > > > addl $foo@dtpoff, %eax > > > > errors out with: > > > > pr93319-1a.s: Assembler messages: > > pr93319-1a.s:20: Error: relocated field and relocation type differ in signedness > > > > So, the part below is OK, except: > > > > - tp = get_thread_pointer (Pmode, true); > > - set_unique_reg_note (get_last_insn (), REG_EQUAL, > > - gen_rtx_MINUS (Pmode, tmp, tp)); > > + tp = get_thread_pointer (ptr_mode, true); > > + tmp = gen_rtx_MINUS (ptr_mode, tmp, tp); > > + if (GET_MODE (tmp) != Pmode) > > + tmp = gen_rtx_ZERO_EXTEND (Pmode, tmp); > > + set_unique_reg_note (get_last_insn (), REG_EQUAL, tmp); > > > > I don't think we should attach this note to the thread pointer > > initialization. I have removed this part from the patch, but please > > review the decision. > > > > and > > > > - dest = gen_rtx_PLUS (Pmode, tp, dest); > > + dest = gen_rtx_PLUS (ptr_mode, tp, dest); > > > > Please leave Pmode here. ptr_mode == Pmode at this point, but Pmode > > better documents the mode selection logic. > > > > Also, the tests fail for me with: > > > > /usr/include/gnu/stubs.h:13:11: fatal error: gnu/stubs-x32.h: No such > > file or directory > > > > so either use __builtin_printf or some other approach that doesn't > > need to #include stdio.h. > > > > A patch that implements above changes is attached to the message. > > > > Here is the updated patch. OK for master? OK if it passes regtests. Thanks, Uros.
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 2c087a4a3e0..8c437dbe1f3 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -10764,12 +10764,24 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) if (TARGET_GNU2_TLS) { if (TARGET_64BIT) - emit_insn (gen_tls_dynamic_gnu2_64 (dest, x)); + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, dest, x)); else emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic)); tp = get_thread_pointer (Pmode, true); - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest)); + + /* NB: Since thread pointer is in ptr_mode, make sure that + PLUS is done in ptr_mode. */ + if (Pmode != ptr_mode) + { + tp = lowpart_subreg (ptr_mode, tp, Pmode); + dest = lowpart_subreg (ptr_mode, dest, Pmode); + dest = gen_rtx_PLUS (ptr_mode, tp, dest); + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); + } + else + dest = gen_rtx_PLUS (Pmode, tp, dest); + dest = force_reg (Pmode, dest); if (GET_MODE (x) != Pmode) x = gen_rtx_ZERO_EXTEND (Pmode, x); @@ -10821,7 +10833,7 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) rtx tmp = ix86_tls_module_base (); if (TARGET_64BIT) - emit_insn (gen_tls_dynamic_gnu2_64 (base, tmp)); + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, base, tmp)); else emit_insn (gen_tls_dynamic_gnu2_32 (base, tmp, pic)); @@ -10864,7 +10876,18 @@ legitimize_tls_address (rtx x, enum tls_model model, bool for_mov) if (TARGET_GNU2_TLS) { - dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, dest, tp)); + /* NB: Since thread pointer is in ptr_mode, make sure that + PLUS is done in ptr_mode. */ + if (Pmode != ptr_mode) + { + tp = lowpart_subreg (ptr_mode, tp, Pmode); + dest = lowpart_subreg (ptr_mode, dest, Pmode); + dest = gen_rtx_PLUS (ptr_mode, tp, dest); + dest = gen_rtx_ZERO_EXTEND (Pmode, dest); + } + else + dest = gen_rtx_PLUS (Pmode, tp, dest); + dest = force_reg (Pmode, dest); if (GET_MODE (x) != Pmode) x = gen_rtx_ZERO_EXTEND (Pmode, x); diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index c9d2f338fe9..d53684096c4 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -15185,14 +15185,14 @@ (define_insn_and_split "*tls_dynamic_gnu2_combine_32" emit_insn (gen_tls_dynamic_gnu2_32 (operands[5], operands[1], operands[2])); }) -(define_expand "tls_dynamic_gnu2_64" +(define_expand "@tls_dynamic_gnu2_64_<mode>" [(set (match_dup 2) - (unspec:DI [(match_operand 1 "tls_symbolic_operand")] - UNSPEC_TLSDESC)) + (unspec:P [(match_operand 1 "tls_symbolic_operand")] + UNSPEC_TLSDESC)) (parallel - [(set (match_operand:DI 0 "register_operand") - (unspec:DI [(match_dup 1) (match_dup 2) (reg:DI SP_REG)] - UNSPEC_TLSDESC)) + [(set (match_operand:P 0 "register_operand") + (unspec:P [(match_dup 1) (match_dup 2) (reg:P SP_REG)] + UNSPEC_TLSDESC)) (clobber (reg:CC FLAGS_REG))])] "TARGET_64BIT && TARGET_GNU2_TLS" { @@ -15200,23 +15200,23 @@ (define_expand "tls_dynamic_gnu2_64" ix86_tls_descriptor_calls_expanded_in_cfun = true; }) -(define_insn "*tls_dynamic_gnu2_lea_64" - [(set (match_operand:DI 0 "register_operand" "=r") - (unspec:DI [(match_operand 1 "tls_symbolic_operand")] - UNSPEC_TLSDESC))] +(define_insn "*tls_dynamic_gnu2_lea_64_<mode>" + [(set (match_operand:P 0 "register_operand" "=r") + (unspec:P [(match_operand 1 "tls_symbolic_operand")] + UNSPEC_TLSDESC))] "TARGET_64BIT && TARGET_GNU2_TLS" - "lea{q}\t{%E1@TLSDESC(%%rip), %0|%0, %E1@TLSDESC[rip]}" + "lea\t{%E1@TLSDESC(%%rip), %0|%0, %E1@TLSDESC[rip]}" [(set_attr "type" "lea") - (set_attr "mode" "DI") + (set_attr "mode" "<MODE>") (set_attr "length" "7") (set_attr "length_address" "4")]) -(define_insn "*tls_dynamic_gnu2_call_64" - [(set (match_operand:DI 0 "register_operand" "=a") - (unspec:DI [(match_operand 1 "tls_symbolic_operand") - (match_operand:DI 2 "register_operand" "0") - (reg:DI SP_REG)] - UNSPEC_TLSDESC)) +(define_insn "*tls_dynamic_gnu2_call_64_<mode>" + [(set (match_operand:P 0 "register_operand" "=a") + (unspec:P [(match_operand 1 "tls_symbolic_operand") + (match_operand:P 2 "register_operand" "0") + (reg:P SP_REG)] + UNSPEC_TLSDESC)) (clobber (reg:CC FLAGS_REG))] "TARGET_64BIT && TARGET_GNU2_TLS" "call\t{*%a1@TLSCALL(%2)|[QWORD PTR [%2+%a1@TLSCALL]]}" @@ -15224,14 +15224,14 @@ (define_insn "*tls_dynamic_gnu2_call_64" (set_attr "length" "2") (set_attr "length_address" "0")]) -(define_insn_and_split "*tls_dynamic_gnu2_combine_64" - [(set (match_operand:DI 0 "register_operand" "=&a") - (plus:DI - (unspec:DI [(match_operand 2 "tls_modbase_operand") - (match_operand:DI 3) - (reg:DI SP_REG)] - UNSPEC_TLSDESC) - (const:DI (unspec:DI +(define_insn_and_split "*tls_dynamic_gnu2_combine_64_<mode>" + [(set (match_operand:P 0 "register_operand" "=&a") + (plus:P + (unspec:P [(match_operand 2 "tls_modbase_operand") + (match_operand:P 3) + (reg:P SP_REG)] + UNSPEC_TLSDESC) + (const:P (unspec:P [(match_operand 1 "tls_symbolic_operand")] UNSPEC_DTPOFF)))) (clobber (reg:CC FLAGS_REG))] @@ -15241,7 +15241,7 @@ (define_insn_and_split "*tls_dynamic_gnu2_combine_64" [(set (match_dup 0) (match_dup 4))] { operands[4] = can_create_pseudo_p () ? gen_reg_rtx (Pmode) : operands[0]; - emit_insn (gen_tls_dynamic_gnu2_64 (operands[4], operands[1])); + emit_insn (gen_tls_dynamic_gnu2_64 (Pmode, operands[4], operands[1])); }) (define_split diff --git a/gcc/testsuite/gcc.target/i386/pr93319-1a.c b/gcc/testsuite/gcc.target/i386/pr93319-1a.c new file mode 100644 index 00000000000..ba9d2b618c5 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr93319-1a.c @@ -0,0 +1,24 @@ +/* { dg-do compile { target { ! ia32 } } } */ +/* { dg-require-effective-target maybe_x32 } */ +/* { dg-require-effective-target fpic } */ +/* { dg-require-effective-target tls_native } */ +/* { dg-options "-mx32 -fPIC -mtls-dialect=gnu2" } */ + +#include <stdio.h> + +extern __thread int bar; +static __thread int foo = 30; + +int * +test1 (void) +{ + printf ("foo: %d\n", foo); + return &foo; +} + +int * +test2 (void) +{ + printf ("bar: %d\n", bar); + return &bar; +} diff --git a/gcc/testsuite/gcc.target/i386/pr93319-1b.c b/gcc/testsuite/gcc.target/i386/pr93319-1b.c new file mode 100644 index 00000000000..788796a9a94 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr93319-1b.c @@ -0,0 +1,7 @@ +/* { dg-do compile { target { ! ia32 } } } */ +/* { dg-require-effective-target maybe_x32 } */ +/* { dg-require-effective-target fpic } */ +/* { dg-require-effective-target tls_native } */ +/* { dg-options "-mx32 -O2 -fPIC -mtls-dialect=gnu2" } */ + +#include "pr93319-1a.c" diff --git a/gcc/testsuite/gcc.target/i386/pr93319-1c.c b/gcc/testsuite/gcc.target/i386/pr93319-1c.c new file mode 100644 index 00000000000..8c462e904bd --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr93319-1c.c @@ -0,0 +1,7 @@ +/* { dg-do compile { target { ! ia32 } } } */ +/* { dg-require-effective-target maybe_x32 } */ +/* { dg-require-effective-target fpic } */ +/* { dg-require-effective-target tls_native } */ +/* { dg-options "-mx32 -O2 -fPIC -mtls-dialect=gnu2 -maddress-mode=long" } */ + +#include "pr93319-1a.c" diff --git a/gcc/testsuite/gcc.target/i386/pr93319-1d.c b/gcc/testsuite/gcc.target/i386/pr93319-1d.c new file mode 100644 index 00000000000..7989e1c9fd4 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr93319-1d.c @@ -0,0 +1,7 @@ +/* { dg-do compile { target { ! ia32 } } } */ +/* { dg-require-effective-target maybe_x32 } */ +/* { dg-require-effective-target fpic } */ +/* { dg-require-effective-target tls_native } */ +/* { dg-options "-mx32 -fPIC -mtls-dialect=gnu2 -maddress-mode=long" } */ + +#include "pr93319-1a.c"
To add x32 support to -mtls-dialect=gnu2, we need to replace DI with P in GNU2 TLS patterns. Since thread pointer is in ptr_mode, PLUS in GNU2 TLS address computation must be done in ptr_mode to support -maddress-mode=long. Also drop the "q" suffix from lea to support both "lea foo@TLSDESC(%rip), %eax" and "foo@TLSDESC(%rip), %rax". Tested on Linux/x86-64. OK for master? Thanks. H.J. --- gcc/ PR target/93319 * config/i386/i386.c (legitimize_tls_address): Pass Pmode to gen_tls_dynamic_gnu2_64. Compute GNU2 TLS address in ptr_mode. * config/i386/i386.md (tls_dynamic_gnu2_64): Renamed to ... (@tls_dynamic_gnu2_64_<mode>): This. Replace DI with P. (*tls_dynamic_gnu2_lea_64): Renamed to ... (*tls_dynamic_gnu2_lea_64_<mode>): This. Replace DI with P. Remove the {q} suffix from lea. (*tls_dynamic_gnu2_call_64): Renamed to ... (*tls_dynamic_gnu2_call_64_<mode>): This. Replace DI with P. (*tls_dynamic_gnu2_combine_64): Renamed to ... (*tls_dynamic_gnu2_combine_64_<mode>): This. Replace DI with P. Pass Pmode to gen_tls_dynamic_gnu2_64. gcc/testsuite/ PR target/93319 * gcc.target/i386/pr93319-1a.c: New test. * gcc.target/i386/pr93319-1b.c: Likewise. * gcc.target/i386/pr93319-1c.c: Likewise. * gcc.target/i386/pr93319-1d.c: Likewise. --- gcc/config/i386/i386.c | 31 +++++++++++-- gcc/config/i386/i386.md | 54 +++++++++++----------- gcc/testsuite/gcc.target/i386/pr93319-1a.c | 24 ++++++++++ gcc/testsuite/gcc.target/i386/pr93319-1b.c | 7 +++ gcc/testsuite/gcc.target/i386/pr93319-1c.c | 7 +++ gcc/testsuite/gcc.target/i386/pr93319-1d.c | 7 +++ 6 files changed, 99 insertions(+), 31 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1a.c create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1b.c create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1c.c create mode 100644 gcc/testsuite/gcc.target/i386/pr93319-1d.c