This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, PR 44133] Preserve sane warnings when deleting loads in SRA
- From: Richard Guenther <rguenther at suse dot de>
- To: Martin Jambor <mjambor at suse dot cz>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jakub Jelinek <jakub at redhat dot com>
- Date: Mon, 17 May 2010 11:28:05 +0200 (CEST)
- Subject: Re: [PATCH, PR 44133] Preserve sane warnings when deleting loads in SRA
- References: <20100516210305.GA3421@alvy.suse.cz>
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