Patchwork [rs6000] Keep TOC register live in TLS lo_sum patterns

login
register
mail settings
Submitter Ulrich Weigand
Date July 10, 2013, 11:08 a.m.
Message ID <201307101108.r6AB8Fr4025718@d06av02.portsmouth.uk.ibm.com>
Download mbox | patch
Permalink /patch/258012/
State New
Headers show

Comments

Ulrich Weigand - July 10, 2013, 11:08 a.m.
Hello,

the ppc64 linker performs an optimization to avoid multi-instruction
TOC accesses in some cases:

      /* Multi-instruction sequences that access the TOC can be
         optimized, eg. addis ra,r2,0; addi rb,ra,x;
         to             nop;           addi rb,r2,x;  */

For this optimization to be valid, the TOC register r2 has to be live
at the point of the second instruction (addi rb,ra,x), even though
this instruction -on its face- does not use r2.  It is apparently
the responsibility of the compiler to keep r2 live at this point
on any such instruction (identified by using some low-part TOC 
relocation).

There is code in rs6000.md that does this for "normal" accesses to
variables in the TOC.  However, code that accesses TLS-related
information in the TOC (GOT_TLSGD and friends) does *not* ensure
r2 is live.

This usually doesn't matter because r2 tends to live across the
whole function anyway.  However, in the context of some out-of-tree
patches I ran into a situation where this caused a bug.  Since it
would probably be theoretically possible to run into this issue
even with mainline GCC (and in any case fixing the problem doesn't
have any drawbacks), I'd suggest to fix this on mainline.

This patch adds an extra operand to the UNSPEC operands of the
TLS-related lo_sum patterns, used to enforce keeping the TOC
register live.

Tested with no regressions on powerpc64-linux.

OK for mainline?

Bye,
Ulrich


ChangeLog:

	* config/rs6000/rs6000.md (""*tls_gd_low<TLSmode:tls_abi_suffix>"):
	Require GOT register as additional operand in UNSPEC.
	("*tls_ld_low<TLSmode:tls_abi_suffix>"): Likewise.
	("*tls_got_dtprel_low<TLSmode:tls_abi_suffix>"): Likewise.
	("*tls_got_tprel_low<TLSmode:tls_abi_suffix>"): Likewise.
	("*tls_gd<TLSmode:tls_abi_suffix>"): Update splitter.
	("*tls_ld<TLSmode:tls_abi_suffix>"): Likewise.
	("tls_got_dtprel_<TLSmode:tls_abi_suffix>"): Likewise.
	("tls_got_tprel_<TLSmode:tls_abi_suffix>"): Likewise.
David Edelsohn - July 10, 2013, 8:44 p.m.
On Wed, Jul 10, 2013 at 7:08 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Hello,
>
> the ppc64 linker performs an optimization to avoid multi-instruction
> TOC accesses in some cases:
>
>       /* Multi-instruction sequences that access the TOC can be
>          optimized, eg. addis ra,r2,0; addi rb,ra,x;
>          to             nop;           addi rb,r2,x;  */
>
> For this optimization to be valid, the TOC register r2 has to be live
> at the point of the second instruction (addi rb,ra,x), even though
> this instruction -on its face- does not use r2.  It is apparently
> the responsibility of the compiler to keep r2 live at this point
> on any such instruction (identified by using some low-part TOC
> relocation).
>
> There is code in rs6000.md that does this for "normal" accesses to
> variables in the TOC.  However, code that accesses TLS-related
> information in the TOC (GOT_TLSGD and friends) does *not* ensure
> r2 is live.
>
> This usually doesn't matter because r2 tends to live across the
> whole function anyway.  However, in the context of some out-of-tree
> patches I ran into a situation where this caused a bug.  Since it
> would probably be theoretically possible to run into this issue
> even with mainline GCC (and in any case fixing the problem doesn't
> have any drawbacks), I'd suggest to fix this on mainline.
>
> This patch adds an extra operand to the UNSPEC operands of the
> TLS-related lo_sum patterns, used to enforce keeping the TOC
> register live.
>
> Tested with no regressions on powerpc64-linux.
>
> OK for mainline?
>
> Bye,
> Ulrich
>
>
> ChangeLog:
>
>         * config/rs6000/rs6000.md (""*tls_gd_low<TLSmode:tls_abi_suffix>"):
>         Require GOT register as additional operand in UNSPEC.
>         ("*tls_ld_low<TLSmode:tls_abi_suffix>"): Likewise.
>         ("*tls_got_dtprel_low<TLSmode:tls_abi_suffix>"): Likewise.
>         ("*tls_got_tprel_low<TLSmode:tls_abi_suffix>"): Likewise.
>         ("*tls_gd<TLSmode:tls_abi_suffix>"): Update splitter.
>         ("*tls_ld<TLSmode:tls_abi_suffix>"): Likewise.
>         ("tls_got_dtprel_<TLSmode:tls_abi_suffix>"): Likewise.
>         ("tls_got_tprel_<TLSmode:tls_abi_suffix>"): Likewise.

This patch is okay.

Thanks, David

Patch

Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 200784)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -10989,7 +10989,7 @@ 
 	    (unspec:TLSmode [(match_dup 1) (match_dup 2)] UNSPEC_TLSGD)))
    (set (match_dup 0)
    	(lo_sum:TLSmode (match_dup 3)
-	    (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGD)))]
+	    (unspec:TLSmode [(match_dup 1) (match_dup 2)] UNSPEC_TLSGD)))]
   "
 {
   operands[3] = gen_reg_rtx (TARGET_64BIT ? DImode : SImode);
@@ -11012,7 +11012,8 @@ 
 (define_insn "*tls_gd_low<TLSmode:tls_abi_suffix>"
   [(set (match_operand:TLSmode 0 "gpc_reg_operand" "=b")
      (lo_sum:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
-       (unspec:TLSmode [(match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
+       (unspec:TLSmode [(match_operand:TLSmode 3 "gpc_reg_operand" "b")
+			(match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
 		       UNSPEC_TLSGD)))]
   "HAVE_AS_TLS && TARGET_TLS_MARKERS && TARGET_CMODEL != CMODEL_SMALL"
   "addi %0,%1,%2@got@tlsgd@l"
@@ -11124,7 +11125,7 @@ 
 	    (unspec:TLSmode [(const_int 0) (match_dup 1)] UNSPEC_TLSLD)))
    (set (match_dup 0)
    	(lo_sum:TLSmode (match_dup 2)
-	    (unspec:TLSmode [(const_int 0)] UNSPEC_TLSLD)))]
+	    (unspec:TLSmode [(const_int 0) (match_dup 1)] UNSPEC_TLSLD)))]
   "
 {
   operands[2] = gen_reg_rtx (TARGET_64BIT ? DImode : SImode);
@@ -11147,7 +11148,9 @@ 
 (define_insn "*tls_ld_low<TLSmode:tls_abi_suffix>"
   [(set (match_operand:TLSmode 0 "gpc_reg_operand" "=b")
      (lo_sum:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
-       (unspec:TLSmode [(const_int 0)] UNSPEC_TLSLD)))]
+       (unspec:TLSmode [(const_int 0)
+                        (match_operand:TLSmode 2 "gpc_reg_operand" "b")]
+                       UNSPEC_TLSLD)))]
   "HAVE_AS_TLS && TARGET_TLS_MARKERS && TARGET_CMODEL != CMODEL_SMALL"
   "addi %0,%1,%&@got@tlsld@l"
   [(set_attr "length" "4")])
@@ -11219,7 +11222,7 @@ 
 	    (unspec:TLSmode [(match_dup 1) (match_dup 2)] UNSPEC_TLSGOTDTPREL)))
    (set (match_dup 0)
 	(lo_sum:TLSmode (match_dup 3)
-	    (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGOTDTPREL)))]
+	    (unspec:TLSmode [(match_dup 1) (match_dup 2)] UNSPEC_TLSGOTDTPREL)))]
   "
 {
   operands[3] = gen_reg_rtx (TARGET_64BIT ? DImode : SImode);
@@ -11242,7 +11245,8 @@ 
 (define_insn "*tls_got_dtprel_low<TLSmode:tls_abi_suffix>"
   [(set (match_operand:TLSmode 0 "gpc_reg_operand" "=r")
      (lo_sum:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
-	 (unspec:TLSmode [(match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
+	 (unspec:TLSmode [(match_operand:TLSmode 3 "gpc_reg_operand" "b")
+			  (match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
 			 UNSPEC_TLSGOTDTPREL)))]
   "HAVE_AS_TLS && TARGET_CMODEL != CMODEL_SMALL"
   "l<TLSmode:tls_insn_suffix> %0,%2@got@dtprel@l(%1)"
@@ -11288,7 +11292,7 @@ 
 	    (unspec:TLSmode [(match_dup 1) (match_dup 2)] UNSPEC_TLSGOTTPREL)))
    (set (match_dup 0)
 	(lo_sum:TLSmode (match_dup 3)
-	    (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGOTTPREL)))]
+	    (unspec:TLSmode [(match_dup 1) (match_dup 2)] UNSPEC_TLSGOTTPREL)))]
   "
 {
   operands[3] = gen_reg_rtx (TARGET_64BIT ? DImode : SImode);
@@ -11311,7 +11315,8 @@ 
 (define_insn "*tls_got_tprel_low<TLSmode:tls_abi_suffix>"
   [(set (match_operand:TLSmode 0 "gpc_reg_operand" "=r")
      (lo_sum:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
-	 (unspec:TLSmode [(match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
+	 (unspec:TLSmode [(match_operand:TLSmode 3 "gpc_reg_operand" "b")
+			  (match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
 			 UNSPEC_TLSGOTTPREL)))]
   "HAVE_AS_TLS && TARGET_CMODEL != CMODEL_SMALL"
   "l<TLSmode:tls_insn_suffix> %0,%2@got@tprel@l(%1)"