diff mbox

Fix PR tree-optimization/51315

Message ID 201201041724.55659.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou Jan. 4, 2012, 4:24 p.m. UTC
> OK.  But passing small structures by value doesn't seem that rare --
> especially in C++ -- and it doesn't feel right to disable SRA just because
> the backend likes to increase the alignment of stack vars.

Agreed.

> ...something like this sounds good, although you seem less than happy
> with it :-)

Just not very comfortable with it, as we're walking a thin line.  I've attached 
a more "dangerous" testcase that is now optimized again (and still works).

Regtested on SPARC/Solaris.  Can you confirm that this fixes the pessimization 
in all cases (and run the C testsuite for your favorite ABI variant)?  TIA.


	PR tree-optimization/51315
	* tree-sra.c (tree_non_aligned_mem_for_access_p): New predicate.
	(build_accesses_from_assign): Use it instead of tree_non_aligned_mem_p.


	* gcc.c-torture/execute/20120104-1.c: New test.

Comments

Richard Sandiford Jan. 4, 2012, 9:54 p.m. UTC | #1
Eric Botcazou <ebotcazou@adacore.com> writes:
> Regtested on SPARC/Solaris.  Can you confirm that this fixes the
> pessimization in all cases (and run the C testsuite for your favorite
> ABI variant)?  TIA.

It does, and there were no regression on a C-only build & test for
mips64-linux-gnu (-mabi=32/-mips16, -mabi=32, -mabi=n32, -mabi=64).

Thanks for the quick fix!

Richard
Richard Biener Jan. 5, 2012, 9:09 a.m. UTC | #2
On Wed, Jan 4, 2012 at 5:24 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> OK.  But passing small structures by value doesn't seem that rare --
>> especially in C++ -- and it doesn't feel right to disable SRA just because
>> the backend likes to increase the alignment of stack vars.
>
> Agreed.
>
>> ...something like this sounds good, although you seem less than happy
>> with it :-)
>
> Just not very comfortable with it, as we're walking a thin line.  I've attached
> a more "dangerous" testcase that is now optimized again (and still works).
>
> Regtested on SPARC/Solaris.  Can you confirm that this fixes the pessimization
> in all cases (and run the C testsuite for your favorite ABI variant)?  TIA.

Ok if it passes testing.

Thanks,
Richard.

>
>        PR tree-optimization/51315
>        * tree-sra.c (tree_non_aligned_mem_for_access_p): New predicate.
>        (build_accesses_from_assign): Use it instead of tree_non_aligned_mem_p.
>
>
>        * gcc.c-torture/execute/20120104-1.c: New test.
>
>
> --
> Eric Botcazou
diff mbox

Patch

Index: tree-sra.c
===================================================================
--- tree-sra.c	(revision 182780)
+++ tree-sra.c	(working copy)
@@ -1095,6 +1095,25 @@  tree_non_aligned_mem_p (tree exp, unsign
   return false;
 }
 
+/* Return true if EXP is a memory reference less aligned than what the access
+   ACC would require.  This is invoked only on strict-alignment targets.  */
+
+static bool
+tree_non_aligned_mem_for_access_p (tree exp, struct access *acc)
+{
+  unsigned int acc_align;
+
+  /* The alignment of the access is that of its expression.  However, it may
+     have been artificially increased, e.g. by a local alignment promotion,
+     so we cap it to the alignment of the type of the base, on the grounds
+     that valid sub-accesses cannot be more aligned than that.  */
+  acc_align = get_object_alignment (acc->expr);
+  if (acc->base && acc_align > TYPE_ALIGN (TREE_TYPE (acc->base)))
+    acc_align = TYPE_ALIGN (TREE_TYPE (acc->base));
+
+  return tree_non_aligned_mem_p (exp, acc_align);
+}
+
 /* Scan expressions occuring in STMT, create access structures for all accesses
    to candidates for scalarization and remove those candidates which occur in
    statements or expressions that prevent them from being split apart.  Return
@@ -1123,8 +1142,7 @@  build_accesses_from_assign (gimple stmt)
   if (lacc)
     {
       lacc->grp_assignment_write = 1;
-      if (STRICT_ALIGNMENT
-	  && tree_non_aligned_mem_p (rhs, get_object_alignment (lhs)))
+      if (STRICT_ALIGNMENT && tree_non_aligned_mem_for_access_p (rhs, lacc))
         lacc->grp_unscalarizable_region = 1;
     }
 
@@ -1134,8 +1152,7 @@  build_accesses_from_assign (gimple stmt)
       if (should_scalarize_away_bitmap && !gimple_has_volatile_ops (stmt)
 	  && !is_gimple_reg_type (racc->type))
 	bitmap_set_bit (should_scalarize_away_bitmap, DECL_UID (racc->base));
-      if (STRICT_ALIGNMENT
-	  && tree_non_aligned_mem_p (lhs, get_object_alignment (rhs)))
+      if (STRICT_ALIGNMENT && tree_non_aligned_mem_for_access_p (lhs, racc))
         racc->grp_unscalarizable_region = 1;
     }