[Bug c/103275] New: don't generate kmov with IE model relocations

[Bug c/103275] New: don't generate kmov with IE model relocations

[jason at zx2c4 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

            Bug ID: 103275
           Summary: don't generate kmov with IE model relocations
           Product: gcc
           Version: 11.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jason at zx2c4 dot com
  Target Milestone: ---

This is a spin-off from https://sourceware.org/bugzilla/show_bug.cgi?id=28595

1.c:
extern __thread int mytls __attribute__((tls_model ("initial-exec")));

__attribute__((noipa)) void
foo (void)
{
  asm ("");
}

int
main ()
{
  foo ();
#ifdef KMOVD
  asm volatile ("kmovd mytls@gotntpoff(%%ebx), %%k0" : : : "k0");
#else
  volatile int a = mytls;
#endif
  return 0;
}
2.c:
__thread int mytls __attribute__((tls_model ("initial-exec")));
$ gcc -O2 -m32 -fPIC /tmp/1.c /tmp/2.c -o /tmp/1 -UKMOVD -mavx512f
$ gcc -O2 -m32 -fPIC /tmp/1.c /tmp/2.c -o /tmp/1 -DKMOVD -mavx512f
/usr/bin/ld: /tmp/ccQrvWxU.o: TLS transition from R_386_TLS_GOTIE to
R_386_TLS_LE_32 against `mytls' at 0x1c in section `.text.startup' failed
/usr/bin/ld: final link failed: bad value
collect2: error: ld returned 1 exit status

That results from the type check returning false here:

    case R_386_TLS_GOTIE:
    case R_386_TLS_IE_32:
      /* Check transition from {IE_32,GOTIE} access model:
                subl foo@{tpoff,gontoff}(%reg1), %reg2
                movl foo@{tpoff,gontoff}(%reg1), %reg2
                addl foo@{tpoff,gontoff}(%reg1), %reg2
       */

      if (offset < 2 || (offset + 4) > sec->size)
        return false;

      val = bfd_get_8 (abfd, contents + offset - 1);
      if ((val & 0xc0) != 0x80 || (val & 7) == 4)
        return false;

      type = bfd_get_8 (abfd, contents + offset - 2);
      return type == 0x8b || type == 0x2b || type == 0x03;


In the ensuing discussion, Florian thought that GAS shouldn't even be
assembling it, because that's not a valid relocation type. As a consequence,
GCC shouldn't be emitting it either, which it is in the context of glibc.
Quoting Jakub:


> https://akkadia.org/drepper/tls.pdf
> I don't remember anymore whether it is ok if some of the TLS
> relocations appear also in instructions not exactly mentioned
> in the above mentioned pdf and if instead of giving the linker
> errors like the above the linker should just not optimize a
> particular TLS model, i.e. still use IE model here when it
> can't optimize it into LE model (then it would be a linker bug),
> or whether the compiler needs to ensure that those @gotntpoff
> etc. appear solely in the instructions mentioned in the pdf
> (here in movl).  In that case it would be a gcc bug, e.g. for
> the @gotntpoff MEMs it would need to disallow them by default
> and only accept them as a special case in a simple *movsi_internal.

[Bug target/103275] don't generate kmov with IE model relocations

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |link-failure

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Well your testcase is just invalid as it is inline asm.

[Bug target/103275] don't generate kmov with IE model relocations

[jason at zx2c4 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #2 from Jason A. Donenfeld <jason at zx2c4 dot com> ---
Yes, but as I wrote, this sort of thing is emitted by gcc when compiling C
code, as described in https://sourceware.org/bugzilla/show_bug.cgi?id=28595

[Bug target/103275] don't generate kmov with IE model relocations

[jason at zx2c4 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #3 from Jason A. Donenfeld <jason at zx2c4 dot com> ---
Here: https://gcc.godbolt.org/z/zqbWK8rE8

On line 76 of the output:

        kmovd   k0, DWORD PTR __libc_tsd_CTYPE_B@gotntpoff[ebx]

[Bug target/103275] [11 Regression] don't generate kmov with IE model relocations

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.3
   Last reconfirmed|                            |2021-11-16
            Summary|don't generate kmov with IE |[11 Regression] don't
                   |model relocations           |generate kmov with IE model
                   |                            |relocations
                 CC|                            |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note, the bug is on the above godbolt testcase only latent on the trunk since
r12-4790-g4b3a325f07acebf47e82de227ce1d5ba62f5bcae
and started with r11-2795-g16516644d8f5c13e666251470d604778d347f796

[Bug target/103275] [11 Regression] don't generate kmov with IE model relocations

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> and started with r11-2795-g16516644d8f5c13e666251470d604778d347f796

I suspect it was latent before that change too ...

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11 Regression] don't       |[11/12 Regression] don't
                   |generate kmov with IE model |generate kmov with IE model
                   |relocations                 |relocations
           Priority|P3                          |P2

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[jason at zx2c4 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #6 from Jason A. Donenfeld <jason at zx2c4 dot com> ---
Working around this now downstream via
https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=876602ee223c6c4225371b428a346f0b2d7f2020
which we'll revert whenever an upstream fix is available.

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #7 from Hongtao.liu <crazylht at gmail dot com> ---
vmovd have same issue, for simplify should we disable 32bit load for sse/mask
register when memory_operand has PIC address.

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #8 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Hongtao.liu from comment #7)
> vmovd have same issue, for simplify should we disable 32bit load for
> sse/mask register when memory_operand has PIC address.

Please disable specific UNSPEC operands with mask register disallowed
in this assembler change:

https://sourceware.org/git/?p=binutils-gdb.git;a=patch;h=d7e3e627027fcf37d63e284144fe27ff4eba36b5

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Just those particular TLS UNSPECs.  And, e.g. UNSPEC_NTPOFF should be fine in
any  insn that takes memory operands...

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #10 from Hongtao.liu <crazylht at gmail dot com> ---
I'm working on a patch which adds a new memory constraint "Bk" which will
exclude TLS UNSPECs for mask register alternative.

The UNSPEC i'm excluding is like below, any other UNSPEC needs to be added?

bool
ix86_notls_memory (rtx mem)
{
  gcc_assert (MEM_P (mem));

  rtx addr = XEXP (mem, 0);
  subrtx_var_iterator::array_type array;
  FOR_EACH_SUBRTX_VAR (iter, array, addr, ALL)
    {
      rtx op = *iter;
      if (GET_CODE (op) == UNSPEC)
        switch (XINT (op, 1))
          {
          case UNSPEC_GOTTPOFF:
          case UNSPEC_GOTNTPOFF:
          case UNSPEC_TP:
          case UNSPEC_TLS_GD:
          case UNSPEC_TLS_LD_BASE:
          case UNSPEC_TLSDESC:
          case UNSPEC_TLS_IE_SUN:
            return false;
          default:
            break;
          }
      /* Should iter.skip_subrtxes ();
         if there's no inner UNSPEC in addr???.  */
    }

  return true;
}

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #11 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Hongtao.liu from comment #10)
> I'm working on a patch which adds a new memory constraint "Bk" which will
> exclude TLS UNSPECs for mask register alternative.
> 
> The UNSPEC i'm excluding is like below, any other UNSPEC needs to be added?
> 
> bool
> ix86_notls_memory (rtx mem)
> {
>   gcc_assert (MEM_P (mem));
> 
>   rtx addr = XEXP (mem, 0);
>   subrtx_var_iterator::array_type array;
>   FOR_EACH_SUBRTX_VAR (iter, array, addr, ALL)
>     {
>       rtx op = *iter;
>       if (GET_CODE (op) == UNSPEC)
> 	switch (XINT (op, 1))
> 	  {
> 	  case UNSPEC_GOTTPOFF:
> 	  case UNSPEC_GOTNTPOFF:
> 	  case UNSPEC_TP:
> 	  case UNSPEC_TLS_GD:
> 	  case UNSPEC_TLS_LD_BASE:
> 	  case UNSPEC_TLSDESC:
> 	  case UNSPEC_TLS_IE_SUN:

This doesn't look right.  For TARGET_64BIT, only

        kmovq   foo@gottpoff(%rip), %k0
        kmovq   foo@tlsld(%rip), %k0

should be disallowed.  For !TARGET_64BIT, only

        kmovd   foo@gotntpoff(%eax), %k0
        kmovd   foo@tpoff(%eax), %k0

should be disallowed.

> 	    return false;
> 	  default:
> 	    break;
> 	  }
>       /* Should iter.skip_subrtxes ();
> 	 if there's no inner UNSPEC in addr???.  */
>     }
> 
>   return true;
> }

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[jason at zx2c4 dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #12 from Jason A. Donenfeld <jason at zx2c4 dot com> ---
(It might be too late at this point to say it, but I thought it's worth
mentioning that another approach might be convincing the binutils people to
accept kmov/vmov/vex relocations.)

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
If you read the tls.pdf, you'll see that it is not really possible, the IE ->
LE transition needs to change the instructions that read from the .got memory
to instructions that set the destination to an immediate.  I think it can deal
currently with:
                subl foo@{tpoff,gotntpoff}(%reg1), %reg2
                movl foo@{tpoff,gotntpoff}(%reg1), %reg2
                addl foo@{tpoff,gotntpoff}(%reg1), %reg2
and is tranformed into movl immediate, %reg2.  There is no instruction that
would set a mask register to an immediate (except for some special cases like
0).

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #14 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to H.J. Lu from comment #11)
> (In reply to Hongtao.liu from comment #10)
> > I'm working on a patch which adds a new memory constraint "Bk" which will
> > exclude TLS UNSPECs for mask register alternative.
> > 
> > The UNSPEC i'm excluding is like below, any other UNSPEC needs to be added?
> > 
> > bool
> > ix86_notls_memory (rtx mem)
> > {
> >   gcc_assert (MEM_P (mem));
> > 
> >   rtx addr = XEXP (mem, 0);
> >   subrtx_var_iterator::array_type array;
> >   FOR_EACH_SUBRTX_VAR (iter, array, addr, ALL)
> >     {
> >       rtx op = *iter;
> >       if (GET_CODE (op) == UNSPEC)
> > 	switch (XINT (op, 1))
> > 	  {
> > 	  case UNSPEC_GOTTPOFF:
> > 	  case UNSPEC_GOTNTPOFF:
> > 	  case UNSPEC_TP:
> > 	  case UNSPEC_TLS_GD:
> > 	  case UNSPEC_TLS_LD_BASE:
> > 	  case UNSPEC_TLSDESC:
> > 	  case UNSPEC_TLS_IE_SUN:
> 
> This doesn't look right.  For TARGET_64BIT, only
> 
> 	kmovq	foo@gottpoff(%rip), %k0
> 	kmovq	foo@tlsld(%rip), %k0
It looks like gcc won't generate kmovq  foo@tlsld(%rip), %k0, but only leaq
 ("lea{q}\t{%&@tlsld(%%rip), %%rdi|rdi, %&@tlsld[rip]}", operands);

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:b5844cb0bc8c7d9be2ff1ecded249cad82b9b71c

commit r12-5445-gb5844cb0bc8c7d9be2ff1ecded249cad82b9b71c
Author: liuhongt <hongtao.liu@intel.com>
Date:   Wed Nov 17 15:48:37 2021 +0800

    Don't allow mask/sse/mmx mov in TLS code sequences.

    As change in assembler, refer to [1], this patch disallow mask/sse/mmx
    mov in TLS code sequences which require integer MOV instructions.

    [1]
https://sourceware.org/git/?p=binutils-gdb.git;a=patch;h=d7e3e627027fcf37d63e284144fe27ff4eba36b5

    gcc/ChangeLog:

            PR target/103275
            * config/i386/constraints.md (Bk): New
            define_memory_constraint.
            * config/i386/i386-protos.h (ix86_gpr_tls_address_pattern_p):
            Declare.
            * config/i386/i386.c (ix86_gpr_tls_address_pattern_p): New
            function.
            * config/i386/i386.md (*movsi_internal): Don't allow
            mask/sse/mmx move in TLS code sequences.
            (*movdi_internal): Ditto.

    gcc/testsuite/ChangeLog:

            * gcc.target/i386/pr103275.c: New test.

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by hongtao Liu
<liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:eb8ff3cbc09e029ca0cbd0d8b09bcaba162ab95a

commit r11-9257-geb8ff3cbc09e029ca0cbd0d8b09bcaba162ab95a
Author: liuhongt <hongtao.liu@intel.com>
Date:   Wed Nov 17 15:48:37 2021 +0800

    Don't allow mask/sse/mmx mov in TLS code sequences.

    As change in assembler, refer to [1], this patch disallow mask/sse/mmx
    mov in TLS code sequences which require integer MOV instructions.

    [1]
https://sourceware.org/git/?p=binutils-gdb.git;a=patch;h=d7e3e627027fcf37d63e284144fe27ff4eba36b5

    gcc/ChangeLog:

            PR target/103275
            * config/i386/constraints.md (Bk): New
            define_memory_constraint.
            * config/i386/i386-protos.h (ix86_gpr_tls_address_pattern_p):
            Declare.
            * config/i386/i386.c (ix86_gpr_tls_address_pattern_p): New
            function.
            * config/i386/i386.md (*movsi_internal): Don't allow
            mask/sse/mmx move in TLS code sequences.
            (*movdi_internal): Ditto.

    gcc/testsuite/ChangeLog:

            * gcc.target/i386/pr103275.c: New test.

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

--- Comment #17 from Hongtao.liu <crazylht at gmail dot com> ---
Fixed in GCC12 and GCC11.

[Bug target/103275] [11/12 Regression] don't generate kmov with IE model relocations

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103275

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Closing as fixed then.