Patchwork [RFC] Fix unwind info for sparc -mflat

login
register
mail settings
Submitter Richard Henderson
Date June 21, 2011, 12:33 a.m.
Message ID <4DFFE6DE.9050207@twiddle.net>
Download mbox | patch
Permalink /patch/101226/
State New
Headers show

Comments

Richard Henderson - June 21, 2011, 12:33 a.m.
The current code generation for -mflat uses 3 insn patterns
which emit up two three insns (sort of) emulating the save
instruction.

The problem is that the unwind info is only produced at the
end of any pattern, leaving a 1 or 2 insn hole for which the
unwind info is not correct.

I tried to figure out why things had been done in this
slightly convoluted manner and failed.  It seems to me that
this is easily represented with the individual instructions.
A comment indicated that there had been problems with the
copy to %o7 being deleted.  Elsewhere we have successfully
used a naked USE pattern to keep such things from being
deleted.

Unfortunately, I can't seem to get -mflat to bootstrap on
gcc62 (as sparc-linux, not sparc64) either before or after
this patch, so I'm not sure what sort of pre-requisite I'm
missing in order to be able to test it...

Comments?


r~
commit 7c9f3f4fa56b6603007d254afad18c47009bc5af
Author: Richard Henderson <rth@twiddle.net>
Date:   Mon Jun 20 16:42:14 2011 -0700

    sparc: Fix -mflat unwind info.
    
    The old definition left a 2 instruction hole in which
    unwind info was out-of-date.
Richard Henderson - June 21, 2011, 12:39 a.m.
On 06/20/2011 05:33 PM, Richard Henderson wrote:
> The current code generation for -mflat uses 3 insn patterns
> which emit up two three insns (sort of) emulating the save

Lest I get sent back for remedial English, that was supposed
to be "emit two or three" but fingers got ahead of brain.


r~
Eric Botcazou - June 21, 2011, 7:27 a.m.
> I tried to figure out why things had been done in this
> slightly convoluted manner and failed.  It seems to me that
> this is easily represented with the individual instructions.

I wanted to avoid the back-and-forth game on the CFA offset and emit the same 
CFIs as in the normal case.  Your solution is probably better in the end, but 
please add a comment in sparc_flat_expand_prologue explaining why we play this 
game to establish the frame.

> A comment indicated that there had been problems with the
> copy to %o7 being deleted.  Elsewhere we have successfully
> used a naked USE pattern to keep such things from being
> deleted.

Fine, thanks for cleaning up the whole thing.

> Unfortunately, I can't seem to get -mflat to bootstrap on
> gcc62 (as sparc-linux, not sparc64) either before or after
> this patch, so I'm not sure what sort of pre-requisite I'm
> missing in order to be able to test it...

Just run the C testsuite, bi-arch preferably, with -mflat; it should be clean.
It contains some unwinding tests.  You can also run the C++ testsuite, but 
there are about 20 (known) failures without -mflat multilibs.

I didn't try a bootstrap, but it probably doesn't work because the models are 
fully intermixable only if you insert flushw instructions at the boundaries.

Patch

diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c
index e50d2f1..7d83dd6 100644
--- a/gcc/config/sparc/sparc.c
+++ b/gcc/config/sparc/sparc.c
@@ -4617,39 +4617,6 @@  emit_save_register_window (rtx increment)
   return insn;
 }
 
-/* Generate a create_flat_frame_1 insn.  */
-
-static rtx
-gen_create_flat_frame_1 (rtx increment)
-{
-  if (TARGET_ARCH64)
-    return gen_create_flat_frame_1di (increment);
-  else
-    return gen_create_flat_frame_1si (increment);
-}
-
-/* Generate a create_flat_frame_2 insn.  */
-
-static rtx
-gen_create_flat_frame_2 (rtx increment)
-{
-  if (TARGET_ARCH64)
-    return gen_create_flat_frame_2di (increment);
-  else
-    return gen_create_flat_frame_2si (increment);
-}
-
-/* Generate a create_flat_frame_3 insn.  */
-
-static rtx
-gen_create_flat_frame_3 (rtx increment)
-{
-  if (TARGET_ARCH64)
-    return gen_create_flat_frame_3di (increment);
-  else
-    return gen_create_flat_frame_3si (increment);
-}
-
 /* Generate an increment for the stack pointer.  */
 
 static rtx
@@ -4793,7 +4760,6 @@  sparc_flat_expand_prologue (void)
 {
   HOST_WIDE_INT size;
   rtx insn;
-  int i;
 
   sparc_leaf_function_p = optimize > 0 && current_function_is_leaf;
 
@@ -4811,103 +4777,64 @@  sparc_flat_expand_prologue (void)
 
   if (size == 0)
     ; /* do nothing.  */
-  else if (frame_pointer_needed)
+  else
     {
-      if (size <= 4096)
-	{
-	  if (return_addr_reg_needed_p (sparc_leaf_function_p))
-	    insn = emit_insn (gen_create_flat_frame_1 (GEN_INT (-size)));
-	  else
-	    insn = emit_insn (gen_create_flat_frame_2 (GEN_INT (-size)));
-	  RTX_FRAME_RELATED_P (insn) = 1;
-	  for (i=0; i < XVECLEN (PATTERN (insn), 0); i++)
-	    RTX_FRAME_RELATED_P (XVECEXP (PATTERN (insn), 0, i)) = 1;
-	}
-      else
-	{
-	  rtx reg = gen_rtx_REG (Pmode, 1), note;
-	  emit_move_insn (reg, GEN_INT (-size));
-	  if (return_addr_reg_needed_p (sparc_leaf_function_p))
-	    {
-	      insn = emit_insn (gen_create_flat_frame_1 (reg));
-	      note
-		= gen_rtx_PARALLEL (VOIDmode,
-				    gen_rtvec
-				    (3, copy_rtx
-					(XVECEXP (PATTERN (insn), 0, 0)),
-					gen_stack_pointer_inc
-					(GEN_INT (-size)),
-					copy_rtx
-					(XVECEXP (PATTERN (insn), 0, 2))));
-	    }
-	  else
-	    {
-	      insn = emit_insn (gen_create_flat_frame_2 (reg));
-	      note
-		= gen_rtx_PARALLEL (VOIDmode,
-				    gen_rtvec
-				    (2, copy_rtx
-					(XVECEXP (PATTERN (insn), 0, 0)),
-					gen_stack_pointer_inc
-					(GEN_INT (-size))));
-	    }
+      rtx size_int_rtx, size_rtx;
+
+      size_rtx = size_int_rtx = GEN_INT (-size);
 
-	  RTX_FRAME_RELATED_P (insn) = 1;
-	  add_reg_note (insn, REG_FRAME_RELATED_EXPR, note);
-	  for (i=0; i < XVECLEN (note, 0); i++)
-	    RTX_FRAME_RELATED_P (XVECEXP (note, 0, i)) = 1;
-	}
-    }
-  else if (return_addr_reg_needed_p (sparc_leaf_function_p))
-    {
       if (size <= 4096)
+	insn = emit_insn (gen_stack_pointer_inc (size_int_rtx));
+      else if (size <= 8192 && !frame_pointer_needed)
 	{
-	  insn = emit_insn (gen_create_flat_frame_3 (GEN_INT (-size)));
+	  insn = emit_insn (gen_stack_pointer_inc (GEN_INT (-4096)));
 	  RTX_FRAME_RELATED_P (insn) = 1;
-	  for (i=0; i < XVECLEN (PATTERN (insn), 0); i++)
-	    RTX_FRAME_RELATED_P (XVECEXP (PATTERN (insn), 0, i)) = 1;
+	  insn = emit_insn (gen_stack_pointer_inc (GEN_INT (4096 - size)));
 	}
       else
 	{
-	  rtx reg = gen_rtx_REG (Pmode, 1), note;
-	  emit_move_insn (reg, GEN_INT (-size));
-	  insn = emit_insn (gen_create_flat_frame_3 (reg));
-	  note
-	    = gen_rtx_PARALLEL (VOIDmode,
-				gen_rtvec
-				(2, gen_stack_pointer_inc (GEN_INT (-size)),
-				    copy_rtx
-				    (XVECEXP (PATTERN (insn), 0, 1))));
-	  RTX_FRAME_RELATED_P (insn) = 1;
-	  add_reg_note (insn, REG_FRAME_RELATED_EXPR, note);
-	  for (i=0; i < XVECLEN (note, 0); i++)
-	    RTX_FRAME_RELATED_P (XVECEXP (note, 0, i)) = 1;
+	  size_rtx = gen_rtx_REG (Pmode, 1);
+	  emit_move_insn (size_rtx, size_int_rtx);
+	  insn = emit_insn (gen_stack_pointer_inc (size_rtx));
+	  add_reg_note (insn, REG_CFA_ADJUST_CFA,
+			gen_stack_pointer_inc (size_int_rtx));
 	}
-    }
-  else
-    {
-      if (size <= 4096)
-	insn = emit_insn (gen_stack_pointer_inc (GEN_INT (-size)));
-      else if (size <= 8192)
+      RTX_FRAME_RELATED_P (insn) = 1;
+
+      if (frame_pointer_needed)
 	{
-	  insn = emit_insn (gen_stack_pointer_inc (GEN_INT (-4096)));
+	  insn = emit_insn (gen_rtx_SET (VOIDmode, hard_frame_pointer_rtx,
+					 gen_rtx_MINUS (Pmode,
+							stack_pointer_rtx,
+							size_rtx)));
 	  RTX_FRAME_RELATED_P (insn) = 1;
-	  insn = emit_insn (gen_stack_pointer_inc (GEN_INT (4096 - size)));
+
+	  add_reg_note (insn, REG_CFA_ADJUST_CFA,
+			gen_rtx_SET (VOIDmode, hard_frame_pointer_rtx,
+				     plus_constant (stack_pointer_rtx,
+						    size)));
+
+	  /* Make sure nothing is scheduled until after the frame
+	     is established.  */
+	  emit_insn (gen_blockage ());
 	}
-      else
+
+      if (return_addr_reg_needed_p (sparc_leaf_function_p))
 	{
-	  rtx reg = gen_rtx_REG (Pmode, 1);
-	  emit_move_insn (reg, GEN_INT (-size));
-	  insn = emit_insn (gen_stack_pointer_inc (reg));
-	  add_reg_note (insn, REG_FRAME_RELATED_EXPR,
-			gen_stack_pointer_inc (GEN_INT (-size)));
-	}
+	  rtx i7 = gen_rtx_REG (Pmode, INCOMING_RETURN_ADDR_REGNUM);
+	  rtx o7 = gen_rtx_REG (Pmode, RETURN_ADDR_REGNUM);
 
-      RTX_FRAME_RELATED_P (insn) = 1;
-    }
+	  insn = emit_move_insn (o7, i7);
+	  RTX_FRAME_RELATED_P (insn) = 1;
 
-  /* Make sure nothing is scheduled until after the frame is established.  */
-  emit_insn (gen_blockage ());
+	  add_reg_note (insn, REG_CFA_REGISTER,
+			gen_rtx_SET (VOIDmode, o7, i7));
+
+	  /* Prevent this instruction from ever being considered dead,
+	     even if this function has no epilogue.  */
+	  emit_insn (gen_rtx_USE (VOIDmode, o7));
+	}
+    }
 
   if (frame_pointer_needed)
     {
diff --git a/gcc/config/sparc/sparc.md b/gcc/config/sparc/sparc.md
index 017b689..2c8d306 100644
--- a/gcc/config/sparc/sparc.md
+++ b/gcc/config/sparc/sparc.md
@@ -6287,39 +6287,6 @@ 
   "save\t%%sp, %0, %%sp"
   [(set_attr "type" "savew")])
 
-;; For the "create flat frame" insns, we need to use special insns
-;; because %fp cannot be clobbered until after the frame is established (so
-;; that it contains the live register window save area) and %i7 changed with
-;; a simple move as it is a fixed register and the move would be eliminated.
-
-(define_insn "create_flat_frame_1<P:mode>"
-  [(set (reg:P 30) (reg:P 14))
-   (set (reg:P 14) (plus:P (reg:P 14)
-			   (match_operand:P 0 "arith_operand" "rI")))
-   (set (reg:P 31) (reg:P 15))]
-  "TARGET_FLAT"
-  "add\t%%sp, %0, %%sp\n\tsub\t%%sp, %0, %%fp\n\tmov\t%%o7, %%i7"
-  [(set_attr "type" "multi")
-   (set_attr "length" "3")])
-
-(define_insn "create_flat_frame_2<P:mode>"
-  [(set (reg:P 30) (reg:P 14))
-   (set (reg:P 14) (plus:P (reg:P 14)
-		           (match_operand:P 0 "arith_operand" "rI")))]
-  "TARGET_FLAT"
-  "add\t%%sp, %0, %%sp\n\tsub\t%%sp, %0, %%fp"
-  [(set_attr "type" "multi")
-   (set_attr "length" "2")])
-
-(define_insn "create_flat_frame_3<P:mode>"
-  [(set (reg:P 14) (plus:P (reg:P 14)
-		           (match_operand:P 0 "arith_operand" "rI")))
-   (set (reg:P 31) (reg:P 15))]
-  "TARGET_FLAT"
-  "add\t%%sp, %0, %%sp\n\tmov\t%%o7, %%i7"
-  [(set_attr "type" "multi")
-   (set_attr "length" "2")])
-
 (define_expand "epilogue"
   [(return)]
   ""