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]

PATCH: PR target/24419: ix86 prologue clobbers memory when it shouldn't


There are 2 issues with ix86 backend:

1. When ix86_expand_prologue uses mov with red zone, memory shouldn't
be clobbered.  But ix86_expand_prologue calls pro_epilogue_adjust_stack
which clobbers memory.
2. We convert stack pointer subtractions to push even when memory isn't
clobbered with patterns like

;; Convert esp subtractions to push.
(define_peephole2
  [(match_scratch:SI 0 "r")
   (parallel [(set (reg:SI SP_REG) (plus:SI (reg:SI SP_REG) (const_int
-4)))
              (clobber (reg:CC FLAGS_REG))])]
  "optimize_size || !TARGET_SUB_ESP_4"
  [(clobber (match_dup 0))
   (set (mem:SI (pre_dec:SI (reg:SI SP_REG))) (match_dup 0))])

It is unsafe with red zone since it will clobber the memory. This
patch will make sure that

1. pro_epilogue_adjust_stack won't clobber memory when it is called
after mov is used in prologue with red zone.
2. Don't convert stack pointer subtractions to push when memory isn't
clobbered if red zone is enabled.

I have tested this patch on Linux/i686 and Linux/x86-64 without any
regressions.


H.J.
----
2005-10-18  H.J. Lu  <hongjiu.lu@intel.com>
	    Jan Hubicka <jh@suse.cz>

	PR target/24419
	* i386.c (pro_epilogue_adjust_stack): Add another argument to
	indicate if memory should be clobbered or not. Adjust stack
	pointer directly if memory shouldn't be clobbered.
	(ix86_expand_prologue): Updated.
	(ix86_expand_epilogue): Likewise.

	* i386.md: Don't convert stack pointer subtractions to push
	when memory isn't clobbered if red zone is enabled.

--- gcc/config/i386/i386.c.stack	2005-10-18 10:08:25.000000000 -0700
+++ gcc/config/i386/i386.c	2005-10-18 10:11:45.000000000 -0700
@@ -4323,22 +4323,40 @@ ix86_emit_save_regs_using_mov (rtx point
    otherwise.  */
 
 static void
-pro_epilogue_adjust_stack (rtx dest, rtx src, rtx offset, int style)
+pro_epilogue_adjust_stack (rtx dest, rtx src, rtx offset, int style,
+			   bool clobber_memory)
 {
   rtx insn;
 
   if (! TARGET_64BIT)
-    insn = emit_insn (gen_pro_epilogue_adjust_stack_1 (dest, src, offset));
+    {
+      if (clobber_memory)
+	insn = emit_insn (gen_pro_epilogue_adjust_stack_1 (dest, src,
+							   offset));
+      else
+	insn = emit_insn (gen_addsi3 (stack_pointer_rtx,
+				      stack_pointer_rtx, offset));
+    }
   else if (x86_64_immediate_operand (offset, DImode))
-    insn = emit_insn (gen_pro_epilogue_adjust_stack_rex64 (dest, src, offset));
+    {
+      if (clobber_memory)
+        insn = emit_insn (gen_pro_epilogue_adjust_stack_rex64 (dest,
+							       src,
+							       offset));
+      else
+	insn = emit_insn (gen_adddi3 (stack_pointer_rtx,
+				      stack_pointer_rtx, offset));
+    }
   else
     {
       rtx r11;
       /* r11 is used by indirect sibcall return as well, set before the
 	 epilogue and used after the epilogue.  ATM indirect sibcall
 	 shouldn't be used together with huge frame sizes in one
-	 function because of the frame_size check in sibcall.c.  */
-      if (style == 0)
+	 function because of the frame_size check in sibcall.c. If
+	 huge frame size is used, memory should always be clobbered
+	 when stack is adjusted.  */
+      if (style == 0 || !clobber_memory)
 	abort ();
       r11 = gen_rtx_REG (DImode, FIRST_REX_INT_REG + 3 /* R11 */);
       insn = emit_insn (gen_rtx_SET (DImode, r11, offset));
@@ -4360,6 +4378,7 @@ ix86_expand_prologue (void)
   bool pic_reg_used;
   struct ix86_frame frame;
   HOST_WIDE_INT allocate;
+  bool using_mov;
 
   ix86_compute_frame_layout (&frame);
 
@@ -4384,7 +4403,8 @@ ix86_expand_prologue (void)
 
   /* When using red zone we may start register saving before allocating
      the stack frame saving one cycle of the prologue.  */
-  if (TARGET_RED_ZONE && frame.save_regs_using_mov)
+  using_mov = TARGET_RED_ZONE && frame.save_regs_using_mov;
+  if (using_mov)
     ix86_emit_save_regs_using_mov (frame_pointer_needed ? hard_frame_pointer_rtx
 				   : stack_pointer_rtx,
 				   -frame.nregs * UNITS_PER_WORD);
@@ -4393,7 +4413,7 @@ ix86_expand_prologue (void)
     ;
   else if (! TARGET_STACK_PROBE || allocate < CHECK_STACK_LIMIT)
     pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
-			       GEN_INT (-allocate), -1);
+			       GEN_INT (-allocate), -1, !using_mov);
   else
     {
       /* Only valid for Win32.  */
@@ -4572,7 +4592,7 @@ ix86_expand_epilogue (int style)
 	      emit_move_insn (hard_frame_pointer_rtx, tmp);
 
 	      pro_epilogue_adjust_stack (stack_pointer_rtx, sa,
-					 const0_rtx, style);
+					 const0_rtx, style, true);
 	    }
 	  else
 	    {
@@ -4586,7 +4606,7 @@ ix86_expand_epilogue (int style)
 	pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
 				   GEN_INT (frame.to_allocate
 					    + frame.nregs * UNITS_PER_WORD),
-				   style);
+				   style, true);
       /* If not an i386, mov & pop is faster than "leave".  */
       else if (TARGET_USE_LEAVE || optimize_size
 	       || !cfun->machine->use_fast_prologue_epilogue)
@@ -4595,7 +4615,7 @@ ix86_expand_epilogue (int style)
 	{
 	  pro_epilogue_adjust_stack (stack_pointer_rtx,
 				     hard_frame_pointer_rtx,
-				     const0_rtx, style);
+				     const0_rtx, style, true);
 	  if (TARGET_64BIT)
 	    emit_insn (gen_popdi1 (hard_frame_pointer_rtx));
 	  else
@@ -4612,11 +4632,12 @@ ix86_expand_epilogue (int style)
 	    abort ();
 	  pro_epilogue_adjust_stack (stack_pointer_rtx,
 				     hard_frame_pointer_rtx,
-				     GEN_INT (offset), style);
+				     GEN_INT (offset), style, true);
 	}
       else if (frame.to_allocate)
 	pro_epilogue_adjust_stack (stack_pointer_rtx, stack_pointer_rtx,
-				   GEN_INT (frame.to_allocate), style);
+				   GEN_INT (frame.to_allocate), style,
+				   true);
 
       for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
 	if (ix86_save_reg (regno, false))
--- gcc/config/i386/i386.md.stack	2005-10-14 14:33:54.000000000 -0700
+++ gcc/config/i386/i386.md	2005-10-18 11:59:10.000000000 -0700
@@ -19527,11 +19527,15 @@
 	      (clobber (mem:BLK (scratch)))])])
 
 ;; Convert esp subtractions to push.
+;; This conversion is safe only under assumption that unallocated stack is
+;; implicitly clobbered as specified by 32bit ABI (for signal handlers and such).
+;; This is not valid with red zone, but we can work harder and enable the
+;; optimization for functions that are not using it.
 (define_peephole2
   [(match_scratch:SI 0 "r")
    (parallel [(set (reg:SI SP_REG) (plus:SI (reg:SI SP_REG) (const_int -4)))
 	      (clobber (reg:CC FLAGS_REG))])]
-  "optimize_size || !TARGET_SUB_ESP_4"
+  "(optimize_size || !TARGET_SUB_ESP_4) && !TARGET_RED_ZONE"
   [(clobber (match_dup 0))
    (set (mem:SI (pre_dec:SI (reg:SI SP_REG))) (match_dup 0))])
 
@@ -19539,7 +19543,7 @@
   [(match_scratch:SI 0 "r")
    (parallel [(set (reg:SI SP_REG) (plus:SI (reg:SI SP_REG) (const_int -8)))
 	      (clobber (reg:CC FLAGS_REG))])]
-  "optimize_size || !TARGET_SUB_ESP_8"
+  "(optimize_size || !TARGET_SUB_ESP_8) && !TARGET_RED_ZONE"
   [(clobber (match_dup 0))
    (set (mem:SI (pre_dec:SI (reg:SI SP_REG))) (match_dup 0))
    (set (mem:SI (pre_dec:SI (reg:SI SP_REG))) (match_dup 0))])
@@ -19659,11 +19663,15 @@
 	      (clobber (mem:BLK (scratch)))])])
 
 ;; Convert esp subtractions to push.
+;; This conversion is safe only under assumption that unallocated stack is
+;; implicitly clobbered as specified by 32bit ABI (for signal handlers and such).
+;; This is not valid with red zone, but we can work harder and enable the
+;; optimization for functions that are not using it.
 (define_peephole2
   [(match_scratch:DI 0 "r")
    (parallel [(set (reg:DI SP_REG) (plus:DI (reg:DI SP_REG) (const_int -8)))
 	      (clobber (reg:CC FLAGS_REG))])]
-  "optimize_size || !TARGET_SUB_ESP_4"
+  "(optimize_size || !TARGET_SUB_ESP_4) && !TARGET_RED_ZONE"
   [(clobber (match_dup 0))
    (set (mem:DI (pre_dec:DI (reg:DI SP_REG))) (match_dup 0))])
 
@@ -19671,7 +19679,7 @@
   [(match_scratch:DI 0 "r")
    (parallel [(set (reg:DI SP_REG) (plus:DI (reg:DI SP_REG) (const_int -16)))
 	      (clobber (reg:CC FLAGS_REG))])]
-  "optimize_size || !TARGET_SUB_ESP_8"
+  "(optimize_size || !TARGET_SUB_ESP_8) && !TARGET_RED_ZONE"
   [(clobber (match_dup 0))
    (set (mem:DI (pre_dec:DI (reg:DI SP_REG))) (match_dup 0))
    (set (mem:DI (pre_dec:DI (reg:DI SP_REG))) (match_dup 0))])


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