public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR 16660, attribute((aligned)) doesn't work for variables on  the stack for greater than required alignment
@ 2007-01-30  2:42 Andrew_Pinski
  2007-01-31 15:20 ` Meissner, Michael
                   ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Andrew_Pinski @ 2007-01-30  2:42 UTC (permalink / raw)
  To: gcc-patches; +Cc: Trevor_Smigiel, Russell_Olsen

[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]

Hi,
  This patch fixes PR 16660 where we ignore the user supplied allignment 
on stack variables if it is greater than the required alignment on the 
specific target.  This fixes the issue by oversizing the stack variable 
and then manually aligning the variable.

I had to remove some asserts from the x86 back-end as they are no longer 
valid from the point of view of the middle-end could have a case where 
preferred_alignment is greater than the target's preferred alignment and 
the stack alignment that is needed can be greater than the target's 
preferred alignment.


OK? Bootstrapped and tested on i686-linux-gnu and powerpc-linux-gnu with 
no regressions.

Thanks,
Andrew Pinski

:ADDPATCH middle-end:

ChangeLog:

        * expr.h (allocate_dynamic_stack_space_1): New function prototype.
        * functionc.c (assign_temp): Take into account the alignment
        of the temp if it is greater than the target's preferred 
alignment.
        * cfgexpand.c: Include optabs.h.
        (get_decl_align_unit): Update comment and don't lower the 
alignment
        if it is greater than the target's preferred alignment.
        (alloc_stack_frame_space): Take an unsigned HOST_WIDE_INT for 
align.
        Take into account the variable's alignment if it is greater than
        the target's preferred alignment.
        (expand_one_stack_var_at): Likewise.
        (defer_stack_allocation): Defer variables who's alignment is 
greater
        than the target's preferred alignment.
        * explow.c (allocate_dynamic_stack_space_1): Split out from
        allocate_dynamic_stack_space and take into acount the required
        alignment.
        (allocate_dynamic_stack_space): Call 
allocate_dynamic_stack_space_1.
        * Makefile.in (cfgexpand.o): Update dependecies.
        * stmt.c (expand_decl):  Take into account the alignment
        of the variable if it is greater than the target's preferred
        alignment.
        * config/i386/i386.c (ix86_compute_frame_layout): Don't assert
        that preferred alignment is greater than the normal preferred
        alignment.  Don't assert that the stack alignment needed is 
greater
        than the normal preferred alignment. 



[-- Attachment #2: attributealignedstack.diff.txt --]
[-- Type: text/plain, Size: 12640 bytes --]

Index: testsuite/gcc.c-torture/execute/pr16660-1.c
===================================================================
--- testsuite/gcc.c-torture/execute/pr16660-1.c	(revision 0)
+++ testsuite/gcc.c-torture/execute/pr16660-1.c	(revision 0)
@@ -0,0 +1,11 @@
+typedef __SIZE_TYPE__ size_t;
+#define ALIGNMENT 256
+int main(void)
+{
+  int a[sizeof(int)/ALIGNMENT] __attribute__((aligned(ALIGNMENT)));
+  size_t b = (size_t)&a;
+  if (b&(ALIGNMENT-1))
+    return 1;
+  return 0;
+}
+
Index: expr.h
===================================================================
--- expr.h	(revision 121163)
+++ expr.h	(working copy)
@@ -707,6 +707,7 @@ extern void update_nonlocal_goto_save_ar
 /* Allocate some space on the stack dynamically and return its address.  An rtx
    says how many bytes.  */
 extern rtx allocate_dynamic_stack_space (rtx, rtx, int);
+extern rtx allocate_dynamic_stack_space_1 (rtx, rtx, int, int);
 
 /* Probe a range of stack addresses from FIRST to FIRST+SIZE, inclusive.
    FIRST is a constant and size is a Pmode RTX.  These are offsets from the
Index: function.c
===================================================================
--- function.c	(revision 121163)
+++ function.c	(working copy)
@@ -812,6 +812,7 @@ assign_temp (tree type_or_decl, int keep
   if (mode == BLKmode || memory_required)
     {
       HOST_WIDE_INT size = int_size_in_bytes (type);
+      unsigned HOST_WIDE_INT align;
       rtx tmp;
 
       /* Zero sized arrays are GNU C extension.  Set size to 1 to avoid
@@ -836,7 +837,37 @@ assign_temp (tree type_or_decl, int keep
 	  size = 1;
 	}
 
-      tmp = assign_stack_temp_for_type (mode, size, keep, type);
+      align = TYPE_ALIGN (type);
+      if (decl && DECL_ALIGN (decl) > align)
+	align = DECL_ALIGN (decl);
+
+      /* When the user specifies an alignment larger than
+         BIGGEST_ALIGNMENT we need to allocate extra space so we can
+         adjust this decl's address.  */
+      if (align > PREFERRED_STACK_BOUNDARY)
+	{
+	  rtx addr;
+	  HOST_WIDE_INT pad = (align - PREFERRED_STACK_BOUNDARY) / BITS_PER_UNIT;
+	  size += pad;
+	  tmp = assign_stack_temp_for_type (mode, size, keep, type);
+	  addr = XEXP (tmp, 0);
+	  /* CEIL_DIV_EXPR needs to worry about the addition overflowing,
+	     but we know it can't.  So add ourselves and then do
+	     TRUNC_DIV_EXPR.  */
+	  addr = expand_binop (Pmode, add_optab, addr,
+			       GEN_INT (align / BITS_PER_UNIT - 1),
+			       NULL_RTX, 1, OPTAB_LIB_WIDEN);
+	  addr = expand_divmod (0, TRUNC_DIV_EXPR, Pmode, addr,
+			        GEN_INT (align / BITS_PER_UNIT),
+			        NULL_RTX, 1);
+	  addr = expand_mult (Pmode, addr,
+			      GEN_INT (align / BITS_PER_UNIT),
+			      NULL_RTX, 1);
+	  tmp = change_address (tmp, VOIDmode, addr);
+	}
+      else
+	tmp = assign_stack_temp_for_type (mode, size, keep, type);
+
       return tmp;
     }
 
Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 121163)
+++ cfgexpand.c	(working copy)
@@ -41,6 +41,7 @@ Boston, MA 02110-1301, USA.  */
 #include "params.h"
 #include "tree-inline.h"
 #include "value-prof.h"
+#include "optabs.h"
 
 /* Verify that there is exactly single jump instruction since last and attach
    REG_BR_PROB note specifying probability.
@@ -151,8 +152,7 @@ static bool has_protected_decls;
    smaller than our cutoff threshold.  Used for -Wstack-protector.  */
 static bool has_short_buffer;
 
-/* Discover the byte alignment to use for DECL.  Ignore alignment
-   we can't do with expected alignment of the stack boundary.  */
+/* Discover the byte alignment to use for DECL.  */
 
 static unsigned int
 get_decl_align_unit (tree decl)
@@ -161,8 +161,6 @@ get_decl_align_unit (tree decl)
 
   align = DECL_ALIGN (decl);
   align = LOCAL_ALIGNMENT (TREE_TYPE (decl), align);
-  if (align > PREFERRED_STACK_BOUNDARY)
-    align = PREFERRED_STACK_BOUNDARY;
   if (cfun->stack_alignment_needed < align)
     cfun->stack_alignment_needed = align;
 
@@ -173,11 +171,21 @@ get_decl_align_unit (tree decl)
    Return the frame offset.  */
 
 static HOST_WIDE_INT
-alloc_stack_frame_space (HOST_WIDE_INT size, HOST_WIDE_INT align)
+alloc_stack_frame_space (HOST_WIDE_INT size, unsigned HOST_WIDE_INT align)
 {
   HOST_WIDE_INT offset, new_frame_offset;
 
   new_frame_offset = frame_offset;
+
+  /* Allocate extra space if the alignment required is greater than the
+     stack boundary and then assume the RTL expansion of the variable, gets
+     the correct alignment.  */
+  if (align > PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT)
+    {
+      size += align;
+      align = PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT;
+    }
+
   if (FRAME_GROWS_DOWNWARD)
     {
       new_frame_offset -= size + frame_phase;
@@ -523,23 +531,31 @@ dump_stack_var_partition (void)
 static void
 expand_one_stack_var_at (tree decl, HOST_WIDE_INT offset)
 {
-  HOST_WIDE_INT align;
+  unsigned HOST_WIDE_INT align;
   rtx x;
 
   /* If this fails, we've overflowed the stack frame.  Error nicely?  */
   gcc_assert (offset == trunc_int_for_mode (offset, Pmode));
 
   x = plus_constant (virtual_stack_vars_rtx, offset);
-  x = gen_rtx_MEM (DECL_MODE (decl), x);
+  align = get_decl_align_unit (decl);
 
-  /* Set alignment we actually gave this decl.  */
-  offset -= frame_phase;
-  align = offset & -offset;
-  align *= BITS_PER_UNIT;
-  if (align > STACK_BOUNDARY || align == 0)
-    align = STACK_BOUNDARY;
-  DECL_ALIGN (decl) = align;
-  DECL_USER_ALIGN (decl) = 0;
+  if (align > PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT)
+    {
+      rtx addr = x;
+      addr = expand_binop (Pmode, add_optab, addr,
+			   GEN_INT (align - 1),
+			   NULL_RTX, 1, OPTAB_LIB_WIDEN);
+      addr = expand_divmod (0, TRUNC_DIV_EXPR, Pmode, addr,
+			    GEN_INT (align),
+			    NULL_RTX, 1);
+      addr = expand_mult (Pmode, addr,
+			  GEN_INT (align),
+			  NULL_RTX, 1);
+      x = addr;
+    }
+
+  x = gen_rtx_MEM (DECL_MODE (decl), x);
 
   set_mem_attributes (x, decl, true);
   SET_DECL_RTL (decl, x);
@@ -740,6 +756,11 @@ defer_stack_allocation (tree var, bool t
   if (optimize == 0 && tree_low_cst (DECL_SIZE_UNIT (var), 1) < 32)
     return false;
 
+  /* When the variable needs additional code to be aligned on the
+     stack we allocate separately just for ease of implementation.  */
+  if (get_decl_align_unit (var) > PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT)
+    return false;
+
   return true;
 }
 
Index: explow.c
===================================================================
--- explow.c	(revision 121163)
+++ explow.c	(working copy)
@@ -1090,16 +1090,28 @@ update_nonlocal_goto_save_area (void)
    TARGET is a place in which the address can be placed.
 
    KNOWN_ALIGN is the alignment (in bits) that we know SIZE has.  */
-
 rtx
 allocate_dynamic_stack_space (rtx size, rtx target, int known_align)
 {
+  return allocate_dynamic_stack_space_1 (size, target, known_align,
+				       BIGGEST_ALIGNMENT);
+}
+
+/* Like allocate_dynamic_stack_space expect provide a way, REQUIRED_ALGIN,
+   to say the required alignment of the address.  */
+rtx
+allocate_dynamic_stack_space_1 (rtx size, rtx target, int known_align,
+				int required_align)
+{
   /* If we're asking for zero bytes, it doesn't matter what we point
      to since we can't dereference it.  But return a reasonable
      address anyway.  */
   if (size == const0_rtx)
     return virtual_stack_dynamic_rtx;
 
+  if (required_align < BIGGEST_ALIGNMENT)
+    required_align = BIGGEST_ALIGNMENT;
+
   /* Otherwise, show we're calling alloca or equivalent.  */
   current_function_calls_alloca = 1;
 
@@ -1128,13 +1140,13 @@ allocate_dynamic_stack_space (rtx size, 
 #if defined (STACK_DYNAMIC_OFFSET) || defined (STACK_POINTER_OFFSET)
 #define MUST_ALIGN 1
 #else
-#define MUST_ALIGN (PREFERRED_STACK_BOUNDARY < BIGGEST_ALIGNMENT)
+#define MUST_ALIGN (PREFERRED_STACK_BOUNDARY < required_align)
 #endif
 
   if (MUST_ALIGN)
     size
       = force_operand (plus_constant (size,
-				      BIGGEST_ALIGNMENT / BITS_PER_UNIT - 1),
+				      required_align / BITS_PER_UNIT - 1),
 		       NULL_RTX);
 
 #ifdef SETJMP_VIA_SAVE_AREA
@@ -1294,13 +1306,13 @@ allocate_dynamic_stack_space (rtx size, 
 	 but we know it can't.  So add ourselves and then do
 	 TRUNC_DIV_EXPR.  */
       target = expand_binop (Pmode, add_optab, target,
-			     GEN_INT (BIGGEST_ALIGNMENT / BITS_PER_UNIT - 1),
+			     GEN_INT (required_align / BITS_PER_UNIT - 1),
 			     NULL_RTX, 1, OPTAB_LIB_WIDEN);
       target = expand_divmod (0, TRUNC_DIV_EXPR, Pmode, target,
-			      GEN_INT (BIGGEST_ALIGNMENT / BITS_PER_UNIT),
+			      GEN_INT (required_align / BITS_PER_UNIT),
 			      NULL_RTX, 1);
       target = expand_mult (Pmode, target,
-			    GEN_INT (BIGGEST_ALIGNMENT / BITS_PER_UNIT),
+			    GEN_INT (required_align / BITS_PER_UNIT),
 			    NULL_RTX, 1);
     }
 
Index: Makefile.in
===================================================================
--- Makefile.in	(revision 121163)
+++ Makefile.in	(working copy)
@@ -2565,7 +2565,7 @@ cfgexpand.o : cfgexpand.c $(TREE_FLOW_H)
    $(RTL_H) $(TREE_H) $(TM_P_H) $(EXPR_H) $(FUNCTION_H) $(TIMEVAR_H) $(TM_H) \
    coretypes.h $(TREE_DUMP_H) except.h langhooks.h tree-pass.h $(RTL_H) \
    $(DIAGNOSTIC_H) toplev.h $(BASIC_BLOCK_H) $(FLAGS_H) debug.h $(PARAMS_H) \
-   value-prof.h
+   value-prof.h $(OPTABS_H)
 cfgrtl.o : cfgrtl.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \
    $(FLAGS_H) insn-config.h $(BASIC_BLOCK_H) $(REGS_H) hard-reg-set.h \
    output.h toplev.h $(FUNCTION_H) except.h $(TM_P_H) insn-config.h $(EXPR_H) \
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 121163)
+++ config/i386/i386.c	(working copy)
@@ -5320,9 +5320,6 @@ ix86_compute_frame_layout (struct ix86_f
 
   gcc_assert (!size || stack_alignment_needed);
   gcc_assert (preferred_alignment >= STACK_BOUNDARY / BITS_PER_UNIT);
-  gcc_assert (preferred_alignment <= PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT);
-  gcc_assert (stack_alignment_needed
-	      <= PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT);
 
   if (stack_alignment_needed < STACK_BOUNDARY / BITS_PER_UNIT)
     stack_alignment_needed = STACK_BOUNDARY / BITS_PER_UNIT;
Index: stmt.c
===================================================================
--- stmt.c	(revision 121163)
+++ stmt.c	(working copy)
@@ -1848,6 +1848,7 @@ void
 expand_decl (tree decl)
 {
   tree type;
+  unsigned HOST_WIDE_INT align;
 
   type = TREE_TYPE (decl);
 
@@ -1872,6 +1873,13 @@ expand_decl (tree decl)
   if (TREE_STATIC (decl) || DECL_EXTERNAL (decl))
     return;
 
+  if (DECL_USER_ALIGN (decl))
+    align = DECL_ALIGN (decl);
+  else if (DECL_MODE (decl) == BLKmode)
+    align = BIGGEST_ALIGNMENT;
+  else
+    align = TYPE_ALIGN (type);
+
   /* Create the RTL representation for the variable.  */
 
   if (type == error_mark_node)
@@ -1892,7 +1900,8 @@ expand_decl (tree decl)
       set_mem_attributes (x, decl, 1);
       SET_DECL_RTL (decl, x);
     }
-  else if (use_register_for_decl (decl))
+  else if (use_register_for_decl (decl)
+	   && align <= PREFERRED_STACK_BOUNDARY)
     {
       /* Automatic variable that can go in a register.  */
       int unsignedp = TYPE_UNSIGNED (type);
@@ -1939,10 +1948,16 @@ expand_decl (tree decl)
 	  oldaddr = XEXP (DECL_RTL (decl), 0);
 	}
 
-      /* Set alignment we actually gave this decl.  */
-      DECL_ALIGN (decl) = (DECL_MODE (decl) == BLKmode ? BIGGEST_ALIGNMENT
-			   : GET_MODE_BITSIZE (DECL_MODE (decl)));
-      DECL_USER_ALIGN (decl) = 0;
+      /* Set alignment we actually gave this decl if we don't have
+	 an user specific alignment and the alignment is less than
+	 the biggest alignment.  */
+      if (! DECL_USER_ALIGN (decl)
+	  || DECL_ALIGN (decl) <= BIGGEST_ALIGNMENT)
+        {
+	  DECL_ALIGN (decl) = (DECL_MODE (decl) == BLKmode ? BIGGEST_ALIGNMENT
+			       : GET_MODE_BITSIZE (DECL_MODE (decl)));
+	  DECL_USER_ALIGN (decl) = 0;
+	}
 
       x = assign_temp (decl, 1, 1, 1);
       set_mem_attributes (x, decl, 1);
@@ -1973,8 +1988,10 @@ expand_decl (tree decl)
 	 DECL_ALIGN says how the variable is to be aligned and we
 	 cannot use it to conclude anything about the alignment of
 	 the size.  */
-      address = allocate_dynamic_stack_space (size, NULL_RTX,
-					      TYPE_ALIGN (TREE_TYPE (decl)));
+      address = allocate_dynamic_stack_space_1 (size, NULL_RTX,
+						TYPE_ALIGN (TREE_TYPE (decl)),
+						MAX (TYPE_ALIGN (TREE_TYPE (decl)),
+						align));
 
       /* Reference the variable indirect through that rtx.  */
       x = gen_rtx_MEM (DECL_MODE (decl), address);

^ permalink raw reply	[flat|nested] 27+ messages in thread
[parent not found: <OF15BFC61A.42B05901-ON882572DE.000355EF-882572DE.00775314@LocalDomain>]
end of thread, other threads:[~2008-01-24 16:50 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-01-30  2:42 [PATCH] Fix PR 16660, attribute((aligned)) doesn't work for variables on the stack for greater than required alignment Andrew_Pinski
2007-01-31 15:20 ` Meissner, Michael
2007-02-02 23:49 ` Richard Henderson
2007-05-17 21:43   ` Andrew_Pinski
     [not found]   ` <OF15BFC61A.42B05901-ON882572DE.000355EF-882572DE.007752F8@playstation.sony.com>
2007-05-17 23:50     ` trevor_smigiel
2007-05-18  0:20       ` Andrew_Pinski
2007-05-18 17:42       ` Andrew_Pinski
2007-07-02 23:56         ` Andrew Pinski
2007-08-04 11:46           ` Andrew Pinski
2007-08-28 20:08             ` Andrew Pinski
2007-09-17 22:22               ` Andrew Pinski
2007-10-09 21:20                 ` Andrew Pinski
2007-10-10  0:31                   ` H.J. Lu
2007-10-10  0:38                     ` Andrew_Pinski
2007-10-10  2:17                       ` H.J. Lu
2007-10-10  3:01                         ` Andrew Pinski
2007-10-10  3:44                           ` How to make gcc psABI conformant H.J. Lu
2007-10-10 11:43                             ` Daniel Jacobowitz
2007-10-10 13:05                               ` H.J. Lu
2007-10-10 13:10                                 ` Daniel Jacobowitz
2007-10-10 13:28                                   ` H.J. Lu
2007-10-10 13:35                                     ` H.J. Lu
2007-10-10  1:07                   ` [PATCH] Fix PR 16660, attribute((aligned)) doesn't work for variables on the stack for greater than required alignment Dorit Nuzman
2008-01-24 14:52                   ` Andrew Pinski
2008-01-24 18:07                     ` H.J. Lu
2007-02-05 22:39 ` Dorit Nuzman
     [not found] <OF15BFC61A.42B05901-ON882572DE.000355EF-882572DE.00775314@LocalDomain>
2007-05-17 21:45 ` Andrew_Pinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).