diff mbox

[gomp4,committed] Remove release_dangling_ssa_names

Message ID 87612stg1u.fsf@kepler.schwinge.homeip.net
State New
Headers show

Commit Message

Thomas Schwinge Sept. 30, 2015, 9:25 a.m. UTC
Hi Tom!

On Wed, 30 Sep 2015 08:17:04 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:
> [ was: Re: [PATCH] Don't create superfluous parm in expand_omp_taskreg ]
> On 24/09/15 11:02, Thomas Schwinge wrote:
> > On Thu, 24 Sep 2015 08:36:27 +0200, Tom de Vries<Tom_deVries@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_deVries@mentor.com>  wrote:
> >>>> > >>Don't create superfluous parm in expand_omp_taskreg
> >>>> > >>
> >>>> > >>2015-08-11  Tom de Vries<tom@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

Thanks for quickly having provided me with a patch!

> 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.

(For the curious, such a patch is not yet needed on trunk, where
expand_omp_target does not yet need to support the "gimple_in_ssa_p"
case.)

> 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

Don't we also want to commit the following change, which was part of your
trunk r227103 (883f001d2c3672e0674bec71f36a2052734a72cf) commit (and now
shows up as a delta between trunk and gomp-4_0-branch)?



Grüße,
 Thomas

Comments

Tom de Vries Sept. 30, 2015, 9:46 a.m. UTC | #1
On 30/09/15 11:25, Thomas Schwinge wrote:
> Don't we also want to commit the following change, which was part of your
> trunk r227103 (883f001d2c3672e0674bec71f36a2052734a72cf) commit (and now
> shows up as a delta between trunk and gomp-4_0-branch)?
>
> --- gcc/tree-cfg.c
> +++ gcc/tree-cfg.c
> @@ -6424,9 +6424,6 @@ replace_ssa_name (tree name, hash_map<tree, tree> *vars_map,
>   	  replace_by_duplicate_decl (&decl, vars_map, to_context);
>   	  new_name = make_ssa_name_fn (DECL_STRUCT_FUNCTION (to_context),
>   				       decl, SSA_NAME_DEF_STMT (name));
> -	  if (SSA_NAME_IS_DEFAULT_DEF (name))
> -	    set_ssa_default_def (DECL_STRUCT_FUNCTION (to_context),
> -				 decl, new_name);
>   	}
>         else
>   	new_name = copy_ssa_name_fn (DECL_STRUCT_FUNCTION (to_context),

Indeed, that bit is part of the patch "Don't create superfluous parm in 
expand_omp_taskreg", but was dropped in the merge (probably because it 
conflicted with the "Fix release_dangling_ssa_names" patch that I just 
reverted).

So we need to apply it.

Thanks,
- Tom
diff mbox

Patch

--- gcc/tree-cfg.c
+++ gcc/tree-cfg.c
@@ -6424,9 +6424,6 @@  replace_ssa_name (tree name, hash_map<tree, tree> *vars_map,
 	  replace_by_duplicate_decl (&decl, vars_map, to_context);
 	  new_name = make_ssa_name_fn (DECL_STRUCT_FUNCTION (to_context),
 				       decl, SSA_NAME_DEF_STMT (name));
-	  if (SSA_NAME_IS_DEFAULT_DEF (name))
-	    set_ssa_default_def (DECL_STRUCT_FUNCTION (to_context),
-				 decl, new_name);
 	}
       else
 	new_name = copy_ssa_name_fn (DECL_STRUCT_FUNCTION (to_context),