This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[mips] fix $gp restore bug


This patch fixes a problem with the interaction of the long-branch compensation
code and function calls clobbering $gp in O32 and O64 ABIs.

If a branch immediately follows a function call, we generate the following:
	jalr  $reg  ; will clobber $gp
	nop ; delay slot
	branch far_label
	lw  $gp,$sp(slot)  ; restore gp in branch delay

But, if that branch is too far for a regular unconditional branch, for PIC O32 &
O64 ABIs it gets expanded to:
	lw  $at,%got(far_label)($gp)
	addiu $at,$at,%lo(far_label)
	jr  $at

Notice that this has the property of retaining the orignal branch's delay slot
at the end.

But of course, inserting that expansion into the previous sequence gives:
	jalr  $reg
	nop ; delay slot
	lw  $at,%got(far_label)($gp)
	addiu $at,$at,%lo(far_label)
	jr  $at
	lw  $gp,$sp(slot)  ; restore gp in branch delay

and hey presto, we've a %got load with a random $gp value  :(

We cannot describe this restriction to the delay slot filler -- (a) there's no
mechanism to describe such conditional register usage, and (b) we don't have
accurate branch distances at that time anyway -- hazard insertion can lengthen
things.

This patch changes the unconditional branch assembly emitter to check whether
the delay slot is occupied by a $gp restoring instruction.  If it is, it emits
the instruction before the long-branch sequence.  We also emit it in the long
branch sequence as well -- I didn't feel the need to optimize that.

tested with mips-linux-gnu, ok?

nathan
-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery

2009-02-02  Nathan Sidwell  <nathan@codesourcery.com>

	* config/mips/mips.md (jump): Deal with $gp restoration in delay
	slot for o32 and o64 ABIs.

	testsuite/
	* gcc.target/mips/branch-2.c: New.

Index: config/mips/mips.md
===================================================================
--- config/mips/mips.md	(revision 143759)
+++ config/mips/mips.md	(working copy)
@@ -5470,6 +5470,26 @@
 	return "%*b\t%l0%/";
       else
 	{
+	  if (final_sequence && (mips_abi == ABI_32 || mips_abi == ABI_O64))
+	    {
+              /* If the delay slot contains a $gp restore, we need to
+                 do that first, because we need it for the load
+		 label.  Other ABIs do not have caller-save $gp.  */
+	      rtx next = NEXT_INSN (insn);
+	      if (INSN_P (next) && !INSN_DELETED_P (next))
+		{
+		  rtx pat = PATTERN (next);
+		  if (GET_CODE (pat) == SET
+		      && REG_P (SET_DEST (pat))
+		      && REGNO (SET_DEST (pat)) == PIC_OFFSET_TABLE_REGNUM)
+		    {
+		      rtx ops[2];
+		      ops[0] = SET_DEST (pat);
+		      ops[1] = SET_SRC (pat);
+		      output_asm_insn (mips_output_move (ops[0], ops[1]), ops);
+		    }
+		}
+	    }
 	  output_asm_insn (mips_output_load_label (), operands);
 	  return "%*jr\t%@%/%]";
 	}
@@ -5488,7 +5508,13 @@
 	      (lt (abs (minus (match_dup 0)
 			      (plus (pc) (const_int 4))))
 		  (const_int 131072)))
-	 (const_int 4) (const_int 16)))])
+	 (const_int 4)
+         (if_then_else
+	    ;; for these two ABIs we may need to move a restore of $gp
+	    (ior (eq (symbol_ref "mips_abi") (symbol_ref "ABI_32"))
+		 (eq (symbol_ref "mips_abi") (symbol_ref "ABI_O64")))
+	    (const_int 20)
+	    (const_int 16))))])
 
 ;; We need a different insn for the mips16, because a mips16 branch
 ;; does not have a delay slot.
Index: testsuite/gcc.target/mips/branch-2.c
===================================================================
--- testsuite/gcc.target/mips/branch-2.c	(revision 0)
+++ testsuite/gcc.target/mips/branch-2.c	(revision 0)
@@ -0,0 +1,44 @@
+/* Check that we correctly expand out-of-range branches */
+/* { dg-do run } */
+/* { dg-options "-O2 -mabi=32 -fPIC" } */
+
+#include <stdlib.h>
+
+/* This is weak so the compiler cannot assume that calls from this TU
+   necessarily arrive here.   And hence that $gp may be clobbered in
+   o32 and o64 ABIs.  */
+
+void __attribute__ ((weak)) Foo (int i)
+{
+  static int once = 0;
+
+  if (!i && once++)
+    exit (0);
+
+#if (_ABIO32 || _ABIO64)
+  /* Clobber $gp */
+  __asm volatile ("li $gp,0");
+#endif
+}
+
+#define N1(X)  (Foo (X))
+#define N2(X)  (N1 (X), N1 (X+(1<<0)))
+#define N3(X)  (N2 (X), N2 (X+(1<<1)))
+#define N4(X)  (N3 (X), N3 (X+(1<<2)))
+#define N5(X)  (N4 (X), N4 (X+(1<<3)))
+#define N6(X)  (N5 (X), N5 (X+(1<<4)))
+#define N7(X)  (N6 (X), N6 (X+(1<<5)))
+#define N8(X)  (N7 (X), N7 (X+(1<<6)))
+#define N9(X)  (N8 (X), N8 (X+(1<<7)))
+#define N10(X)  (N9 (X), N9 (X+(1<<8)))
+#define N11(X)  (N10 (X), N10 (X+(1<<9)))
+#define N12(X)  (N11 (X), N11 (X+(1<<10)))
+#define N13(X)  (N12 (X), N12 (X+(1<<11)))
+#define N14(X)  (N13 (X), N13 (X+(1<<12)))
+
+int main (void)
+{
+  while (1)
+    N14 (0);
+  return 0;
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]