Re: [PATCH] Remove also 2nd argument for unused delete operator (PR tree-optimization/91270).

[Martin Liška]

On 7/29/19 11:59 AM, Richard Biener wrote:
> On Sun, Jul 28, 2019 at 4:57 PM Martin Liška <mli...@suse.cz> wrote:
>>
>> Hi.
>>
>> And there's one more patch that deals with delete operator
>> which has a second argument (size). That definition GIMPLE
>> statement of the size must be also properly marked.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
> 
> This isn't a proper fix.  You can't mark a random operand as necessary
> during elimination - this only works in very constraint cases.  Here
> it will break down if the stmt you just marked depends on another stmt
> only used by the stmt you just marked (and thus also not necessary).

Ah, I see.

> 
> The fix is to propagate_necessity where you either have to excempt
> the two-operator delete from handling

We want to process also these delete operators.

> or to mark its second operand
> as necessary despite eventually deleting the call.

Really marking that as necessary? Why?

> You can avoid
> this in case the allocation function used the exact same size argument.

That's not the case as the operator new happens in a loop:

  <bb 5> :
  # iftmp.0_15 = PHI <iftmp.0_21(3), iftmp.0_20(4)>
  _23 = operator new [] (iftmp.0_15);
  _24 = _23;
  MEM[(sizetype *)_24] = _19;
  _26 = _24 + 8;
  _27 = _26;
  _2 = _19 + 18446744073709551615;
  _28 = (long int) _2;

  <bb 6> :
  # _12 = PHI <_27(5), _29(7)>
  # _13 = PHI <_28(5), _30(7)>
  if (_13 < 0)
    goto <bb 8>; [INV]
  else
    goto <bb 7>; [INV]

Btw. is the hunk moved to propagate_necessity still wrong:

diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index cf507fa0453..1c890889932 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -803,7 +803,23 @@ propagate_necessity (bool aggressive)
                           || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
                           || DECL_FUNCTION_CODE (def_callee) == 
BUILT_IN_CALLOC))
                      || DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee)))
-               continue;
+               {
+                 /* Some delete operators have 2 arguments, where
+                    the second argument is size of the deallocated memory.  */
+                 if (gimple_call_num_args (stmt) == 2)
+                   {
+                     tree ptr = gimple_call_arg (stmt, 1);
+                     if (TREE_CODE (ptr) == SSA_NAME)
+                       {
+                         gimple *def_stmt = SSA_NAME_DEF_STMT (ptr);
+                         if (!gimple_nop_p (def_stmt)
+                             && !gimple_plf (def_stmt, STMT_NECESSARY))
+                           gimple_set_plf (stmt, STMT_NECESSARY, false);
+                       }
+                   }
+
+                 continue;
+               }
            }
 
          FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)

Thanks,
Martin

> 
> Richard.
> 
>> Thanks,
>> Martin