| Message ID | 5616D66A.7010906@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 8, 2015 at 10:47 PM, Jeff Law <law@redhat.com> wrote: > And other minor leak. This time in tree-stdarg. Unlike other cases, we're > dropping just the virtual definition, other definitions on the statement > need to be preserved (they're going to be re-used). Additionally, this one > is missing the call to unlink_stmt_vdef. > > Like other cases, I've got a minimized test, but no good way to add it to > the testsuite right now. > > Bootstrapped and regression tested on x86_64-linux-gnu. Installed on the > trunk. > > Jeff > > commit 4d303443cc66bf32f3f045014dd22f0e475f0d50 > Author: Jeff Law <law@redhat.com> > Date: Thu Oct 8 14:46:03 2015 -0600 > > [PATCH] [3/n] Fix minor SSA_NAME leaks > > * tree-stdarg.c (expand_ifn_va_arg_1): Add missing call to > unlink_stmt_vdef and release_ssa_name_fn. > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog > index 64309c1..31e2f30 100644 > --- a/gcc/ChangeLog > +++ b/gcc/ChangeLog > @@ -1,5 +1,8 @@ > 2015-10-08 Jeff Law <law@redhat.com> > > + * tree-stdarg.c (expand_ifn_va_arg_1): Add missing call to > + unlink_stmt_vdef and release_ssa_name_fn. > + > * tree-ssa-dse.c (dse_optimize_stmt): Add missing call to > release_defs. > > diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c > index d69fa06..3e6d98c 100644 > --- a/gcc/tree-stdarg.c > +++ b/gcc/tree-stdarg.c > @@ -1080,6 +1080,8 @@ expand_ifn_va_arg_1 (function *fun) > > /* Remove the IFN_VA_ARG gimple_call. It's the last stmt in the > bb. */ > + unlink_stmt_vdef (stmt); > + release_ssa_name_fn (fun, gimple_vdef (stmt)); I'd prefer a release_defs () call here, it works well in combination with unlink_stmt_vdef. > gsi_remove (&i, true); > gcc_assert (gsi_end_p (i)); > >
On 10/09/2015 03:37 AM, Richard Biener wrote: > On Thu, Oct 8, 2015 at 10:47 PM, Jeff Law <law@redhat.com> wrote: >> And other minor leak. This time in tree-stdarg. Unlike other cases, we're >> dropping just the virtual definition, other definitions on the statement >> need to be preserved (they're going to be re-used). Additionally, this one >> is missing the call to unlink_stmt_vdef. >> >> Like other cases, I've got a minimized test, but no good way to add it to >> the testsuite right now. >> >> Bootstrapped and regression tested on x86_64-linux-gnu. Installed on the >> trunk. >> >> Jeff >> >> commit 4d303443cc66bf32f3f045014dd22f0e475f0d50 >> Author: Jeff Law <law@redhat.com> >> Date: Thu Oct 8 14:46:03 2015 -0600 >> >> [PATCH] [3/n] Fix minor SSA_NAME leaks >> >> * tree-stdarg.c (expand_ifn_va_arg_1): Add missing call to >> unlink_stmt_vdef and release_ssa_name_fn. >> >> diff --git a/gcc/ChangeLog b/gcc/ChangeLog >> index 64309c1..31e2f30 100644 >> --- a/gcc/ChangeLog >> +++ b/gcc/ChangeLog >> @@ -1,5 +1,8 @@ >> 2015-10-08 Jeff Law <law@redhat.com> >> >> + * tree-stdarg.c (expand_ifn_va_arg_1): Add missing call to >> + unlink_stmt_vdef and release_ssa_name_fn. >> + >> * tree-ssa-dse.c (dse_optimize_stmt): Add missing call to >> release_defs. >> >> diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c >> index d69fa06..3e6d98c 100644 >> --- a/gcc/tree-stdarg.c >> +++ b/gcc/tree-stdarg.c >> @@ -1080,6 +1080,8 @@ expand_ifn_va_arg_1 (function *fun) >> >> /* Remove the IFN_VA_ARG gimple_call. It's the last stmt in the >> bb. */ >> + unlink_stmt_vdef (stmt); >> + release_ssa_name_fn (fun, gimple_vdef (stmt)); > > I'd prefer a release_defs () call here, it works well in combination with > unlink_stmt_vdef. I would too, but the LHS of the original call is re-used in the lowered statements. See : lhs = gimple_call_lhs (stmt); if (lhs != NULL_TREE) { unsigned int nargs = gimple_call_num_args (stmt); gcc_assert (useless_type_conversion_p (TREE_TYPE (lhs), type)); if (nargs == 3) { /* We've transported the size of with WITH_SIZE_EXPR here as the last argument of the internal fn call. Now reinstate it. */ tree size = gimple_call_arg (stmt, nargs - 1); expr = build2 (WITH_SIZE_EXPR, TREE_TYPE (expr), expr, size); } /* We use gimplify_assign here, rather than gimple_build_assign, because gimple_assign knows how to deal with variable-sized types. */ gimplify_assign (lhs, expr, &pre); } My first preference is always to release everything. If I'm not doing so it's because of something like this. jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 64309c1..31e2f30 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,8 @@ 2015-10-08 Jeff Law <law@redhat.com> + * tree-stdarg.c (expand_ifn_va_arg_1): Add missing call to + unlink_stmt_vdef and release_ssa_name_fn. + * tree-ssa-dse.c (dse_optimize_stmt): Add missing call to release_defs. diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c index d69fa06..3e6d98c 100644 --- a/gcc/tree-stdarg.c +++ b/gcc/tree-stdarg.c @@ -1080,6 +1080,8 @@ expand_ifn_va_arg_1 (function *fun) /* Remove the IFN_VA_ARG gimple_call. It's the last stmt in the bb. */ + unlink_stmt_vdef (stmt); + release_ssa_name_fn (fun, gimple_vdef (stmt)); gsi_remove (&i, true); gcc_assert (gsi_end_p (i));
And other minor leak. This time in tree-stdarg. Unlike other cases, we're dropping just the virtual definition, other definitions on the statement need to be preserved (they're going to be re-used). Additionally, this one is missing the call to unlink_stmt_vdef. Like other cases, I've got a minimized test, but no good way to add it to the testsuite right now. Bootstrapped and regression tested on x86_64-linux-gnu. Installed on the trunk. Jeff commit 4d303443cc66bf32f3f045014dd22f0e475f0d50 Author: Jeff Law <law@redhat.com> Date: Thu Oct 8 14:46:03 2015 -0600 [PATCH] [3/n] Fix minor SSA_NAME leaks * tree-stdarg.c (expand_ifn_va_arg_1): Add missing call to unlink_stmt_vdef and release_ssa_name_fn.