Bug 89290 - [8 Regression] ICE in change_address_1, at emit-rtl.c:2286
Summary: [8 Regression] ICE in change_address_1, at emit-rtl.c:2286
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 9.0
: P3 normal
Target Milestone: 8.3
Assignee: Jakub Jelinek
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2019-02-11 17:35 UTC by G. Steinmetz
Modified: 2019-02-14 07:42 UTC (History)
2 users (show)

See Also:
Host:
Target: x86_64-pc-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-02-11 00:00:00


Attachments
Untested patch (603 bytes, patch)
2019-02-12 10:05 UTC, Uroš Bizjak
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description G. Steinmetz 2019-02-11 17:35:18 UTC
With options -O0 -mcmodel=large on x86_64-pc-linux-gnu :


$ cat pr56847.c
struct S { long int a, b; } e;
__thread struct S s;

void
foo (void)
{
  s = e;
}


$ gcc-7          -c pr56847.c -O0 -mcmodel=large
$ gcc-9-20190210 -c pr56847.c -O2 -mcmodel=large
$ gcc-9-20190210 -c pr56847.c -O0
$
$ gcc-9-20190210 -c pr56847.c -O0 -mcmodel=large
during RTL pass: split2
pr56847.c: In function 'foo':
pr56847.c:8:1: internal compiler error: in change_address_1, at emit-rtl.c:2286
    8 | }
      | ^
0x7a18d0 change_address_1
        ../../gcc/emit-rtl.c:2286
0x7a4896 adjust_address_1(rtx_def*, machine_mode, poly_int<1u, long>, int, int, int, poly_int<1u, long>)
        ../../gcc/emit-rtl.c:2420
0xd5c619 split_double_mode(machine_mode, rtx_def**, int, rtx_def**, rtx_def**)
        ../../gcc/config/i386/i386.c:18661
0xd5ca5b ix86_split_to_parts
        ../../gcc/config/i386/i386.c:25393
0xd671cd ix86_split_long_move(rtx_def**)
        ../../gcc/config/i386/i386.c:25483
0xf88d20 gen_split_8(rtx_insn*, rtx_def**)
        ../../gcc/config/i386/i386.md:2267
0x10ccd04 split_10
        ../../gcc/config/i386/i386.md:9883
0x112b377 split_insns(rtx_def*, rtx_insn*)
        ../../gcc/config/i386/i386.md:13184
0x7a6331 try_split(rtx_def*, rtx_insn*, int)
        ../../gcc/emit-rtl.c:3851
0xa00281 split_insn
        ../../gcc/recog.c:2901
0xa043a2 split_all_insns()
        ../../gcc/recog.c:3005
0xa044a8 execute
        ../../gcc/recog.c:3905
Comment 1 Jakub Jelinek 2019-02-11 19:39:35 UTC
The problem is that adjust_address with
(mem/c:TI (const:DI (unspec:DI [
                (symbol_ref:DI ("s") [flags 0x2a] <var_decl 0x7ffff7ffbb40 s>)
            ] UNSPEC_NTPOFF)) [1 s+0 S16 A64 AS1])
as first argument, E_DImode and 8 as last two doesn't validate.
We have originally:
(insn 7 6 10 2 (set (mem/c:TI (plus:DI (unspec:DI [
                        (const_int 0 [0])
                    ] UNSPEC_TP)
                (const:DI (unspec:DI [
                            (symbol_ref:DI ("s") [flags 0x2a]  <var_decl 0x7ff97a130b40 s>)
                        ] UNSPEC_NTPOFF))) [1 s+0 S16 A64])
        (reg:TI 83)) "pr89290.c":7:5 65 {*movti_internal}
     (nil))
then the ix86_rewrite_tls_address splitter rewrites this into:
(insn 12 6 10 2 (set (mem/c:TI (const:DI (unspec:DI [
                        (symbol_ref:DI ("s") [flags 0x2a]  <var_decl 0x7ff97a130b40 s>)
                    ] UNSPEC_NTPOFF)) [1 s+0 S16 A64 AS1])
        (reg:TI 83)) "pr89290.c":7:5 -1
     (nil))
and finally we ICE in the split_double_move splitter.
Comment 2 Uroš Bizjak 2019-02-12 09:29:43 UTC
(In reply to Jakub Jelinek from comment #1)
> then the ix86_rewrite_tls_address splitter rewrites this into:
> (insn 12 6 10 2 (set (mem/c:TI (const:DI (unspec:DI [
>                         (symbol_ref:DI ("s") [flags 0x2a]  <var_decl
> 0x7ff97a130b40 s>)
>                     ] UNSPEC_NTPOFF)) [1 s+0 S16 A64 AS1])
>         (reg:TI 83)) "pr89290.c":7:5 -1
>      (nil))
> and finally we ICE in the split_double_move splitter.

The address is constant and offsettable_address_addr_space_p claims that the address

(const:DI (unspec:DI [
            (symbol_ref:DI ("s") [flags 0x2a] <var_decl 0x7f4cf23f1b40 s>)
        ] UNSPEC_NTPOFF))


is offsettable due to:

  if (CONSTANT_ADDRESS_P (y))
    return 1;

So, we should probably allow addresses in the form of:

        movq    %rax, %fs:s+8@tpoff
Comment 3 Uroš Bizjak 2019-02-12 09:46:16 UTC
This address should be valid:

(const:DI (plus:DI (unspec:DI [
                (symbol_ref:DI ("s") [flags 0x2a] <var_decl 0x7f1b48c08b40 s>)
            ] UNSPEC_NTPOFF)
        (const_int 8 [0x8])))

and there is code that allows this form in ix86_legitimate_address_p:

	      /* foo@dtpoff(%rX) is ok.  */
	      if (GET_CODE (disp) != CONST
		  || GET_CODE (XEXP (disp, 0)) != PLUS
		  || GET_CODE (XEXP (XEXP (disp, 0), 0)) != UNSPEC
		  || !CONST_INT_P (XEXP (XEXP (disp, 0), 1))
		  || (XINT (XEXP (XEXP (disp, 0), 0), 1) != UNSPEC_DTPOFF
		      && XINT (XEXP (XEXP (disp, 0), 0), 1) != UNSPEC_NTPOFF))
		/* Non-constant pic memory reference.  */
		return false;

Jakub, can you maybe look into this issue?
Comment 4 Jakub Jelinek 2019-02-12 09:49:29 UTC
(In reply to Uroš Bizjak from comment #3)
> This address should be valid:
> 
> (const:DI (plus:DI (unspec:DI [
>                 (symbol_ref:DI ("s") [flags 0x2a] <var_decl 0x7f1b48c08b40
> s>)
>             ] UNSPEC_NTPOFF)
>         (const_int 8 [0x8])))
> 
> and there is code that allows this form in ix86_legitimate_address_p:
> 
> 	      /* foo@dtpoff(%rX) is ok.  */
> 	      if (GET_CODE (disp) != CONST
> 		  || GET_CODE (XEXP (disp, 0)) != PLUS
> 		  || GET_CODE (XEXP (XEXP (disp, 0), 0)) != UNSPEC
> 		  || !CONST_INT_P (XEXP (XEXP (disp, 0), 1))
> 		  || (XINT (XEXP (XEXP (disp, 0), 0), 1) != UNSPEC_DTPOFF
> 		      && XINT (XEXP (XEXP (disp, 0), 0), 1) != UNSPEC_NTPOFF))
> 		/* Non-constant pic memory reference.  */
> 		return false;
> 
> Jakub, can you maybe look into this issue?

I will, probably by looking at why it works fine without -mcmodel=large, because in that case it generates those CONSTs with PLUS, UNSPEC_NTPOFF and CONST_INT offset.
Comment 5 Uroš Bizjak 2019-02-12 10:05:16 UTC
Created attachment 45664 [details]
Untested patch

Kind of a proposed patch that fixes the testcase. More or less shot in the dark, as this is not really my area of experience.
Comment 6 Jakub Jelinek 2019-02-12 16:21:01 UTC
I think the right fix is:
--- gcc/config/i386/predicates.md.jj	2019-01-01 12:37:32.267727037 +0100
+++ gcc/config/i386/predicates.md	2019-02-12 17:07:15.937097266 +0100
@@ -182,7 +182,7 @@
 	  rtx op1 = XEXP (XEXP (op, 0), 0);
 	  rtx op2 = XEXP (XEXP (op, 0), 1);
 
-	  if (ix86_cmodel == CM_LARGE)
+	  if (ix86_cmodel == CM_LARGE && GET_CODE (op1) != UNSPEC)
 	    return false;
 	  if (!CONST_INT_P (op2))
 	    return false;
Even with -mcmodel=large, it is ok to offset the TLS LE by signed 32-bit constants, we don't really support > 2GB thread local segments, we don't have relocations for that etc. and nobody with sane mind should expect that to work.
Comment 7 Jakub Jelinek 2019-02-13 08:46:08 UTC
Author: jakub
Date: Wed Feb 13 08:45:37 2019
New Revision: 268837

URL: https://gcc.gnu.org/viewcvs?rev=268837&root=gcc&view=rev
Log:
	PR target/89290
	* config/i386/predicates.md (x86_64_immediate_operand): Allow
	TLS UNSPECs offsetted by signed 32-bit CONST_INT even with
	-mcmodel=large.

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

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr89290.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/predicates.md
    trunk/gcc/testsuite/ChangeLog
Comment 8 Jakub Jelinek 2019-02-13 14:40:24 UTC
Fixed on the trunk so far.
Comment 9 Jakub Jelinek 2019-02-14 07:40:18 UTC
Author: jakub
Date: Thu Feb 14 07:39:46 2019
New Revision: 268864

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

	PR target/89290
	* config/i386/predicates.md (x86_64_immediate_operand): Allow
	TLS UNSPECs offsetted by signed 32-bit CONST_INT even with
	-mcmodel=large.

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

Added:
    branches/gcc-8-branch/gcc/testsuite/gcc.target/i386/pr89290.c
Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/config/i386/predicates.md
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
Comment 10 Jakub Jelinek 2019-02-14 07:42:30 UTC
Fixed.