[PATCH] Fix PR44900 (SRA miscompilation)

[Martin Jambor <mjambor at suse dot cz>]

Hi,

the simple patch below fixes PR 44900.  Apparently we rarely happen to
land in the SRA_UDH_LEFT refreshing mode in
load_assign_lhs_subreplacements because cleaning up after it has not
worked for a long time.  But it does require a special combination of
assignments between special unions to trigger, such as the one in the
testcase.

This patch is aginst the 4.5 branch but the one for trunk is exactly
the same.  I have bootstrapped and regression tested both without any
problems.  OK for trunk and the branch?

Thanks,

Martin



2010-07-20  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/44900
	* tree-sra.c (load_assign_lhs_subreplacements): Updated comments.
	(sra_modify_assign): Move gsi to the next statmenent unconditionally.

Index: gcc-4_5-branch/gcc/testsuite/g++.dg/torture/pr44900.C
===================================================================
--- /dev/null
+++ gcc-4_5-branch/gcc/testsuite/g++.dg/torture/pr44900.C
@@ -0,0 +1,91 @@
+/* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-msse" } */
+/* { dg-require-effective-target sse } */
+
+typedef float __m128 __attribute__ ((__vector_size__ (16), __may_alias__));
+typedef float __v4sf __attribute__ ((__vector_size__ (16)));
+
+extern __inline __m128 __attribute__((__gnu_inline__, __always_inline__,
+__artificial__))
+_mm_set_ps (const float __Z, const float __Y, const float __X, const float __W)
+{
+  return __extension__ (__m128)(__v4sf){ __W, __X, __Y, __Z };
+}
+
+struct vec
+{
+        union {
+                __m128 v;
+                float  e[4];
+        };
+
+        static const vec & zero()
+        {
+                static const vec v = _mm_set_ps(0, 0, 0, 0);
+                return v;
+        }
+
+        vec() {}
+        vec(const __m128 & a) : v(a) {}
+
+        operator const __m128&() const { return v; }
+};
+
+struct vec2
+{
+        vec _v1;
+        vec _v2;
+
+        vec2() {}
+        vec2(const vec & a, const vec & b) : _v1(a), _v2(b) {}
+
+        static vec2 load(const float * a)
+        {
+                return vec2(
+                        __builtin_ia32_loadups(&a[0]),
+                        __builtin_ia32_loadups(&a[4]));
+        }
+
+        const vec & v1() const { return _v1; }
+        const vec & v2() const { return _v2; }
+};
+
+extern "C" {
+        int  printf (const char*, ...);
+        void abort(void);
+}
+
+inline bool test_assert( bool is_succeed, const char * file_name, int line_num
+)
+{
+        if ( !is_succeed )
+        {
+                printf("error: %s(%d)\n", file_name, line_num);
+        }
+
+        return is_succeed;
+}
+
+inline bool operator==(const vec & a, const vec & b)
+{ return 0xf == __builtin_ia32_movmskps(__builtin_ia32_cmpeqps(a, b)); }
+
+#define test(x, y) test_assert( (x)==(y), __FILE__, __LINE__ )
+
+int main( int argc, char * argv[] )
+{
+        __attribute__((aligned(16))) float data[] =
+        { 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5 };
+
+        float * p = &data[2];
+        vec2 a;
+
+        a = vec2::load(p);
+
+        vec v1 = a.v1();
+        vec v2 = a.v2();
+
+	if (v2.e[3] != 7.0)
+	  abort();
+
+        return 0;
+}
Index: gcc-4_5-branch/gcc/tree-sra.c
===================================================================
--- gcc-4_5-branch.orig/gcc/tree-sra.c
+++ gcc-4_5-branch/gcc/tree-sra.c
@@ -2445,9 +2445,11 @@ handle_unscalarized_data_in_subtree (str
    (sub)tree.  If that is not possible, refresh the TOP_RACC base aggregate and
    load the accesses from it.  LEFT_OFFSET is the offset of the left whole
    subtree being copied, RIGHT_OFFSET is the same thing for the right subtree.
-   GSI is stmt iterator used for statement insertions.  *REFRESHED is true iff
-   the rhs top aggregate has already been refreshed by contents of its scalar
-   reductions and is set to true if this function has to do it.  */
+   NEW_GSI is stmt iterator used for statement insertions after the original
+   assignment, OLD_GSI is used to insert statements before the assignment.
+   *REFRESHED keeps the information whether we have needed to refresh
+   replacements of the LHS and from which side of the assignments this takes
+   place.  */
 
 static void
 load_assign_lhs_subreplacements (struct access *lacc, struct access *top_racc,
@@ -2747,9 +2749,7 @@ sra_modify_assign (gimple *stmt, gimple_
 					   &orig_gsi, gsi, &refreshed, lhs);
 	  if (refreshed != SRA_UDH_RIGHT)
 	    {
-	      if (*stmt == gsi_stmt (*gsi))
-		gsi_next (gsi);
-
+	      gsi_next (gsi);
 	      unlink_stmt_vdef (*stmt);
 	      gsi_remove (&orig_gsi, true);
 	      sra_stats.deleted++;