[4.5, PR 44133] Request to backport two patches to the 4.5 branch
Martin Jambor
mjambor@suse.cz
Mon Jun 28 17:18:00 GMT 2010
Ping. (I have re-bootrapped and re-tested on today's branch).
Thanks,
Martin
On Wed, Jun 16, 2010 at 06:47:41PM +0200, Martin Jambor wrote:
> 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);
More information about the Gcc-patches
mailing list