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]

back out dwarf2out_stack_adjust change


Mark would prefer that I just back out Alexandre's change.  I've sent
test results as

<http://gcc.gnu.org/ml/gcc-testresults/2006-09/msg00082.html> (before)
<http://gcc.gnu.org/ml/gcc-testresults/2006-09/msg00043.html> (after)

While preparing this change, I found out that although the C and C++
frontends never emit nested function calls that can throw, the Java
frontend can call library routines which can throw.
Fortunately, the Java frontend uses asynchronous unwind tables and so
removing Alexandre's change does not affect it.

Here's what you'd need to do to make this routine really work:
could put Alexandre's change back in 

1. Have it integrated with the rest of the DWARF code that computes the
   CFA.
2. Have it use the control flow graph, and propagate the argument size
   along CFA edges.
3. Remove the code that does
       if (args_size < 0)
         args_size = 0;
   and in general, never change args_size except to reflect a SET 
   (or post-increment or similar) of the stack pointer.
4. Add gcc_assert checking to ensure that:
   a. The argument size is never computed to be negative.
   b. The argument size computed at each call instruction is at
      least as large as the arguments for that call instruction.
   c. The computed argument size plus the CFA is a multiple of the
      stack boundary at every call instruction.
   d. Every instruction, without exception, which modifies (SET or
      similar) the stack pointer is understood by this code and
      reflected in a change to args_size or to the CFA.
   e. At a return instruction, the argument size is 0.

If all this was done, Alexandre's change should go back in, the code
in i386.c which says
  warning (0, "unwind tables currently require either a frame pointer "
	   "or -maccumulate-outgoing-args for correctness");
could be removed, and many other things would work better.

-- 
- Geoffrey Keating <geoffk@apple.com>

===File ~/patches/gcc-stackadjust-2.patch===================
2006-08-31  Geoffrey Keating  <geoffk@apple.com>

	Revert this change:
	2006-03-17  Alexandre Oliva  <aoliva@redhat.com>
	* dwarf2out.c (dwarf2out_stack_adjust): Always track the stack
	pointer, instead of assuming it is possible to derive the
	correct args size from a call insn.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 116606)
+++ gcc/dwarf2out.c	(working copy)
@@ -1086,7 +1086,7 @@
    much extra space it needs to pop off the stack.  */
 
 static void
-dwarf2out_stack_adjust (rtx insn, bool after_p ATTRIBUTE_UNUSED)
+dwarf2out_stack_adjust (rtx insn, bool after_p)
 {
   HOST_WIDE_INT offset;
   const char *label;
@@ -1099,8 +1099,32 @@
   if (prologue_epilogue_contains (insn) || sibcall_epilogue_contains (insn))
     return;
 
-  if (BARRIER_P (insn))
+  /* If only calls can throw, and we have a frame pointer,
+     save up adjustments until we see the CALL_INSN.  */
+  if (!flag_asynchronous_unwind_tables && cfa.reg != STACK_POINTER_REGNUM)
     {
+      if (CALL_P (insn) && !after_p)
+	{
+	  /* Extract the size of the args from the CALL rtx itself.  */
+	  insn = PATTERN (insn);
+	  if (GET_CODE (insn) == PARALLEL)
+	    insn = XVECEXP (insn, 0, 0);
+	  if (GET_CODE (insn) == SET)
+	    insn = SET_SRC (insn);
+	  gcc_assert (GET_CODE (insn) == CALL);
+	  dwarf2out_args_size ("", INTVAL (XEXP (insn, 1)));
+	}
+      return;
+    }
+
+  if (CALL_P (insn) && !after_p)
+    {
+      if (!flag_asynchronous_unwind_tables)
+	dwarf2out_args_size ("", args_size);
+      return;
+    }
+  else if (BARRIER_P (insn))
+    {
       /* When we see a BARRIER, we know to reset args_size to 0.  Usually
 	 the compiler will have already emitted a stack adjustment, but
 	 doesn't bother for calls to noreturn functions.  */
@@ -1121,20 +1145,9 @@
 	if (GET_CODE (XVECEXP (PATTERN (insn), 0, i)) == SET)
 	  offset += stack_adjust_offset (XVECEXP (PATTERN (insn), 0, i));
     }
-  else if (GET_CODE (insn) == CALL_INSN)
-    offset = 0;
   else
     return;
 
-  /* We handle this separately because we want stack adjustments in a
-     CALL_INSN to be handled.  */;
-  if (GET_CODE (insn) == CALL_INSN)
-    {
-      /* If only calls can throw, adjust args_size only at call sites.  */
-      if (!flag_asynchronous_unwind_tables)
-	dwarf2out_args_size ("", args_size);
-    }
-
   if (offset == 0)
     return;
 
@@ -1149,16 +1162,6 @@
   if (args_size < 0)
     args_size = 0;
 
-  /* If only calls can throw and we have a frame pointer, we'll save
-     up adjustments until we see the CALL_INSN.  We used to return
-     early and derive args_size from NARGS in the CALL_INSN itself,
-     but that doesn't compute the right value if we have nested call
-     expansions, e.g., stack adjustments for a call have already been
-     emitted, and then we issue another call to compute an argument
-     for the enclosing call (i.e., bar (foo ())).  */
-  if (!flag_asynchronous_unwind_tables && cfa.reg != STACK_POINTER_REGNUM)
-    return;
-
   label = dwarf2out_cfi_label ();
   def_cfa_1 (label, &cfa);
   if (flag_asynchronous_unwind_tables)
============================================================


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