On Sun, Jan 19, 2020 at 9:48 AM Uros Bizjak <ubiz...@gmail.com> wrote:
>
> On Sun, Jan 19, 2020 at 6:43 PM Uros Bizjak <ubiz...@gmail.com> wrote:
> >
> > On Sun, Jan 19, 2020 at 2:58 PM H.J. Lu <hjl.to...@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 <f...@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.
--
H.J.
