Patchwork Fix !EXIT_IGNORE_STACK vs over-aligned variables

login
register
mail settings
Submitter Richard Henderson
Date March 3, 2011, 10:18 p.m.
Message ID <4D7013B1.8050100@redhat.com>
Download mbox | patch
Permalink /patch/85349/
State New
Headers show

Comments

Richard Henderson - March 3, 2011, 10:18 p.m.
DJ reported this yesterday on m32c, but this would affect all such
targets: mcore, m32c, arc, h8300, m68hc11.

In my opinion, we ought to work with these targets to add the few
insns to their epilogue to handle alloca, rather than keep these
very infrequently used code paths.  Material for 4.7, for sure.

In the meantime, I need to get the emit_stack_save code emitted
at the right place at the start of the function.  The problem was
that cfgexpand.c was expecting to emit the sequence *before* the
mark, whereas function.c was actually emitting the sequence after
the mark.

I tried rearranging cfgexpand such that the mark was set so that
we could continue to insert afterward, but this resulted in an ICE
due to insertting the sequence outside any basic block.  In such
cases, we really don't have a viable location against which we 
can set the mark.

Then I changed things such that we insert before the mark.  This
worked.  If we're not doing over-alignement, this means that we
insert code before the FUNCTION_BEG note rather than after.  I 
think this is ok -- the save is logically part of the prologue.

At which point I needed to audit the other users of emit_stack_save
to figure out if the change to insert before is ok.  It turns out
that the one use in function.c is the only user that provided a 
non-null 'after' parameter.  Further, *no* user of emit_stack_restore
provided a non-null 'after' parameter.

So I've removed the after parameter from both emit_stack_save and
emit_stack_restore, adjusted all users, and changed the use in
function.c to do the sequence manipulation itself.

Tested on m32c-elf and x86_64-linux.  Committed.


r~
* explow.c (emit_stack_save): Remove 'after' parameter.
        (emit_stack_restore): Likewise.
        * expr.h: Update to match.
        * builtins.c, calls.c, stmt.c: Likewise.
        * config/alpha/alpha.md, config/avr/avr.md: Likewise.
        * config/mips/mips.md, config/pa/pa.md, config/vax/vax.md: Likewise.
        * function.c (expand_function_end): Insert the emit_stack_save
        sequence before parm_birth_insn instead of after.

Patch

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 106b2ca..3361264 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -784,7 +784,7 @@  expand_builtin_setjmp_setup (rtx buf_addr, rtx receiver_label)
 			    plus_constant (buf_addr,
 					   2 * GET_MODE_SIZE (Pmode)));
   set_mem_alias_set (stack_save, setjmp_alias_set);
-  emit_stack_save (SAVE_NONLOCAL, &stack_save, NULL_RTX);
+  emit_stack_save (SAVE_NONLOCAL, &stack_save);
 
   /* If there is further processing to do, do it.  */
 #ifdef HAVE_builtin_setjmp_setup
@@ -932,7 +932,7 @@  expand_builtin_longjmp (rtx buf_addr, rtx value)
 	  emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx));
 
 	  emit_move_insn (hard_frame_pointer_rtx, fp);
-	  emit_stack_restore (SAVE_NONLOCAL, stack, NULL_RTX);
+	  emit_stack_restore (SAVE_NONLOCAL, stack);
 
 	  emit_use (hard_frame_pointer_rtx);
 	  emit_use (stack_pointer_rtx);
@@ -1005,7 +1005,7 @@  expand_builtin_nonlocal_goto (tree exp)
 	 The non-local goto handler will then adjust it to contain the
 	 proper value and reload the argument pointer, if needed.  */
       emit_move_insn (hard_frame_pointer_rtx, r_fp);
-      emit_stack_restore (SAVE_NONLOCAL, r_sp, NULL_RTX);
+      emit_stack_restore (SAVE_NONLOCAL, r_sp);
 
       /* USE of hard_frame_pointer_rtx added for consistency;
 	 not clear if really needed.  */
@@ -1075,7 +1075,7 @@  expand_builtin_update_setjmp_buf (rtx buf_addr)
     emit_insn (gen_setjmp ());
 #endif
 
-  emit_stack_save (SAVE_NONLOCAL, &stack_save, NULL_RTX);
+  emit_stack_save (SAVE_NONLOCAL, &stack_save);
 }
 
 /* Expand a call to __builtin_prefetch.  For a target that does not support
@@ -1558,10 +1558,10 @@  expand_builtin_apply (rtx function, rtx arguments, rtx argsize)
   /* Save the stack with nonlocal if available.  */
 #ifdef HAVE_save_stack_nonlocal
   if (HAVE_save_stack_nonlocal)
-    emit_stack_save (SAVE_NONLOCAL, &old_stack_level, NULL_RTX);
+    emit_stack_save (SAVE_NONLOCAL, &old_stack_level);
   else
 #endif
-    emit_stack_save (SAVE_BLOCK, &old_stack_level, NULL_RTX);
+    emit_stack_save (SAVE_BLOCK, &old_stack_level);
 
   /* Allocate a block of memory onto the stack and copy the memory
      arguments to the outgoing arguments address.  We can pass TRUE
@@ -1677,10 +1677,10 @@  expand_builtin_apply (rtx function, rtx arguments, rtx argsize)
   /* Restore the stack.  */
 #ifdef HAVE_save_stack_nonlocal
   if (HAVE_save_stack_nonlocal)
-    emit_stack_restore (SAVE_NONLOCAL, old_stack_level, NULL_RTX);
+    emit_stack_restore (SAVE_NONLOCAL, old_stack_level);
   else
 #endif
-    emit_stack_restore (SAVE_BLOCK, old_stack_level, NULL_RTX);
+    emit_stack_restore (SAVE_BLOCK, old_stack_level);
 
   OK_DEFER_POP;
 
diff --git a/gcc/calls.c b/gcc/calls.c
index 5297763..f539f66 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1092,7 +1092,7 @@  initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
 
 		  if (*old_stack_level == 0)
 		    {
-		      emit_stack_save (SAVE_BLOCK, old_stack_level, NULL_RTX);
+		      emit_stack_save (SAVE_BLOCK, old_stack_level);
 		      *old_pending_adj = pending_stack_adjust;
 		      pending_stack_adjust = 0;
 		    }
@@ -2488,7 +2488,7 @@  expand_call (tree exp, rtx target, int ignore)
 	{
 	  if (old_stack_level == 0)
 	    {
-	      emit_stack_save (SAVE_BLOCK, &old_stack_level, NULL_RTX);
+	      emit_stack_save (SAVE_BLOCK, &old_stack_level);
 	      old_stack_pointer_delta = stack_pointer_delta;
 	      old_pending_adj = pending_stack_adjust;
 	      pending_stack_adjust = 0;
@@ -2643,8 +2643,7 @@  expand_call (tree exp, rtx target, int ignore)
 			      : reg_parm_stack_space));
 	      if (old_stack_level == 0)
 		{
-		  emit_stack_save (SAVE_BLOCK, &old_stack_level,
-				   NULL_RTX);
+		  emit_stack_save (SAVE_BLOCK, &old_stack_level);
 		  old_stack_pointer_delta = stack_pointer_delta;
 		  old_pending_adj = pending_stack_adjust;
 		  pending_stack_adjust = 0;
@@ -3101,7 +3100,7 @@  expand_call (tree exp, rtx target, int ignore)
 
       if (old_stack_level)
 	{
-	  emit_stack_restore (SAVE_BLOCK, old_stack_level, NULL_RTX);
+	  emit_stack_restore (SAVE_BLOCK, old_stack_level);
 	  stack_pointer_delta = old_stack_pointer_delta;
 	  pending_stack_adjust = old_pending_adj;
 	  old_stack_allocated = stack_pointer_delta - pending_stack_adjust;
diff --git a/gcc/config/alpha/alpha.md b/gcc/config/alpha/alpha.md
index 04f3ff8..d6fba76 100644
--- a/gcc/config/alpha/alpha.md
+++ b/gcc/config/alpha/alpha.md
@@ -6765,7 +6765,7 @@ 
   /* This bit is the same as expand_builtin_longjmp.  */
   emit_move_insn (hard_frame_pointer_rtx, fp);
   emit_move_insn (pv, lab);
-  emit_stack_restore (SAVE_NONLOCAL, stack, NULL_RTX);
+  emit_stack_restore (SAVE_NONLOCAL, stack);
   emit_use (hard_frame_pointer_rtx);
   emit_use (stack_pointer_rtx);
 
diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
index 17fd319..b9e92f4 100644
--- a/gcc/config/avr/avr.md
+++ b/gcc/config/avr/avr.md
@@ -169,7 +169,7 @@ 
   emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx));
 
   emit_move_insn (hard_frame_pointer_rtx, r_fp);
-  emit_stack_restore (SAVE_NONLOCAL, r_sp, NULL_RTX);
+  emit_stack_restore (SAVE_NONLOCAL, r_sp);
 
   emit_use (hard_frame_pointer_rtx);
   emit_use (stack_pointer_rtx);
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index 0883780..bb87103 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -5649,7 +5649,7 @@ 
      restores $gp as well.  */
   mips_emit_move (hard_frame_pointer_rtx, fp);
   mips_emit_move (pv, lab);
-  emit_stack_restore (SAVE_NONLOCAL, stack, NULL_RTX);
+  emit_stack_restore (SAVE_NONLOCAL, stack);
   mips_emit_move (gp, gpv);
   emit_use (hard_frame_pointer_rtx);
   emit_use (stack_pointer_rtx);
diff --git a/gcc/config/pa/pa.md b/gcc/config/pa/pa.md
index f22692c..24317a5 100644
--- a/gcc/config/pa/pa.md
+++ b/gcc/config/pa/pa.md
@@ -6865,7 +6865,7 @@ 
     fp = force_reg (Pmode, fp);
   emit_move_insn (hard_frame_pointer_rtx, plus_constant (fp, -8));
 
-  emit_stack_restore (SAVE_NONLOCAL, stack, NULL_RTX);
+  emit_stack_restore (SAVE_NONLOCAL, stack);
 
   emit_use (hard_frame_pointer_rtx);
   emit_use (stack_pointer_rtx);
@@ -8310,7 +8310,7 @@  add,l %2,%3,%3\;bv,n %%r0(%3)"
   emit_move_insn (hard_frame_pointer_rtx, plus_constant (fp, -8));
 
   /* This bit is the same as expand_builtin_longjmp.  */
-  emit_stack_restore (SAVE_NONLOCAL, stack, NULL_RTX);
+  emit_stack_restore (SAVE_NONLOCAL, stack);
   emit_use (hard_frame_pointer_rtx);
   emit_use (stack_pointer_rtx);
 
diff --git a/gcc/config/vax/vax.md b/gcc/config/vax/vax.md
index 649f17e..8c3ef00 100644
--- a/gcc/config/vax/vax.md
+++ b/gcc/config/vax/vax.md
@@ -1624,7 +1624,7 @@ 
   emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx));
 
   emit_move_insn (hard_frame_pointer_rtx, fp);
-  emit_stack_restore (SAVE_NONLOCAL, stack, NULL_RTX);
+  emit_stack_restore (SAVE_NONLOCAL, stack);
 
   emit_use (hard_frame_pointer_rtx);
   emit_use (stack_pointer_rtx);
diff --git a/gcc/explow.c b/gcc/explow.c
index 2a18206..34adcb9 100644
--- a/gcc/explow.c
+++ b/gcc/explow.c
@@ -961,13 +961,10 @@  round_push (rtx size)
 /* Save the stack pointer for the purpose in SAVE_LEVEL.  PSAVE is a pointer
    to a previously-created save area.  If no save area has been allocated,
    this function will allocate one.  If a save area is specified, it
-   must be of the proper mode.
-
-   The insns are emitted after insn AFTER, if nonzero, otherwise the insns
-   are emitted at the current position.  */
+   must be of the proper mode.  */
 
 void
-emit_stack_save (enum save_level save_level, rtx *psave, rtx after)
+emit_stack_save (enum save_level save_level, rtx *psave)
 {
   rtx sa = *psave;
   /* The default is that we use a move insn and save in a Pmode object.  */
@@ -1013,38 +1010,17 @@  emit_stack_save (enum save_level save_level, rtx *psave, rtx after)
 	}
     }
 
-  if (after)
-    {
-      rtx seq;
-
-      start_sequence ();
-      do_pending_stack_adjust ();
-      /* We must validize inside the sequence, to ensure that any instructions
-	 created by the validize call also get moved to the right place.  */
-      if (sa != 0)
-	sa = validize_mem (sa);
-      emit_insn (fcn (sa, stack_pointer_rtx));
-      seq = get_insns ();
-      end_sequence ();
-      emit_insn_after (seq, after);
-    }
-  else
-    {
-      do_pending_stack_adjust ();
-      if (sa != 0)
-	sa = validize_mem (sa);
-      emit_insn (fcn (sa, stack_pointer_rtx));
-    }
+  do_pending_stack_adjust ();
+  if (sa != 0)
+    sa = validize_mem (sa);
+  emit_insn (fcn (sa, stack_pointer_rtx));
 }
 
 /* Restore the stack pointer for the purpose in SAVE_LEVEL.  SA is the save
-   area made by emit_stack_save.  If it is zero, we have nothing to do.
-
-   Put any emitted insns after insn AFTER, if nonzero, otherwise at
-   current position.  */
+   area made by emit_stack_save.  If it is zero, we have nothing to do.  */
 
 void
-emit_stack_restore (enum save_level save_level, rtx sa, rtx after)
+emit_stack_restore (enum save_level save_level, rtx sa)
 {
   /* The default is that we use a move insn.  */
   rtx (*fcn) (rtx, rtx) = gen_move_insn;
@@ -1086,18 +1062,7 @@  emit_stack_restore (enum save_level save_level, rtx sa, rtx after)
 
   discard_pending_stack_adjust ();
 
-  if (after)
-    {
-      rtx seq;
-
-      start_sequence ();
-      emit_insn (fcn (stack_pointer_rtx, sa));
-      seq = get_insns ();
-      end_sequence ();
-      emit_insn_after (seq, after);
-    }
-  else
-    emit_insn (fcn (stack_pointer_rtx, sa));
+  emit_insn (fcn (stack_pointer_rtx, sa));
 }
 
 /* Invoke emit_stack_save on the nonlocal_goto_save_area for the current
@@ -1118,7 +1083,7 @@  update_nonlocal_goto_save_area (void)
 		   integer_one_node, NULL_TREE, NULL_TREE);
   r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE);
 
-  emit_stack_save (SAVE_NONLOCAL, &r_save, NULL_RTX);
+  emit_stack_save (SAVE_NONLOCAL, &r_save);
 }
 
 /* Return an rtx representing the address of an area of memory dynamically
diff --git a/gcc/expr.h b/gcc/expr.h
index 263f861..b6e6e6b 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -633,10 +633,10 @@  extern void anti_adjust_stack_and_probe (rtx, bool);
 enum save_level {SAVE_BLOCK, SAVE_FUNCTION, SAVE_NONLOCAL};
 
 /* Save the stack pointer at the specified level.  */
-extern void emit_stack_save (enum save_level, rtx *, rtx);
+extern void emit_stack_save (enum save_level, rtx *);
 
 /* Restore the stack pointer from a save area of the specified level.  */
-extern void emit_stack_restore (enum save_level, rtx, rtx);
+extern void emit_stack_restore (enum save_level, rtx);
 
 /* Invoke emit_stack_save for the nonlocal_goto_save_area.  */
 extern void update_nonlocal_goto_save_area (void);
diff --git a/gcc/function.c b/gcc/function.c
index 8b80485..19b480d 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -5121,10 +5121,15 @@  expand_function_end (void)
   if (! EXIT_IGNORE_STACK
       && cfun->calls_alloca)
     {
-      rtx tem = 0;
+      rtx tem = 0, seq;
 
-      emit_stack_save (SAVE_FUNCTION, &tem, parm_birth_insn);
-      emit_stack_restore (SAVE_FUNCTION, tem, NULL_RTX);
+      start_sequence ();
+      emit_stack_save (SAVE_FUNCTION, &tem);
+      seq = get_insns ();
+      end_sequence ();
+      emit_insn_before (seq, parm_birth_insn);
+
+      emit_stack_restore (SAVE_FUNCTION, tem);
     }
 
   /* ??? This should no longer be necessary since stupid is no longer with
diff --git a/gcc/stmt.c b/gcc/stmt.c
index 7d4cbb0..b65c6db 100644
--- a/gcc/stmt.c
+++ b/gcc/stmt.c
@@ -2006,7 +2006,7 @@  expand_stack_save (void)
   rtx ret = NULL_RTX;
 
   do_pending_stack_adjust ();
-  emit_stack_save (SAVE_BLOCK, &ret, NULL_RTX);
+  emit_stack_save (SAVE_BLOCK, &ret);
   return ret;
 }
 
@@ -2017,7 +2017,7 @@  expand_stack_restore (tree var)
   rtx sa = expand_normal (var);
 
   sa = convert_memory_address (Pmode, sa);
-  emit_stack_restore (SAVE_BLOCK, sa, NULL_RTX);
+  emit_stack_restore (SAVE_BLOCK, sa);
 }
 
 /* Do the insertion of a case label into case_list.  The labels are