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]

Re: [PATCH, PR 44133] Preserve sane warnings when deleting loads in SRA


On Sun, 16 May 2010, Martin Jambor wrote:

> Hi,
> 
> this is a fix for PR 44133.  We emit a sane warning for uninitialized
> loads by replacing the uses of the load by a default definition of a
> SRA replacement rather than of the (usually temporary) variable that
> we load into.  
> 
> As it happens, in these situations there would not normally be a
> replacement created so we create one late and since it does not have
> to be marked for renaming so we don't.  There is also a new access
> function to assert these two approaches to replacement creation do not
> mix.
> 
> If you look at the testcase you will find out tat the warning is
> issued for a wrong line.  This also happens in 4.4 and so is a
> diferent bug that I did not attempt to address.
> 
> Bootstrapped and tested on x86_64-linux, OK for trunk?

Are you sure that it helps even when you add another memory store
before the access in the testcase, like

+ /* { dg-do compile } */
+ /* { dg-options "-O2 -Wall" } */
+
+ struct S { int i, j; };
+
int x;
+ int foo (int l)
+ {
+   struct S s;
x = 0;
+   s.j = l - 22;   /* { dg-warning ".s\.i. is used uninitialized" } */
+   return s.i + s.j;
+ }

?  How does the function look like after SRA with your patch?

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> 2010-05-15  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.
> 	* testsuite/gcc.dg/tree-ssa/sra-9.c: Make the scan regular
> 	expression more precise.
> 
> 
> Index: mine/gcc/tree-sra.c
> ===================================================================
> *** mine.orig/gcc/tree-sra.c
> --- mine/gcc/tree-sra.c
> *************** sort_and_splice_var_accesses (tree var)
> *** 1586,1599 ****
>      ACCESS->replacement.  */
>   
>   static tree
> ! create_access_replacement (struct access *access)
>   {
>     tree repl;
>   
>     repl = create_tmp_var (access->type, "SR");
>     get_var_ann (repl);
>     add_referenced_var (repl);
> !   mark_sym_for_renaming (repl);
>   
>     if (!access->grp_partial_lhs
>         && (TREE_CODE (access->type) == COMPLEX_TYPE
> --- 1586,1600 ----
>      ACCESS->replacement.  */
>   
>   static tree
> ! 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);
> !   if (rename)
> !     mark_sym_for_renaming (repl);
>   
>     if (!access->grp_partial_lhs
>         && (TREE_CODE (access->type) == COMPLEX_TYPE
> *************** get_access_replacement (struct access *a
> *** 1669,1678 ****
>     gcc_assert (access->grp_to_be_replaced);
>   
>     if (!access->replacement_decl)
> !     access->replacement_decl = create_access_replacement (access);
>     return access->replacement_decl;
>   }
>   
>   /* Build a subtree of accesses rooted in *ACCESS, and move the pointer in the
>      linked list along the way.  Stop when *ACCESS is NULL or the access pointed
>      to it is not "within" the root.  */
> --- 1670,1693 ----
>     gcc_assert (access->grp_to_be_replaced);
>   
>     if (!access->replacement_decl)
> !     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;
> + }
> + 
> + 
>   /* Build a subtree of accesses rooted in *ACCESS, and move the pointer in the
>      linked list along the way.  Stop when *ACCESS is NULL or the access pointed
>      to it is not "within" the root.  */
> *************** sra_modify_constructor_assign (gimple *s
> *** 2507,2535 ****
>   }
>   
>   /* 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_reg (TREE_TYPE (decl), "SR");
>   
> !       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);
> --- 2522,2542 ----
>   }
>   
>   /* Create a new suitable default definition SSA_NAME and replace all uses of
> !    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, struct access *racc)
>   {
> !   tree repl, decl;
>   
> !   decl = get_unrenamed_access_replacement (racc);
> ! 
> !   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);
> *************** sra_modify_assign (gimple *stmt, gimple_
> *** 2717,2723 ****
>   					     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);
> --- 2724,2730 ----
>   					     false, false);
>   		  gcc_assert (*stmt == gsi_stmt (*gsi));
>   		  if (TREE_CODE (lhs) == SSA_NAME)
> ! 		    replace_uses_with_default_def_ssa_name (lhs, racc);
>   
>   		  unlink_stmt_vdef (*stmt);
>   		  gsi_remove (gsi, true);
> Index: mine/gcc/testsuite/gcc.dg/tree-ssa/pr44133.c
> ===================================================================
> *** /dev/null
> --- mine/gcc/testsuite/gcc.dg/tree-ssa/pr44133.c
> ***************
> *** 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: mine/gcc/testsuite/gcc.dg/tree-ssa/sra-9.c
> ===================================================================
> *** mine.orig/gcc/testsuite/gcc.dg/tree-ssa/sra-9.c
> --- mine/gcc/testsuite/gcc.dg/tree-ssa/sra-9.c
> *************** int foo (int d)
> *** 16,20 ****
>   }
>   
>   /* There should be no reference to s.b.  */
> ! /* { dg-final { scan-tree-dump-times "s\.b" 0 "optimized"} } */
>   /* { dg-final { cleanup-tree-dump "optimized" } } */
> --- 16,20 ----
>   }
>   
>   /* There should be no reference to s.b.  */
> ! /* { dg-final { scan-tree-dump-times "= s\.b" 0 "optimized"} } */
>   /* { dg-final { cleanup-tree-dump "optimized" } } */
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex


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