Patchwork [rs6000] Preserve link stack for 476 cpus

login
register
mail settings
Submitter Peter Bergner
Date Oct. 13, 2011, 3:49 p.m.
Message ID <1318520967.3133.69.camel@otta>
Download mbox | patch
Permalink /patch/119562/
State New
Headers show

Comments

Peter Bergner - Oct. 13, 2011, 3:49 p.m.
On Mon, 2011-09-12 at 15:29 -0400, David Edelsohn wrote:
> First, please choose a more informative option name.
> -mpreserve-link-stack seems like something generally useful for all
> processors and someone may randomly add the option.  It always is
> useful to preserve the link stack -- that's why you're jumping through
> hoops to fix this bug.  Maybe -mpreserve-ppc476-link-stack .

Done.


> I would prefer that this patch were maintained by the chip vendors
> distributing SDKs for PPC476 instead of complicating the FSF codebase.

Talking with the chip folks, they said there were a number of companies
already downloading the FSF gcc sources and building it unpatched and
that they expected more to do so in the future, so I'm not sure how many
(if any) are actually even relying on a SDK.  So...


> Otherwise, please implement this like Xilinx FPU in rs6000.opt,
> rs6000.h, ppc476.h and config.gcc where TARGET_LINK_STACK is defined
> as 0 unless GCC explicitly is configured for powerpc476.

Here's a patch to do that, by adding a variant to the powerpc*-*-linux*
target for the 476.  I bootstrapped and regtested this as before, meaning
I also tested this with the -mpreserve-ppc476-link-stack on by default,
as well as configuring without 476 support and verified that the
TARGET_LINK_STACK tests are not only optimized away, but so is the
-mpreserve-ppc476-link-stack option itself.

Is this ok for mainline now?

Peter


	* config.gcc (powerpc*-*-linux*): Add powerpc*-*-linux*ppc476* variant.
	* config/rs6000/476.h: New file.
	* config/rs6000/476.opt: Likewise.
	* config/rs6000/rs6000.h (TARGET_LINK_STACK): New define.
	(SET_TARGET_LINK_STACK): Likewise.
	* config/rs6000/rs6000.c (rs6000_option_override_internal): Enable
	TARGET_LINK_STACK for -mtune=476 and -mtune=476fp.
	(rs6000_legitimize_tls_address): Emit the link stack preserving GOT
	code if TARGET_LINK_STACK.
	(rs6000_emit_load_toc_table): Likewise.
	(output_function_profiler): Likewise
	(macho_branch_islands): Likewise
	(machopic_output_stub): Likewise
	* config/rs6000/rs6000.md (load_toc_v4_PIC_1, load_toc_v4_PIC_1b):
	Convert to a define_expand.
	(load_toc_v4_PIC_1_normal): New define_insn.
	(load_toc_v4_PIC_1_476): Likewise.
	(load_toc_v4_PIC_1b_normal): Likewise.
	(load_toc_v4_PIC_1b_476): Likewise.
Richard Henderson - Oct. 13, 2011, 5:03 p.m.
On 10/13/2011 08:49 AM, Peter Bergner wrote:
> +	  if (TARGET_LINK_STACK)
> +	    asm_fprintf (file, "\tbl 1f\n\tb 2f\n1:\n\tblr\n2:\n");
> +	  else
> +	    asm_fprintf (file, "\tbcl 20,31,1f\n1:\n");

Wouldn't it be better to set up an out-of-line "blr" insn that could
be shared by all instances?  That would solve a lot of this sort of
this sort of branch-to-branch-to-branch ugliness.

See the i386 port for an example of this, if you need it.


r~

Patch

Index: gcc/config.gcc
===================================================================
--- gcc/config.gcc	(revision 179091)
+++ gcc/config.gcc	(working copy)
@@ -2133,6 +2133,9 @@  powerpc-*-linux* | powerpc64-*-linux*)
 	esac
 	tmake_file="${tmake_file} t-slibgcc-libgcc"
 	case ${target} in
+	    powerpc*-*-linux*ppc476*)
+		tm_file="${tm_file} rs6000/476.h"
+		extra_options="${extra_options} rs6000/476.opt" ;;
 	    powerpc*-*-linux*altivec*)
 		tm_file="${tm_file} rs6000/linuxaltivec.h" ;;
 	    powerpc*-*-linux*spe*)
Index: gcc/config/rs6000/476.h
===================================================================
--- gcc/config/rs6000/476.h	(revision 0)
+++ gcc/config/rs6000/476.h	(revision 0)
@@ -0,0 +1,29 @@ 
+/* Enable IBM PowerPC 476 support.
+   Copyright (C) 2011 Free Software Foundation, Inc.
+   Contributed by Peter Bergner (bergner@vnet.ibm.com)
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published
+   by the Free Software Foundation; either version 3, or (at your
+   option) any later version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+   License for more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#undef TARGET_LINK_STACK
+#define TARGET_LINK_STACK (rs6000_link_stack)
+
+#undef SET_TARGET_LINK_STACK
+#define SET_TARGET_LINK_STACK(X) do { TARGET_LINK_STACK = (X); } while (0)
Index: gcc/config/rs6000/476.opt
===================================================================
--- gcc/config/rs6000/476.opt	(revision 0)
+++ gcc/config/rs6000/476.opt	(revision 0)
@@ -0,0 +1,24 @@ 
+; IBM PowerPC 476 options.
+;
+; Copyright (C) 2011 Free Software Foundation, Inc.
+; Contributed by Peter Bergner (bergner@vnet.ibm.com)
+;
+; This file is part of GCC.
+;
+; GCC is free software; you can redistribute it and/or modify it under
+; the terms of the GNU General Public License as published by the Free
+; Software Foundation; either version 3, or (at your option) any later
+; version.
+;
+; GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+; WARRANTY; without even the implied warranty of MERCHANTABILITY or
+; FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+; for more details.
+;
+; You should have received a copy of the GNU General Public License
+; along with GCC; see the file COPYING3.  If not see
+; <http://www.gnu.org/licenses/>.
+
+mpreserve-ppc476-link-stack
+Target Var(rs6000_link_stack) Init(-1) Save
+Preserve the PowerPC 476's link stack by matching up a blr with the bcl/bl insns used for GOT accesses
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 179091)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -3244,6 +3244,11 @@  rs6000_option_override_internal (bool gl
     target_option_default_node = target_option_current_node
       = build_target_option_node ();
 
+  /* If not explicitly specified via option, decide whether to generate the
+     extra blr's required to preserve the link stack on some cpus (eg, 476).  */
+  if (TARGET_LINK_STACK == -1)
+    SET_TARGET_LINK_STACK (rs6000_cpu == PROCESSOR_PPC476);
+
   return ret;
 }
 
@@ -5932,6 +5937,8 @@  rs6000_legitimize_tls_address (rtx addr,
 		  lab = gen_label_rtx ();
 		  emit_insn (gen_load_toc_v4_PIC_1b (gsym, lab));
 		  emit_move_insn (tmp1, gen_rtx_REG (Pmode, LR_REGNO));
+		  if (TARGET_LINK_STACK)
+		    emit_insn (gen_addsi3 (tmp1, tmp1, GEN_INT (4)));
 		  emit_move_insn (tmp2, mem);
 		  last = emit_insn (gen_addsi3 (got, tmp1, tmp2));
 		  set_unique_reg_note (last, REG_EQUAL, gsym);
@@ -18923,6 +18930,8 @@  rs6000_emit_load_toc_table (int fromprol
 	  lab = gen_label_rtx ();
 	  emit_insn (gen_load_toc_v4_PIC_1b (tocsym, lab));
 	  emit_move_insn (dest, gen_rtx_REG (Pmode, LR_REGNO));
+	  if (TARGET_LINK_STACK)
+	    emit_insn (gen_addsi3 (dest, dest, GEN_INT (4)));
 	  emit_move_insn (temp0, gen_rtx_MEM (Pmode, dest));
 	}
       emit_insn (gen_addsi3 (dest, temp0, dest));
@@ -22493,7 +22502,11 @@  output_function_profiler (FILE *file, in
 	}
       else if (TARGET_SECURE_PLT && flag_pic)
 	{
-	  asm_fprintf (file, "\tbcl 20,31,1f\n1:\n\t{st|stw} %s,4(%s)\n",
+	  if (TARGET_LINK_STACK)
+	    asm_fprintf (file, "\tbl 1f\n\tb 2f\n1:\n\tblr\n2:\n");
+	  else
+	    asm_fprintf (file, "\tbcl 20,31,1f\n1:\n");
+	  asm_fprintf (file, "\t{st|stw} %s,4(%s)\n",
 		       reg_names[0], reg_names[1]);
 	  asm_fprintf (file, "\tmflr %s\n", reg_names[12]);
 	  asm_fprintf (file, "\t{cau|addis} %s,%s,",
@@ -22518,10 +22531,22 @@  output_function_profiler (FILE *file, in
 	  asm_fprintf (file, "\t{st|stw} %s,4(%s)\n",
 		       reg_names[0], reg_names[1]);
 	  /* Now, we need to get the address of the label.  */
-	  fputs ("\tbcl 20,31,1f\n\t.long ", file);
-	  assemble_name (file, buf);
-	  fputs ("-.\n1:", file);
-	  asm_fprintf (file, "\tmflr %s\n", reg_names[11]);
+	  if (TARGET_LINK_STACK)
+	    {
+	      fputs ("\tbl 1f\n\tb 2f\n\t.long ", file);
+	      assemble_name (file, buf);
+	      fputs ("-.\n1:\n\tblr\n2:", file);
+	      asm_fprintf (file, "\tmflr %s\n", reg_names[11]);
+	      asm_fprintf (file, "\taddi %s,%s,4\n",
+			   reg_names[11], reg_names[11]);
+	    }
+	  else
+	    {
+	      fputs ("\tbcl 20,31,1f\n\t.long ", file);
+	      assemble_name (file, buf);
+	      fputs ("-.\n1:", file);
+	      asm_fprintf (file, "\tmflr %s\n", reg_names[11]);
+	    }
 	  asm_fprintf (file, "\t{l|lwz} %s,0(%s)\n",
 		       reg_names[0], reg_names[11]);
 	  asm_fprintf (file, "\t{cax|add} %s,%s,%s\n",
@@ -25004,11 +25029,22 @@  macho_branch_islands (void)
 #endif /* DBX_DEBUGGING_INFO || XCOFF_DEBUGGING_INFO */
       if (flag_pic)
 	{
-	  strcat (tmp_buf, ":\n\tmflr r0\n\tbcl 20,31,");
-	  strcat (tmp_buf, label);
-	  strcat (tmp_buf, "_pic\n");
-	  strcat (tmp_buf, label);
-	  strcat (tmp_buf, "_pic:\n\tmflr r11\n");
+	  if (TARGET_LINK_STACK)
+	    {
+	      strcat (tmp_buf, ":\n\tmflr r0\n\tbl $+8\n");
+	      strcat (tmp_buf, label);
+	      strcat (tmp_buf, "_pic:\n\tb $+8\n");
+	      strcat (tmp_buf, "\tblr\n");
+	      strcat (tmp_buf, "\tmflr r11\n");
+	    }
+	  else
+	    {
+	      strcat (tmp_buf, ":\n\tmflr r0\n\tbcl 20,31,");
+	      strcat (tmp_buf, label);
+	      strcat (tmp_buf, "_pic\n");
+	      strcat (tmp_buf, label);
+	      strcat (tmp_buf, "_pic:\n\tmflr r11\n");
+	    }
 
 	  strcat (tmp_buf, "\taddis r11,r11,ha16(");
 	  strcat (tmp_buf, name_buf);
@@ -25154,8 +25190,16 @@  machopic_output_stub (FILE *file, const
       sprintf (local_label_0, "\"L%011d$spb\"", label);
 
       fprintf (file, "\tmflr r0\n");
-      fprintf (file, "\tbcl 20,31,%s\n", local_label_0);
-      fprintf (file, "%s:\n\tmflr r11\n", local_label_0);
+      if (TARGET_LINK_STACK)
+	{
+	  fprintf (file, "\tbl $+8\n");
+	  fprintf (file, "%s:\n\tb $+8\n\tblr\n\tmflr r11\n", local_label_0);
+	}
+      else
+	{
+	  fprintf (file, "\tbcl 20,31,%s\n", local_label_0);
+	  fprintf (file, "%s:\n\tmflr r11\n", local_label_0);
+	}
       fprintf (file, "\taddis r11,r11,ha16(%s-%s)\n",
 	       lazy_ptr_name, local_label_0);
       fprintf (file, "\tmtlr r0\n");
Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 179091)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -313,6 +313,14 @@  extern const char *host_detect_local_cpu
 #define HAVE_AS_TLS 0
 #endif
 
+#ifndef TARGET_LINK_STACK
+#define TARGET_LINK_STACK 0
+#endif
+
+#ifndef SET_TARGET_LINK_STACK
+#define SET_TARGET_LINK_STACK(X) do { } while (0)
+#endif
+
 /* Return 1 for a symbol ref for a thread-local storage symbol.  */
 #define RS6000_SYMBOL_REF_TLS_P(RTX) \
   (GET_CODE (RTX) == SYMBOL_REF && SYMBOL_REF_TLS_MODEL (RTX) != 0)
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 179091)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -12081,27 +12081,65 @@  (define_insn "load_toc_v4_pic_si"
   [(set_attr "type" "branch")
    (set_attr "length" "4")])
 
-(define_insn "load_toc_v4_PIC_1"
+(define_expand "load_toc_v4_PIC_1"
+  [(parallel [(set (reg:SI LR_REGNO)
+		   (match_operand:SI 0 "immediate_operand" "s"))
+	      (use (unspec [(match_dup 0)] UNSPEC_TOC))])]
+  "TARGET_ELF && DEFAULT_ABI != ABI_AIX
+   && (flag_pic == 2 || (flag_pic && TARGET_SECURE_PLT))"
+  "")
+
+(define_insn "load_toc_v4_PIC_1_normal"
   [(set (reg:SI LR_REGNO)
 	(match_operand:SI 0 "immediate_operand" "s"))
    (use (unspec [(match_dup 0)] UNSPEC_TOC))]
-  "TARGET_ELF && DEFAULT_ABI != ABI_AIX
+  "!TARGET_LINK_STACK && TARGET_ELF && DEFAULT_ABI != ABI_AIX
    && (flag_pic == 2 || (flag_pic && TARGET_SECURE_PLT))"
   "bcl 20,31,%0\\n%0:"
   [(set_attr "type" "branch")
    (set_attr "length" "4")])
 
-(define_insn "load_toc_v4_PIC_1b"
+(define_insn "load_toc_v4_PIC_1_476"
+  [(set (reg:SI LR_REGNO)
+	(match_operand:SI 0 "immediate_operand" "s"))
+   (use (unspec [(match_dup 0)] UNSPEC_TOC))]
+  "TARGET_LINK_STACK && TARGET_ELF && DEFAULT_ABI != ABI_AIX
+   && (flag_pic == 2 || (flag_pic && TARGET_SECURE_PLT))"
+  "bl $+8\n%0:\n\tb $+8\n\tblr"
+  [(set_attr "type" "branch")
+   (set_attr "length" "12")])
+
+(define_expand "load_toc_v4_PIC_1b"
+  [(parallel [(set (reg:SI LR_REGNO)
+		   (unspec:SI [(match_operand:SI 0 "immediate_operand" "s")
+			       (label_ref (match_operand 1 "" ""))]
+		           UNSPEC_TOCPTR))
+	      (match_dup 1)])]
+  "TARGET_ELF && DEFAULT_ABI != ABI_AIX && flag_pic == 2"
+  "")
+
+(define_insn "load_toc_v4_PIC_1b_normal"
   [(set (reg:SI LR_REGNO)
 	(unspec:SI [(match_operand:SI 0 "immediate_operand" "s")
 		    (label_ref (match_operand 1 "" ""))]
 		UNSPEC_TOCPTR))
    (match_dup 1)]
-  "TARGET_ELF && DEFAULT_ABI != ABI_AIX && flag_pic == 2"
+  "!TARGET_LINK_STACK && TARGET_ELF && DEFAULT_ABI != ABI_AIX && flag_pic == 2"
   "bcl 20,31,$+8\;.long %0-$"
   [(set_attr "type" "branch")
    (set_attr "length" "8")])
 
+(define_insn "load_toc_v4_PIC_1b_476"
+  [(set (reg:SI LR_REGNO)
+	(unspec:SI [(match_operand:SI 0 "immediate_operand" "s")
+		    (label_ref (match_operand 1 "" ""))]
+		UNSPEC_TOCPTR))
+   (match_dup 1)]
+  "TARGET_LINK_STACK && TARGET_ELF && DEFAULT_ABI != ABI_AIX && flag_pic == 2"
+  "bl $+12\n\tb $+12\n\t.long %0-$\n\tblr"
+  [(set_attr "type" "branch")
+   (set_attr "length" "16")])
+
 (define_insn "load_toc_v4_PIC_2"
   [(set (match_operand:SI 0 "gpc_reg_operand" "=r")
 	(mem:SI (plus:SI (match_operand:SI 1 "gpc_reg_operand" "b")