[rs6000] Keep TOC register live in TLS lo_sum patterns

Message ID	201307101108.r6AB8Fr4025718@d06av02.portsmouth.uk.ibm.com
State	New
Headers	show Return-Path: <gcc-patches-return-344843-incoming=patchwork.ozlabs.org@gcc.gnu.org> DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:to:date:from:cc:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=nABTKbN+9PsuhWV/ Oc4LCe9QmRYW3PaMS0NuZKZDvIvxXY0vBY+8bQcvyYfcctNjg0bVPuZPTwOzLmsg XW5aD5EQ1yO9/Is2s6YRiyB+EkHA3VnxFyHIfwP83fIb0PEzBscXrkOJKrdCvhWR lA1ENO4lxJuPX8k2KAS2qDoOCmA= Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk Sender: gcc-patches-owner@gcc.gnu.org Gateway: Authorized Use Only! Violators will be prosecuted for <gcc-patches@gcc.gnu.org> from <uweigand@de.ibm.com>; Wed, 10 Jul 2013 12:02:40 +0100 Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 10 Jul 2013 12:02:38 +0100 Message-Id: <201307101108.r6AB8Fr4025718@d06av02.portsmouth.uk.ibm.com> Subject: [PATCH, rs6000] Keep TOC register live in TLS lo_sum patterns To: gcc-patches@gcc.gnu.org Date: Wed, 10 Jul 2013 13:08:14 +0200 (CEST) From: "Ulrich Weigand" <uweigand@de.ibm.com> Cc: dje.gcc@gmail.com MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit

Message ID

201307101108.r6AB8Fr4025718@d06av02.portsmouth.uk.ibm.com

State

New

Headers

DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id
	:list-unsubscribe:list-archive:list-post:list-help:sender
	:message-id:subject:to:date:from:cc:mime-version:content-type
	:content-transfer-encoding; q=dns; s=default; b=nABTKbN+9PsuhWV/
	Oc4LCe9QmRYW3PaMS0NuZKZDvIvxXY0vBY+8bQcvyYfcctNjg0bVPuZPTwOzLmsg
	XW5aD5EQ1yO9/Is2s6YRiyB+EkHA3VnxFyHIfwP83fIb0PEzBscXrkOJKrdCvhWR
	lA1ENO4lxJuPX8k2KAS2qDoOCmA=
Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
Sender: gcc-patches-owner@gcc.gnu.org
Message-Id: <201307101108.r6AB8Fr4025718@d06av02.portsmouth.uk.ibm.com>
Subject: [PATCH, rs6000] Keep TOC register live in TLS lo_sum patterns
To: gcc-patches@gcc.gnu.org
Date: Wed, 10 Jul 2013 13:08:14 +0200 (CEST)
From: "Ulrich Weigand" <uweigand@de.ibm.com>
Cc: dje.gcc@gmail.com
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Commit Message

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

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.

Comments

David Edelsohn July 10, 2013, 8:44 p.m. UTC | #1

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

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)"

[rs6000] Keep TOC register live in TLS lo_sum patterns

Commit Message

Comments

Patch