diff mbox

Fix 64-bit Solaris 2/x86 IE TLS code sequence (PR target/43309)

Message ID AANLkTi=KSntp2aC7hWAwbTV9ej640kRqc3iVU-BXcwfW@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak Dec. 20, 2010, 9:25 p.m. UTC
On Mon, Dec 20, 2010 at 10:03 PM, Richard Henderson <rth@redhat.com> wrote:
> On 12/20/2010 11:26 AM, Uros Bizjak wrote:
>> +       /* The SUN linker took the AMD64 TLS spec literally
>> +          and can only handle %rax as destination of the
>> +          initial executable code sequence.  */
>> +
>> +       rtx rax = gen_rtx_REG (Pmode, AX_REG);
>
> It's much better not to use an explicit hard register
> and instead rely on the 'a' constraint in the pattern.

IIRC, there were certain problems with reload trying to allocate %rax
to the pattern.

Anyway, attached patch also generates correct code for my small tests.
Rainer, can you please test attached patch?

Uros.

Comments

Rainer Orth Dec. 21, 2010, 4:30 p.m. UTC | #1
Uros Bizjak <ubizjak@gmail.com> writes:

> Anyway, attached patch also generates correct code for my small tests.
> Rainer, can you please test attached patch?

Unfortunately, this doesn't work.  E.g. for the g++.dg/tls/static-1.C
testcase, I get this difference (among others) for static-1a.s:

--- 11-gcc/gcc/testsuite/g++/static-1a.s        2010-12-21 17:18:41.725085343 +0
100
+++ 11-gcc.new/gcc/testsuite/g++/static-1a.s    2010-12-21 17:17:58.242204790 +0
100
@@ -6,14 +6,21 @@
 _Z4testv:
 .LFB0:
        movq    %fs:0, %rax
+       movq    %rax, %rcx
        addq    _ZN1A1iE@gottpoff(%rip), %rax

I.e. there's an intervening insn between the movq %fs:0, %rax and the
addq, which current Sun ld cannot handle.

All the testcases that were fixed by my patch (augmented to remove the
expander) are now failing again.

	Rainer
Richard Henderson Dec. 21, 2010, 4:38 p.m. UTC | #2
On 12/21/2010 08:30 AM, Rainer Orth wrote:
> Uros Bizjak <ubizjak@gmail.com> writes:
> 
>> Anyway, attached patch also generates correct code for my small tests.
>> Rainer, can you please test attached patch?
> 
> Unfortunately, this doesn't work.  E.g. for the g++.dg/tls/static-1.C
> testcase, I get this difference (among others) for static-1a.s:
> 
> --- 11-gcc/gcc/testsuite/g++/static-1a.s        2010-12-21 17:18:41.725085343 +0
> 100
> +++ 11-gcc.new/gcc/testsuite/g++/static-1a.s    2010-12-21 17:17:58.242204790 +0
> 100
> @@ -6,14 +6,21 @@
>  _Z4testv:
>  .LFB0:
>         movq    %fs:0, %rax
> +       movq    %rax, %rcx
>         addq    _ZN1A1iE@gottpoff(%rip), %rax
> 
> I.e. there's an intervening insn between the movq %fs:0, %rax and the
> addq, which current Sun ld cannot handle.

I thought that might happen, Uros.  You can't split the patterns
for Sun ld.


r~
Rainer Orth Dec. 22, 2010, 2:34 p.m. UTC | #3
Richard Henderson <rth@redhat.com> writes:

> I thought that might happen, Uros.  You can't split the patterns
> for Sun ld.

How should we proceed from here?  Return to my version of the patch?
While performing a x86_64-unknown-linux-gnu bootstrap for good measure,
I noticed that there's a problem there: bootstrap fails like this:

/var/gcc/src/hg/trunk/solaris/gcc/config/i386/i386.c: In function 'legitimize_tls_address':
/var/gcc/src/hg/trunk/solaris/gcc/config/i386/i386.c:12551:4: error: implicit declaration of function 'gen_tls_initial_exec_64_sun' [-Werror=implicit-function-declaration]
/var/gcc/src/hg/trunk/solaris/gcc/config/i386/i386.c:12551:4: error: passing argument 1 of 'emit_insn' makes pointer from integer without a cast [-Werror]
/var/gcc/src/hg/trunk/solaris/gcc/rtl.h:1725:12: note: expected 'rtx' but argument is of type 'int'
cc1: all warnings being treated as errors

make[3]: *** [i386.o] Error 1

gen_tls_initial_exec_64_sun isn't declared in insn-flags.h, probably
because TARGET_SUN_TLS is 0 in i386.h.  What's the best way to avoid
this?  Wrap the TARGET_64BIT && TARGET_SUN_TLS section in i386.c
(legitimize_tls_address) in #if TARGET_SUN_TLS?

	Rainer
diff mbox

Patch

Index: i386.c
===================================================================
--- i386.c	(revision 168050)
+++ i386.c	(working copy)
@@ -12571,7 +12571,21 @@  legitimize_tls_address (rtx x, enum tls_
       off = gen_const_mem (Pmode, off);
       set_mem_alias_set (off, ix86_GOT_alias_set ());
 
-      if (TARGET_64BIT || TARGET_ANY_GNU_TLS)
+      if (TARGET_64BIT && TARGET_SUN_TLS)
+	{
+	  /* The SUN linker took the AMD64 TLS spec literally
+	     and can only handle %rax as destination of the
+	     initial executable code sequence.  */
+
+	  dest = gen_reg_rtx (Pmode);
+
+	  base = get_thread_pointer (false);
+	  emit_insn (gen_rtx_SET (VOIDmode, dest, base));
+	  emit_insn (gen_add_tls_addr_di_sun (dest, dest, off));
+
+	  return dest;
+	}
+      else if (TARGET_64BIT || TARGET_ANY_GNU_TLS)
 	{
           base = get_thread_pointer (for_mov || !TARGET_TLS_DIRECT_SEG_REFS);
 	  off = force_reg (Pmode, off);
Index: i386.md
===================================================================
--- i386.md	(revision 168050)
+++ i386.md	(working copy)
@@ -93,6 +93,7 @@ 
   UNSPEC_TLS_GD
   UNSPEC_TLS_LD_BASE
   UNSPEC_TLSDESC
+  UNSPEC_TLSDESC_SUN
 
   ;; Other random patterns
   UNSPEC_SCAS
@@ -12661,6 +12664,31 @@ 
 ;; Segment register for the thread base ptr load
 (define_mode_attr tp_seg [(SI "gs") (DI "fs")])
 
+;; The SUN linker took the AMD64 TLS spec literally and can only handle
+;; %rax as the destination of the initial executable code sequence.
+(define_insn "*load_tp_di_sun"
+  [(set (match_operand:DI 0 "register_operand" "=a")
+	(unspec:DI [(const_int 0)] UNSPEC_TP))]
+  "TARGET_64BIT && TARGET_SUN_TLS"
+  "mov{q}\t{%%fs:0, %0|%0, QWORD PTR fs:0}"
+  [(set_attr "type" "imov")
+   (set_attr "modrm" "0")
+   (set_attr "length" "7")
+   (set_attr "memory" "load")
+   (set_attr "imm_disp" "false")])
+
+(define_insn "add_tls_addr_di_sun"
+  [(set (match_operand:DI 0 "register_operand" "=a")
+  	(plus:DI
+	  (match_operand:DI 1 "register_operand" "0")
+	  (match_operand:DI 2 "memory_operand" "m")))
+   (clobber (reg:CC FLAGS_REG))
+   (unspec [(const_int 0)] UNSPEC_TLSDESC_SUN)]
+  "TARGET_64BIT && TARGET_SUN_TLS"
+  "add{q}\t{%2, %0|%0, %2}";
+  [(set_attr "type" "alu")
+   (set_attr "mode" "DI")])
+
 ;; Load and add the thread base pointer from %gs:0.
 (define_insn "*load_tp_<mode>"
   [(set (match_operand:P 0 "register_operand" "=r")