[ was: Re: [PATCH] Don't create superfluous parm in expand_omp_taskreg ]
On 24/09/15 11:02, Thomas Schwinge wrote:
Hi Tom!
On Thu, 24 Sep 2015 08:36:27 +0200, Tom de Vries<tom_devr...@mentor.com> wrote:
>On 24/09/15 08:23, Thomas Schwinge wrote:
> >On Tue, 11 Aug 2015 20:53:39 +0200, Tom de Vries<tom_devr...@mentor.com>
wrote:
> >>Don't create superfluous parm in expand_omp_taskreg
> >>
> >>2015-08-11 Tom de Vries<t...@codesourcery.com>
> >>
> >> * omp-low.c (expand_omp_taskreg): If in ssa, set rhs of parcopy stmt to
> >> parm_decl, rather than generating a dummy default def in cfun.
> >> * tree-cfg.c (replace_ssa_name): Assume no default defs. Make sure
> >> ssa_name from cfun and child_fn do not share a stmt as def stmt.
> >> (move_stmt_op): Handle PARM_DECl.
> >> (gather_ssa_name_hash_map_from): New function.
> >> (move_sese_region_to_fn): Add default defs for function params, and add
> >> them to vars_map. Release copied ssa names.
> >> * tree-cfg.h (gather_ssa_name_hash_map_from): Declare.
> >
> >Do I understand correct that with this change present on trunk (which I'm
> >currently merging into gomp-4_0-branch), the changes you've earlier done
> >on gomp-4_0-branch to gcc/omp-low.c:release_dangling_ssa_names,
> >gcc/tree-cfg.c:replace_ssa_name, should now be reverted? That is, how
> >much of the following patches can be reverted now (listed backwards in
> >time)?
>
>indeed, in the above commit we release the dangling ssa names in
>move_sese_region_to_fn. So after committing this patch to the
>gomp-4_0-branch, the call to release_dangling_ssa_names is no longer
>necessary, and the function release_dangling_ssa_names can be removed.
<SNIP>
<tschwinge> Well, I'm asking because in my merge tree, I'm running
into an assertion that you added there -- not sure yet whether I've
done something wrong, though.
The source of the problem was in expand_omp_target, which needed similar
changes as expand_omp_taskreg got in the "Don't create superfluous parm
in expand_omp_taskreg" patch.
Now that the merge (
https://gcc.gnu.org/viewcvs/gcc/branches/gomp-4_0-branch/gcc/omp-low.c?limit_changes=0&r1=228091&r2=228090&pathrev=228091
) contains that change, I've committed these two patches to gomp-4_0-branch:
- Revert "Fix release_dangling_ssa_names"
(Reverting an earlier attempt to handle the
release_dangling_ssa_names TODO, which was committed to the
gomp-4_0-branch)
- Remove release_dangling_ssa_names
Thanks,
- Tom
Revert "Fix release_dangling_ssa_names"
2015-09-24 Tom de Vries <t...@codesourcery.com>
Revert:
2015-08-05 Tom de Vries <t...@codesourcery.com>
* omp-low.c (release_dangling_ssa_names): Release SSA_NAMEs with NULL
def stmt.
* tree-cfg.c (replace_ssa_name): Don't move default def nops. Set def
stmt of unused SSA_NAME to NULL.
---
gcc/omp-low.c | 33 ++++++++++++++++++++++++---------
gcc/tree-cfg.c | 12 ------------
2 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index a72db53..04a60ab 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -10541,10 +10541,11 @@ make_pass_expand_omp (gcc::context *ctxt)
return new pass_expand_omp (ctxt);
}
-/* After running pass_expand_omp_ssa to expand the oacc kernels directive, we
- are left in the original function with anonymous SSA_NAMEs, with a NULL
- defining statement. This function finds those SSA_NAMEs and releases
- them. */
+/* After running pass_expand_omp_ssa to expand the oacc kernels
+ directive, we are left in the original function with anonymous
+ SSA_NAMEs, with a defining statement that has been deleted. This
+ pass finds those SSA_NAMEs and releases them.
+ TODO: Either fix this elsewhere, or make the fix unnecessary. */
static void
release_dangling_ssa_names (void)
@@ -10559,12 +10560,26 @@ release_dangling_ssa_names (void)
gimple *stmt = SSA_NAME_DEF_STMT (name);
if (stmt != NULL)
continue;
+ bool found = false;
- release_ssa_name (name);
- gcc_assert (SSA_NAME_IN_FREE_LIST (name));
- if (dump_file
- && (dump_flags & TDF_DETAILS))
- fprintf (dump_file, "Released dangling ssa name %u\n", i);
+ ssa_op_iter op_iter;
+ def_operand_p def_p;
+ FOR_EACH_PHI_OR_STMT_DEF (def_p, stmt, op_iter, SSA_OP_ALL_DEFS)
+ {
+ tree def = DEF_FROM_PTR (def_p);
+ if (def == name)
+ {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ {
+ if (dump_file)
+ fprintf (dump_file, "Released dangling ssa name %u\n", i);
+ release_ssa_name (name);
+ }
}
}
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index cd7a4b4..a3c3b20 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -6422,12 +6422,6 @@ replace_ssa_name (tree name, hash_map<tree, tree> *vars_map,
{
gcc_assert (!SSA_NAME_IS_DEFAULT_DEF (name));
replace_by_duplicate_decl (&decl, vars_map, to_context);
- /* If name is a default def, then we don't move the defining stmt
- (which is a nop). Because (1) the nop doesn't belong to the sese
- region, and (2) while setting the def stmt of name to NULL would
- trigger release_ssa_name in release_dangling_ssa_names, it wouldn't
- be released since it's a default def, and subsequently cause an
- ssa verification failure. */
new_name = make_ssa_name_fn (DECL_STRUCT_FUNCTION (to_context),
decl, SSA_NAME_DEF_STMT (name));
if (SSA_NAME_IS_DEFAULT_DEF (name))
@@ -6443,12 +6437,6 @@ replace_ssa_name (tree name, hash_map<tree, tree> *vars_map,
SSA_NAME_DEF_STMT (name) = NULL;
vars_map->put (name, new_name);
-
- if (!SSA_NAME_IS_DEFAULT_DEF (name))
- /* The statement has been moved to the child function. It no longer
- defines name in the original function. Mark the def stmt NULL, and
- let release_dangling_ssa_names deal with it. */
- SSA_NAME_DEF_STMT (name) = NULL;
}
else
new_name = *loc;
--
1.9.1
Remove release_dangling_ssa_names
2015-09-24 Tom de Vries <t...@codesourcery.com>
* omp-low.c (release_dangling_ssa_names): Remove.
(pass_omp_expand_ssa::execute): Remove call to
release_dangling_ssa_names.
---
gcc/omp-low.c | 46 +---------------------------------------------
1 file changed, 1 insertion(+), 45 deletions(-)
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 04a60ab..6bdfaa2 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -10541,48 +10541,6 @@ make_pass_expand_omp (gcc::context *ctxt)
return new pass_expand_omp (ctxt);
}
-/* After running pass_expand_omp_ssa to expand the oacc kernels
- directive, we are left in the original function with anonymous
- SSA_NAMEs, with a defining statement that has been deleted. This
- pass finds those SSA_NAMEs and releases them.
- TODO: Either fix this elsewhere, or make the fix unnecessary. */
-
-static void
-release_dangling_ssa_names (void)
-{
- unsigned int i;
- for (i = 1; i < num_ssa_names; ++i)
- {
- tree name = ssa_name (i);
- if (name == NULL_TREE)
- continue;
-
- gimple *stmt = SSA_NAME_DEF_STMT (name);
- if (stmt != NULL)
- continue;
- bool found = false;
-
- ssa_op_iter op_iter;
- def_operand_p def_p;
- FOR_EACH_PHI_OR_STMT_DEF (def_p, stmt, op_iter, SSA_OP_ALL_DEFS)
- {
- tree def = DEF_FROM_PTR (def_p);
- if (def == name)
- {
- found = true;
- break;
- }
- }
-
- if (!found)
- {
- if (dump_file)
- fprintf (dump_file, "Released dangling ssa name %u\n", i);
- release_ssa_name (name);
- }
- }
-}
-
namespace {
const pass_data pass_data_expand_omp_ssa =
@@ -10613,9 +10571,7 @@ public:
}
virtual unsigned int execute (function *)
{
- unsigned res = execute_expand_omp ();
- release_dangling_ssa_names ();
- return res;
+ return execute_expand_omp ();
}
opt_pass * clone () { return new pass_expand_omp_ssa (m_ctxt); }
--
1.9.1
