This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 7/29/19 11:59 AM, Richard Biener wrote:
> On Sun, Jul 28, 2019 at 4:57 PM Martin Liška <mliska@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]