Bug 90193 - [8 Regression] asm goto with TLS "m" input operand generates incorrect assembler in O1 and O2
Summary: [8 Regression] asm goto with TLS "m" input operand generates incorrect assemb...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 8.3.0
: P3 normal
Target Milestone: 8.4
Assignee: Jakub Jelinek
URL:
Keywords: inline-asm, wrong-code
Depends on:
Blocks:
 
Reported: 2019-04-20 02:26 UTC by Mathieu Desnoyers
Modified: 2022-03-08 00:06 UTC (History)
4 users (show)

See Also:
Host:
Target: x86_64-*-*, i?86-*-*
Build:
Known to work: 7.3.0, 7.4.0, 8.3.1, 9.0
Known to fail: 8.1.0, 8.2.0, 8.3.0
Last reconfirmed: 2019-04-20 00:00:00


Attachments
gcc9-pr90193.patch (840 bytes, patch)
2019-04-22 14:31 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mathieu Desnoyers 2019-04-20 02:26:57 UTC
This issue can be reproduced with:

gcc version 8.3.0 (GCC)
Target: x86_64-pc-linux-gnu
Configured with: ./configure --prefix=/home/efficios/local

Command line reproducing the bug:

/home/efficios/local/bin/gcc -O1 -o test-asm-goto test-asm-goto.c
(same with -O2)

Compiler output:

/tmp/ccsWO2Fm.o: In function `main':
test-asm-goto.c:(.text+0x1): undefined reference to `.L2'
collect2: error: ld returned 1 exit status

Preprocessed file reproducing the bug:

# 1 "test-asm-goto.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 31 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 32 "<command-line>" 2
# 1 "test-asm-goto.c"
__thread int var;

static int fct(void)
{
 asm goto ( "jmp %l[testlabel]\n\t"
   : : [var] "m" (var) : : testlabel);
 return 0;
testlabel:
 return 1;
}

int main()
{
 return fct();
}

It works fine with gcc 7.3.0 with and without optimizations, and it works fine with gcc 8.3.0 without optimizations. It also affects compilation with -m32 (32-bit x86).
Comment 1 Alexander Monakov 2019-04-20 06:35:15 UTC
split1 transforms JUMP_INSN with the asm into a plain INSN, after which the cfg becomes corrupted in various ways.
Comment 2 Mathieu Desnoyers 2019-04-20 13:19:03 UTC
By the way, it can also be reproduced by replacing the "jmp" instruction within the inline asm by a ".long":

# 1 "test-asm-goto-data.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 31 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 32 "<command-line>" 2
# 1 "test-asm-goto-data.c"
__thread int var;

static int fct(void)
{
 asm goto ( ".long %l[testlabel]\n\t"
   : : [var] "m" (var) : : testlabel);
 return 0;
testlabel:
 return 1;
}

int main()
{
 return fct();
}
Comment 3 Andrew Pinski 2019-04-20 18:44:50 UTC
This is a target issue ...

I think is the insn and split which causes the problem:
(define_insn_and_split "*add_tp_<mode>"
  [(set (match_operand:PTR 0 "register_operand" "=r")
        (plus:PTR
          (unspec:PTR [(const_int 0)] UNSPEC_TP)
          (match_operand:PTR 1 "register_operand" "0")))
   (clobber (reg:CC FLAGS_REG))]
  ""
  "#"
  ""
  [(parallel
     [(set (match_dup 0)
           (plus:PTR (match_dup 1) (match_dup 2)))
      (clobber (reg:CC FLAGS_REG))])]
{
  addr_space_t as = DEFAULT_TLS_SEG_REG;

  operands[2] = gen_const_mem (<MODE>mode, const0_rtx);
  set_mem_addr_space (operands[2], as);
})

Which was introduced/changed in r251075.
Comment 4 Segher Boessenkool 2019-04-20 20:56:40 UTC
It actually ICEs if you have checking enabled.
Comment 5 Andrew Pinski 2019-04-20 20:59:29 UTC
(In reply to Segher Boessenkool from comment #4)
> It actually ICEs if you have checking enabled.

Didn't for me with:
GNU C17 (GCC) version 9.0.1 20190401 (experimental) [master revision 449a19898aa:0239598e3c8:8fe074cf790f632b22e59c24f102e528407bb04e] (x86_64-pc-linux-gnu)
        compiled by GNU C version 9.0.1 20190401 (experimental) [master revision 449a19898aa:0239598e3c8:8fe074cf790f632b22e59c24f102e528407bb04e], GMP version 6.1.2, MPFR version 4.0.1, MPC version 1.1.0, isl version none
Comment 6 Segher Boessenkool 2019-04-20 21:04:48 UTC
It emits an insn instead if a jump_insn in the asm, during split1, in

(define_split
  [(match_operand 0 "tls_address_pattern")]
  "TARGET_TLS_DIRECT_SEG_REFS"
  [(match_dup 0)]
  "operands[0] = ix86_rewrite_tls_address (operands[0]);")
Comment 7 Segher Boessenkool 2019-04-20 21:35:14 UTC
The same splitter is what causes the bb of the asm to be marked as
always falling through, which is why that non-fallthrough label is
eventually deleted.
Comment 8 Segher Boessenkool 2019-04-20 21:39:23 UTC
(As Alexander said in comment 1...  I need to learn how to read some day).
Comment 9 Jakub Jelinek 2019-04-22 13:58:34 UTC
Looks like missing asm goto support in classify_insn to me.
Comment 10 Jakub Jelinek 2019-04-22 14:31:03 UTC
Created attachment 46222 [details]
gcc9-pr90193.patch

Untested fix.
Comment 11 Mathieu Desnoyers 2019-04-22 14:59:01 UTC
The proposed fix "gcc9-pr90193.patch" applied on top of gcc-8.3.0 fixes the issue for both x86-64 and for x86-32 (-m32) from a 64-bit x86 gcc.
Comment 12 Jakub Jelinek 2019-04-24 15:50:07 UTC
Author: jakub
Date: Wed Apr 24 15:49:36 2019
New Revision: 270550

URL: https://gcc.gnu.org/viewcvs?rev=270550&root=gcc&view=rev
Log:
	PR target/90193
	* rtl.c (classify_insn): Return JUMP_INSN for asm goto.
	* emit-rtl.c (try_split): Copy over REG_LABEL_TARGET.

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

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr90193.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/emit-rtl.c
    trunk/gcc/rtl.c
    trunk/gcc/testsuite/ChangeLog
Comment 13 Jakub Jelinek 2019-04-24 15:51:40 UTC
Fixed on the trunk so far.
Comment 14 Jakub Jelinek 2019-04-30 21:11:37 UTC
Author: jakub
Date: Tue Apr 30 21:11:06 2019
New Revision: 270757

URL: https://gcc.gnu.org/viewcvs?rev=270757&root=gcc&view=rev
Log:
	Backported from mainline
	2019-04-24  Jakub Jelinek  <jakub@redhat.com>

	PR target/90193
	* rtl.c (classify_insn): Return JUMP_INSN for asm goto.
	* emit-rtl.c (try_split): Copy over REG_LABEL_TARGET.

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

Added:
    branches/gcc-8-branch/gcc/testsuite/gcc.target/i386/pr90193.c
Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/emit-rtl.c
    branches/gcc-8-branch/gcc/rtl.c
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
Comment 15 Jakub Jelinek 2019-05-01 07:19:33 UTC
Fixed for 8.4+ too.