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]

[4.5, PR 44133] Request to backport two patches to the 4.5 branch


Hi,

the fix for PR 44133 I committed to trunk depends on a previous 4.6
only patch http://gcc.gnu.org/ml/gcc-patches/2010-04/msg00440.html
which is an enhancement to SRA deleting register-type loads from parts
of aggregates that are known not to be initialized.

The easiest way to fix the bug on the branch would therefore be to
backport the enhancement and then the fix on top of it.  I believe the
both changes is sufficiently small to ask the release managers for a
permission to do just that.

BTW, the bug manifests itself differently from what was happening
since the enhancement was committed, there simply is no warning
emitted which is a regression from 4.4.

The 4.5 versions of the two patches are below.  Right now I am in the
process of bootstrapping and testing them.  Is it OK to commit them to
the 4.5 branch if there are no issues?

Thanks,

Martin


2010-06-16  Martin Jambor  <mjambor@suse.cz>

	Backport from mainline
	2010-04-13  Martin Jambor  <mjambor@suse.cz>

        * tree-sra.c (replace_uses_with_default_def_ssa_name): New function.
        (sra_modify_assign): Delete stmts loading dead data even if racc has no
        children.  Call replace_uses_with_default_def_ssa_name to handle
        SSA_NAES on lhs.

        * testsuite/gcc.dg/tree-ssa/sra-9.c: New test.

Index: gcc-4_5-branch/gcc/testsuite/gcc.dg/tree-ssa/sra-9.c
===================================================================
--- /dev/null
+++ gcc-4_5-branch/gcc/testsuite/gcc.dg/tree-ssa/sra-9.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fdump-tree-optimized" } */
+
+struct S
+{
+  int a, b, c;
+  int z[20];
+};
+
+int foo (int d)
+{
+  struct S s;
+
+  s.a = d;
+  return s.a + s.b;
+}
+
+/* There should be no reference to s.b.  */
+/* { dg-final { scan-tree-dump-times "= s\.b" 0 "optimized"} } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
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
@@ -2555,6 +2555,37 @@ sra_modify_constructor_assign (gimple *s
     }
 }
 
+/* Create a new suitable default definition SSA_NAME and replace all uses of
+   SSA with it.  */
+
+static void
+replace_uses_with_default_def_ssa_name (tree ssa)
+{
+  tree repl, decl = SSA_NAME_VAR (ssa);
+  if (TREE_CODE (decl) == PARM_DECL)
+    {
+      tree tmp = create_tmp_var (TREE_TYPE (decl), "SR");
+      if (TREE_CODE (TREE_TYPE (tmp)) == COMPLEX_TYPE
+	  || TREE_CODE (TREE_TYPE (tmp)) == VECTOR_TYPE)
+	DECL_GIMPLE_REG_P (tmp) = 1;
+
+      get_var_ann (tmp);
+      add_referenced_var (tmp);
+      repl = make_ssa_name (tmp, gimple_build_nop ());
+      set_default_def (tmp, repl);
+    }
+  else
+    {
+      repl = gimple_default_def (cfun, decl);
+      if (!repl)
+	{
+	  repl = make_ssa_name (decl, gimple_build_nop ());
+	  set_default_def (decl, repl);
+	}
+    }
+
+  replace_uses_by (ssa, repl);
+}
 
 /* Callback of scan_function to process assign statements.  It examines both
    sides of the statement, replaces them with a scalare replacement if there is
@@ -2730,26 +2761,28 @@ sra_modify_assign (gimple *stmt, gimple_
 	}
       else
 	{
-	  if (access_has_children_p (racc))
+	  if (racc)
 	    {
-	      if (!racc->grp_unscalarized_data
-		  /* Do not remove SSA name definitions (PR 42704).  */
-		  && TREE_CODE (lhs) != SSA_NAME)
+	      if (!racc->grp_to_be_replaced && !racc->grp_unscalarized_data)
 		{
-		  generate_subtree_copies (racc->first_child, lhs,
-					   racc->offset, 0, 0, gsi,
-					   false, false);
+		  if (racc->first_child)
+		    generate_subtree_copies (racc->first_child, lhs,
+					     racc->offset, 0, 0, gsi,
+					     false, false);
 		  gcc_assert (*stmt == gsi_stmt (*gsi));
+		  if (TREE_CODE (lhs) == SSA_NAME)
+		    replace_uses_with_default_def_ssa_name (lhs);
+
 		  unlink_stmt_vdef (*stmt);
 		  gsi_remove (gsi, true);
 		  sra_stats.deleted++;
 		  return SRA_SA_REMOVED;
 		}
-	      else
+	      else if (racc->first_child)
 		generate_subtree_copies (racc->first_child, lhs,
 					 racc->offset, 0, 0, gsi, false, true);
 	    }
-	  else if (access_has_children_p (lacc))
+	  if (access_has_children_p (lacc))
 	    generate_subtree_copies (lacc->first_child, rhs, lacc->offset,
 				     0, 0, gsi, true, true);
 	}



2010-06-16  Martin Jambor  <mjambor@suse.cz>

	Backport from mainline
	2010-05-17  Martin Jambor  <mjambor@suse.cz>

        PR middle-end/44133
        * tree-sra.c (create_access_replacement): New parameter rename, mark
        the replaement for renaming only when it is true.
        (get_access_replacement): Pass true in the rename parameter of
        create_access_replacement.
        (get_unrenamed_access_replacement): New function.
        (replace_uses_with_default_def_ssa_name): New parameter racc, get the
        replacement declaration from it.

        * testsuite/gcc.dg/tree-ssa/pr44133.c: New test.

Index: gcc-4_5-branch/gcc/testsuite/gcc.dg/tree-ssa/pr44133.c
===================================================================
--- /dev/null
+++ gcc-4_5-branch/gcc/testsuite/gcc.dg/tree-ssa/pr44133.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+
+struct S { int i, j; };
+
+int foo (int l)
+{
+  struct S s;
+  s.j = l - 22;   /* { dg-warning ".s\.i. is used uninitialized" } */
+  return s.i + s.j;
+}
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
@@ -1659,14 +1659,15 @@ sort_and_splice_var_accesses (tree var)
    ACCESS->replacement.  */
 
 static tree
-create_access_replacement (struct access *access)
+create_access_replacement (struct access *access, bool rename)
 {
   tree repl;
 
   repl = create_tmp_var (access->type, "SR");
   get_var_ann (repl);
   add_referenced_var (repl);
-  mark_sym_for_renaming (repl);
+  if (rename)
+    mark_sym_for_renaming (repl);
 
   if (!access->grp_partial_lhs
       && (TREE_CODE (access->type) == COMPLEX_TYPE
@@ -1715,7 +1716,20 @@ get_access_replacement (struct access *a
   gcc_assert (access->grp_to_be_replaced);
 
   if (!access->replacement_decl)
-    access->replacement_decl = create_access_replacement (access);
+    access->replacement_decl = create_access_replacement (access, true);
+  return access->replacement_decl;
+}
+
+/* Return ACCESS scalar replacement, create it if it does not exist yet but do
+   not mark it for renaming.  */
+
+static inline tree
+get_unrenamed_access_replacement (struct access *access)
+{
+  gcc_assert (!access->grp_to_be_replaced);
+
+  if (!access->replacement_decl)
+    access->replacement_decl = create_access_replacement (access, false);
   return access->replacement_decl;
 }
 
@@ -2556,32 +2570,21 @@ sra_modify_constructor_assign (gimple *s
 }
 
 /* Create a new suitable default definition SSA_NAME and replace all uses of
-   SSA with it.  */
+   SSA with it, RACC is access describing the uninitialized part of an
+   aggregate that is being loaded.  */
 
 static void
-replace_uses_with_default_def_ssa_name (tree ssa)
+replace_uses_with_default_def_ssa_name (tree ssa, struct access *racc)
 {
-  tree repl, decl = SSA_NAME_VAR (ssa);
-  if (TREE_CODE (decl) == PARM_DECL)
-    {
-      tree tmp = create_tmp_var (TREE_TYPE (decl), "SR");
-      if (TREE_CODE (TREE_TYPE (tmp)) == COMPLEX_TYPE
-	  || TREE_CODE (TREE_TYPE (tmp)) == VECTOR_TYPE)
-	DECL_GIMPLE_REG_P (tmp) = 1;
-
-      get_var_ann (tmp);
-      add_referenced_var (tmp);
-      repl = make_ssa_name (tmp, gimple_build_nop ());
-      set_default_def (tmp, repl);
-    }
-  else
+  tree repl, decl;
+
+  decl = get_unrenamed_access_replacement (racc);
+
+  repl = gimple_default_def (cfun, decl);
+  if (!repl)
     {
-      repl = gimple_default_def (cfun, decl);
-      if (!repl)
-	{
-	  repl = make_ssa_name (decl, gimple_build_nop ());
-	  set_default_def (decl, repl);
-	}
+      repl = make_ssa_name (decl, gimple_build_nop ());
+      set_default_def (decl, repl);
     }
 
   replace_uses_by (ssa, repl);
@@ -2771,7 +2774,7 @@ sra_modify_assign (gimple *stmt, gimple_
 					     false, false);
 		  gcc_assert (*stmt == gsi_stmt (*gsi));
 		  if (TREE_CODE (lhs) == SSA_NAME)
-		    replace_uses_with_default_def_ssa_name (lhs);
+		    replace_uses_with_default_def_ssa_name (lhs, racc);
 
 		  unlink_stmt_vdef (*stmt);
 		  gsi_remove (gsi, true);


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