diff mbox

powerpc64 large-toc vs. reload

Message ID 20110619140336.GM21332@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra June 19, 2011, 2:03 p.m. UTC 
I was alerted to a problem with large toc (-mcmodel=medium/large) code
a few days ago by warnings emitted during a binutils build.

dwarf.c: In function 'display_debug_lines_raw':
dwarf.c:2409:1: note: non-delegitimized UNSPEC UNSPEC_TOCREL (44) found in variable location
dwarf.c:2409:1: note: non-delegitimized UNSPEC UNSPEC_TOCREL (44) found in variable location

On investigating why this was happening, I found that these UNSPECs
were from the high part calculation of a toc-relative address that
didn't get a register.  reload allocated the pseudo to a stack slot..
The stack slot didn't match the tight pattern in delegitimize_address
which is why we have an UNSPEC left in the debug info.  The real
problem of course is that reload should never allocate a stack slot
for a simple address calculation that can be rematerialised anywhere
in the function with just one instruction.  So after quite a bit of
digging around in reload, I finally figured out that the problem has a
really easy solution.  Simply tell reload that those high part address
calculations are constants.  Which is true.

That's what the create_TOC_reference change, and the rs6000.md changes
below do.  (I also fix large-toc tls patterns.)  The rest of the patch
just adjusts for the changed RTL.

Bootstrap and regression tests powerpc64-linux in progress.  OK to
apply mainline and 4.6 assuming no regressions?

	* config/rs6000/rs6000.c (create_TOC_reference): Wrap high part
	of toc-relative address in CONST.
	(rs6000_delegitimize_address): Recognize changed address.
	(rs6000_legitimize_reload_address): Likewise.
	(rs6000_emit_move): Don't force these constants to memory.
	* config/rs6000/rs6000.md (tls_gd, tls_gd_high): Wrap high part of
	toc-relative address in CONST.
	(tls_ld, tls_ld_high, tls_got_dtprel, tls_got_dtprel_high): Likewise.
	(tls_got_tprel, tls_got_tprel_high, largetoc_high): Likewise.

Comments

David Edelsohn June 20, 2011, 3:04 a.m. UTC | #1 
On Sun, Jun 19, 2011 at 10:03 AM, Alan Modra <amodra@gmail.com> wrote:
> I was alerted to a problem with large toc (-mcmodel=medium/large) code
> a few days ago by warnings emitted during a binutils build.
>
> dwarf.c: In function 'display_debug_lines_raw':
> dwarf.c:2409:1: note: non-delegitimized UNSPEC UNSPEC_TOCREL (44) found in variable location
> dwarf.c:2409:1: note: non-delegitimized UNSPEC UNSPEC_TOCREL (44) found in variable location
>
> On investigating why this was happening, I found that these UNSPECs
> were from the high part calculation of a toc-relative address that
> didn't get a register.  reload allocated the pseudo to a stack slot..
> The stack slot didn't match the tight pattern in delegitimize_address
> which is why we have an UNSPEC left in the debug info.  The real
> problem of course is that reload should never allocate a stack slot
> for a simple address calculation that can be rematerialised anywhere
> in the function with just one instruction.  So after quite a bit of
> digging around in reload, I finally figured out that the problem has a
> really easy solution.  Simply tell reload that those high part address
> calculations are constants.  Which is true.
>
> That's what the create_TOC_reference change, and the rs6000.md changes
> below do.  (I also fix large-toc tls patterns.)  The rest of the patch
> just adjusts for the changed RTL.
>
> Bootstrap and regression tests powerpc64-linux in progress.  OK to
> apply mainline and 4.6 assuming no regressions?
>
>        * config/rs6000/rs6000.c (create_TOC_reference): Wrap high part
>        of toc-relative address in CONST.
>        (rs6000_delegitimize_address): Recognize changed address.
>        (rs6000_legitimize_reload_address): Likewise.
>        (rs6000_emit_move): Don't force these constants to memory.
>        * config/rs6000/rs6000.md (tls_gd, tls_gd_high): Wrap high part of
>        toc-relative address in CONST.
>        (tls_ld, tls_ld_high, tls_got_dtprel, tls_got_dtprel_high): Likewise.
>        (tls_got_tprel, tls_got_tprel_high, largetoc_high): Likewise.

Okay.

Thanks, David
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 175175)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -5803,12 +5803,13 @@  rs6000_delegitimize_address (rtx orig_x)
 		   || TARGET_MINIMAL_TOC
 		   || TARGET_CMODEL != CMODEL_SMALL))
 	      || (TARGET_CMODEL != CMODEL_SMALL
-		  && GET_CODE (XEXP (x, 0)) == PLUS
-		  && GET_CODE (XEXP (XEXP (x, 0), 0)) == REG
-		  && REGNO (XEXP (XEXP (x, 0), 0)) == TOC_REGISTER
-		  && GET_CODE (XEXP (XEXP (x, 0), 1)) == HIGH
+		  && GET_CODE (XEXP (x, 0)) == CONST
+		  && GET_CODE (XEXP (XEXP (x, 0), 0)) == PLUS
+		  && GET_CODE (XEXP (XEXP (XEXP (x, 0), 0), 0)) == REG
+		  && REGNO (XEXP (XEXP (XEXP (x, 0), 0), 0)) == TOC_REGISTER
+		  && GET_CODE (XEXP (XEXP (XEXP (x, 0), 0), 1)) == HIGH
 		  && rtx_equal_p (XEXP (x, 1),
-				  XEXP (XEXP (XEXP (x, 0), 1), 0)))))
+				  XEXP (XEXP (XEXP (XEXP (x, 0), 0), 1), 0)))))
 	{
 	  y = XVECEXP (y, 0, 0);
 	  if (offset != NULL_RTX)
@@ -6147,11 +6148,12 @@  rs6000_legitimize_reload_address (rtx x,
       && GET_CODE (XEXP (x, 0)) == PLUS
       && GET_CODE (XEXP (XEXP (x, 0), 0)) == REG
       && REGNO (XEXP (XEXP (x, 0), 0)) == TOC_REGISTER
-      && GET_CODE (XEXP (XEXP (x, 0), 1)) == HIGH
+      && GET_CODE (XEXP (XEXP (x, 0), 1)) == CONST
+      && GET_CODE (XEXP (XEXP (XEXP (x, 0), 1), 0)) == HIGH
       && GET_CODE (XEXP (x, 1)) == CONST
       && GET_CODE (XEXP (XEXP (x, 1), 0)) == UNSPEC
       && XINT (XEXP (XEXP (x, 1), 0), 1) == UNSPEC_TOCREL
-      && rtx_equal_p (XEXP (XEXP (XEXP (x, 0), 1), 0), XEXP (x, 1)))
+      && rtx_equal_p (XEXP (XEXP (XEXP (XEXP (x, 0), 1), 0), 0), XEXP (x, 1)))
     {
       push_reload (XEXP (x, 0), NULL_RTX, &XEXP (x, 0), NULL,
 		   BASE_REG_CLASS, Pmode, VOIDmode, 0, 0,
@@ -7197,6 +7196,11 @@  rs6000_emit_move (rtx dest, rtx source, 
 	}
       else if (mode == Pmode
 	       && CONSTANT_P (operands[1])
+	       && GET_CODE (operands[1]) != HIGH
+	       && !(TARGET_CMODEL != CMODEL_SMALL
+		    && GET_CODE (operands[1]) == CONST
+		    && GET_CODE (XEXP (operands[1], 0)) == PLUS
+		    && GET_CODE (XEXP (XEXP (operands[1], 0), 1)) == HIGH)
 	       && ((GET_CODE (operands[1]) != CONST_INT
 		    && ! easy_fp_constant (operands[1], mode))
 		   || (GET_CODE (operands[1]) == CONST_INT
@@ -7204,7 +7208,6 @@  rs6000_emit_move (rtx dest, rtx source, 
 			   > (TARGET_CMODEL != CMODEL_SMALL ? 3 : 2)))
 		   || (GET_CODE (operands[0]) == REG
 		       && FP_REGNO_P (REGNO (operands[0]))))
-	       && GET_CODE (operands[1]) != HIGH
 	       && ! legitimate_constant_pool_address_p (operands[1], mode,
 							false)
 	       && ! toc_relative_expr_p (operands[1])
@@ -19063,7 +19078,9 @@  create_TOC_reference (rtx symbol, rtx la
   tocreg = gen_rtx_REG (Pmode, TOC_REGISTER);
   if (TARGET_CMODEL != CMODEL_SMALL)
     {
-      rtx hi = gen_rtx_PLUS (Pmode, tocreg, gen_rtx_HIGH (Pmode, tocrel));
+      rtx hi = gen_rtx_CONST (Pmode,
+			      gen_rtx_PLUS (Pmode, tocreg, 
+					    gen_rtx_HIGH (Pmode, tocrel)));
       if (largetoc_reg != NULL)
 	{
 	  emit_move_insn (largetoc_reg, hi);
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 175175)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -11523,9 +11523,10 @@  (define_insn_and_split "*tls_gd<TLSmode:
   "addi %0,%1,%2@got@tlsgd"
   "&& TARGET_CMODEL != CMODEL_SMALL"
   [(set (match_dup 3)
-  	(plus:TLSmode (match_dup 1)
-	  (high:TLSmode
-	    (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGD))))
+	(const:TLSmode
+	  (plus:TLSmode (match_dup 1)
+	    (high:TLSmode
+	      (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGD)))))
    (set (match_dup 0)
    	(lo_sum:TLSmode (match_dup 3)
 	    (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGD)))]
@@ -11540,10 +11541,11 @@  (define_insn_and_split "*tls_gd<TLSmode:
 
 (define_insn "*tls_gd_high<TLSmode:tls_abi_suffix>"
   [(set (match_operand:TLSmode 0 "gpc_reg_operand" "=b")
-     (plus:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
-       (high:TLSmode
-	  (unspec:TLSmode [(match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
-			  UNSPEC_TLSGD))))]
+     (const:TLSmode
+       (plus:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
+	 (high:TLSmode
+	   (unspec:TLSmode [(match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
+			   UNSPEC_TLSGD)))))]
   "HAVE_AS_TLS && TARGET_TLS_MARKERS && TARGET_CMODEL != CMODEL_SMALL"
   "addis %0,%1,%2@got@tlsgd@ha"
   [(set_attr "length" "4")])
@@ -11658,9 +11660,10 @@  (define_insn_and_split "*tls_ld<TLSmode:
   "addi %0,%1,%&@got@tlsld"
   "&& TARGET_CMODEL != CMODEL_SMALL"
   [(set (match_dup 2)
-  	(plus:TLSmode (match_dup 1)
-	  (high:TLSmode
-	    (unspec:TLSmode [(const_int 0)] UNSPEC_TLSLD))))
+	(const:TLSmode
+	  (plus:TLSmode (match_dup 1)
+	    (high:TLSmode
+	      (unspec:TLSmode [(const_int 0)] UNSPEC_TLSLD)))))
    (set (match_dup 0)
    	(lo_sum:TLSmode (match_dup 2)
 	    (unspec:TLSmode [(const_int 0)] UNSPEC_TLSLD)))]
@@ -11675,9 +11678,10 @@  (define_insn_and_split "*tls_ld<TLSmode:
 
 (define_insn "*tls_ld_high<TLSmode:tls_abi_suffix>"
   [(set (match_operand:TLSmode 0 "gpc_reg_operand" "=b")
-     (plus:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
-       (high:TLSmode
-	  (unspec:TLSmode [(const_int 0)] UNSPEC_TLSLD))))]
+     (const:TLSmode
+       (plus:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
+	 (high:TLSmode
+	   (unspec:TLSmode [(const_int 0)] UNSPEC_TLSLD)))))]
   "HAVE_AS_TLS && TARGET_TLS_MARKERS && TARGET_CMODEL != CMODEL_SMALL"
   "addis %0,%1,%&@got@tlsld@ha"
   [(set_attr "length" "4")])
@@ -11753,9 +11757,10 @@  (define_insn_and_split "tls_got_dtprel_<
   "l<TLSmode:tls_insn_suffix> %0,%2@got@dtprel(%1)"
   "&& TARGET_CMODEL != CMODEL_SMALL"
   [(set (match_dup 3)
-	(plus:TLSmode (match_dup 1)
-	  (high:TLSmode
-	    (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGOTDTPREL))))
+	(const:TLSmode
+	  (plus:TLSmode (match_dup 1)
+	    (high:TLSmode
+	      (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGOTDTPREL)))))
    (set (match_dup 0)
 	(lo_sum:TLSmode (match_dup 3)
 	    (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGOTDTPREL)))]
@@ -11770,10 +11775,11 @@  (define_insn_and_split "tls_got_dtprel_<
 
 (define_insn "*tls_got_dtprel_high<TLSmode:tls_abi_suffix>"
   [(set (match_operand:TLSmode 0 "gpc_reg_operand" "=b")
-     (plus:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
-       (high:TLSmode
-	 (unspec:TLSmode [(match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
-			 UNSPEC_TLSGOTDTPREL))))]
+     (const:TLSmode
+       (plus:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
+	 (high:TLSmode
+	   (unspec:TLSmode [(match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
+			   UNSPEC_TLSGOTDTPREL)))))]
   "HAVE_AS_TLS && TARGET_CMODEL != CMODEL_SMALL"
   "addis %0,%1,%2@got@dtprel@ha"
   [(set_attr "length" "4")])
@@ -11823,9 +11829,10 @@  (define_insn_and_split "tls_got_tprel_<T
   "l<TLSmode:tls_insn_suffix> %0,%2@got@tprel(%1)"
   "&& TARGET_CMODEL != CMODEL_SMALL"
   [(set (match_dup 3)
-	(plus:TLSmode (match_dup 1)
-	  (high:TLSmode
-	    (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGOTTPREL))))
+	(const:TLSmode
+	  (plus:TLSmode (match_dup 1)
+	    (high:TLSmode
+	      (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGOTTPREL)))))
    (set (match_dup 0)
 	(lo_sum:TLSmode (match_dup 3)
 	    (unspec:TLSmode [(match_dup 2)] UNSPEC_TLSGOTTPREL)))]
@@ -11840,10 +11847,11 @@  (define_insn_and_split "tls_got_tprel_<T
 
 (define_insn "*tls_got_tprel_high<TLSmode:tls_abi_suffix>"
   [(set (match_operand:TLSmode 0 "gpc_reg_operand" "=b")
-     (plus:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
-       (high:TLSmode
-	 (unspec:TLSmode [(match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
-			 UNSPEC_TLSGOTTPREL))))]
+     (const:TLSmode
+       (plus:TLSmode (match_operand:TLSmode 1 "gpc_reg_operand" "b")
+	 (high:TLSmode
+	   (unspec:TLSmode [(match_operand:TLSmode 2 "rs6000_tls_symbol_ref" "")]
+			   UNSPEC_TLSGOTTPREL)))))]
   "HAVE_AS_TLS && TARGET_CMODEL != CMODEL_SMALL"
   "addis %0,%1,%2@got@tprel@ha"
   [(set_attr "length" "4")])
@@ -12157,8 +12165,9 @@  (define_insn "elf_low"
 ;; Largetoc support
 (define_insn "largetoc_high"
   [(set (match_operand:DI 0 "gpc_reg_operand" "=b")
-        (plus:DI (match_operand:DI 1 "gpc_reg_operand" "b")
-	         (high:DI (match_operand:DI 2 "" ""))))]
+	(const:DI
+	  (plus:DI (match_operand:DI 1 "gpc_reg_operand" "b")
+		   (high:DI (match_operand:DI 2 "" "")))))]
    "TARGET_ELF && TARGET_CMODEL != CMODEL_SMALL"
    "{cau|addis} %0,%1,%2@ha")