[PR50527] Don't assume alignment of vla-related allocas.

Submitted by Tom de Vries on Sept. 28, 2011, 9:34 a.m.

Details

Message ID 4E82EA3E.4050000@mentor.com
State New
Headers show

Commit Message

Tom de Vries Sept. 28, 2011, 9:34 a.m.
Richard,

I got a patch for PR50527.

The patch prevents the alignment of vla-related allocas to be set to
BIGGEST_ALIGNMENT in ccp. The alignment may turn out smaller after folding
the alloca.

Bootstrapped and regtested on x86_64.

OK for trunk?

Thanks,
- Tom

2011-09-27  Tom de Vries  <tom@codesourcery.com>

	* tree-ssa-ccp.c (evaluate_stmt): Don't assume alignment for vla-related
	allocas.

	* gcc.dg/pr50527.c: New test.

Comments

Richard Guenther Sept. 28, 2011, 9:53 a.m.
On Wed, Sep 28, 2011 at 11:34 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> Richard,
>
> I got a patch for PR50527.
>
> The patch prevents the alignment of vla-related allocas to be set to
> BIGGEST_ALIGNMENT in ccp. The alignment may turn out smaller after folding
> the alloca.
>
> Bootstrapped and regtested on x86_64.
>
> OK for trunk?

Hmm.  As gfortran with -fstack-arrays uses VLAs it's probably bad that
the vectorizer then will no longer see that the arrays are properly aligned.

I'm not sure what the best thing to do is here, other than trying to record
the alignment requirement of the VLA somewhere.

Forcing the alignment of the alloca replacement decl to BIGGEST_ALIGNMENT
has the issue that it will force stack-realignment which isn't free (and the
point was to make the decl cheaper than the alloca).  But that might
possibly be the better choice.

Any other thoughts?

Thanks,
Richard.

> Thanks,
> - Tom
>
> 2011-09-27  Tom de Vries  <tom@codesourcery.com>
>
>        * tree-ssa-ccp.c (evaluate_stmt): Don't assume alignment for vla-related
>        allocas.
>
>        * gcc.dg/pr50527.c: New test.
>

Patch hide | download patch | download mbox

Index: gcc/tree-ssa-ccp.c
===================================================================
--- gcc/tree-ssa-ccp.c (revision 179210)
+++ gcc/tree-ssa-ccp.c (working copy)
@@ -1632,6 +1632,8 @@  evaluate_stmt (gimple stmt)
 	      break;
 
 	    case BUILT_IN_ALLOCA:
+	      if (gimple_call_alloca_for_var_p (stmt))
+		break;
 	      val.lattice_val = CONSTANT;
 	      val.value = build_int_cst (TREE_TYPE (gimple_get_lhs (stmt)), 0);
 	      val.mask = shwi_to_double_int
Index: gcc/testsuite/gcc.dg/pr50527.c
===================================================================
--- /dev/null (new file)
+++ gcc/testsuite/gcc.dg/pr50527.c (revision 0)
@@ -0,0 +1,46 @@ 
+/* { dg-do run } */
+/* { dg-options "-Os --param large-stack-frame=30" } */
+
+extern void abort (void);
+
+void __attribute__((noinline)) 
+bar (char *a)
+{
+}
+
+void __attribute__((noinline)) 
+foo (char *a, int b)
+{
+}
+
+void __attribute__((noinline))
+test_align (char *p, int aligned, unsigned int mask)
+{
+  int p_aligned = ((unsigned long int)p & mask) == 0;
+  if (aligned != p_aligned)
+    abort ();
+}
+
+int
+main ()
+{
+  const int kIterations = 4;
+  char results[kIterations];
+  int i;
+  unsigned int mask;
+
+  mask = 0xf;
+  test_align (results, ((unsigned long int)results & mask) == 0, mask);
+  mask = 0x7;
+  test_align (results, ((unsigned long int)results & mask) == 0, mask);
+  mask = 0x3;
+  test_align (results, ((unsigned long int)results & mask) == 0, mask);
+  mask = 0x1;
+  test_align (results, ((unsigned long int)results & mask) == 0, mask);
+
+  bar (results);
+  for (i = 0; i < kIterations; i++)
+    foo ("%d ", results[i]);
+
+  return 0;
+}