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

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

[Jeff Law]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

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

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1039 bytes --]

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));
 	gsi_remove (&i, true);
 	gcc_assert (gsi_end_p (i));
 

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

[Richard Biener]

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));
>
>

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

[Jeff Law]

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