[PATCH] PR target/93319: x32: Add x32 support to -mtls-dialect=gnu2
Uros Bizjak
ubizjak@gmail.com
Mon Jan 20 08:30:00 GMT 2020
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.
More information about the Gcc-patches
mailing list