Re: [PATCH] [3/n] Fix minor SSA_NAME leaks

[Jeff Law <law at redhat dot com>]

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