Re: [PATCH] Detect not-cloned new/delete operators in DCE.

["Martin Liška" <mliska@suse.cz>]

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

On 8/6/19 5:44 PM, Marc Glisse wrote:
> On Tue, 6 Aug 2019, Martin Liška wrote:
> 
>> Anyway, I'm sending patch that considers only such new/delete operators
>> that are not a clone of an original type. That should make the current
>> DCE code more solid.
> 
> DECL_IS_REPLACEABLE_OPERATOR_NEW_P seems to have been replaced with DECL_IS_OPERATOR_NEW_P. Is that on purpose?

Whoops, that was not intentional, thanks for heads up.

> 
> I like your cleanup of having a single function to decide if this is the kind of operator new/delete DCE can handle, but you may have introduced long lines in the substitution.
> 

Long lines should be fixed now as well.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

[-- Attachment #2: 0001-Detect-not-cloned-new-delete-operators-in-DCE.patch --]
[-- Type: text/x-patch, Size: 7245 bytes --]

From 6ccff415a8931558cf7f1a01b34daec999e9f5af Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Tue, 6 Aug 2019 16:14:48 +0200
Subject: [PATCH] Detect not-cloned new/delete operators in DCE.

gcc/ChangeLog:

2019-08-06  Martin Liska  <mliska@suse.cz>

	* gimple.c (gimple_call_operator_delete_p): Remove.
	* gimple.h (gimple_call_operator_delete_p): Likewise.
	* tree-ssa-dce.c (operator_new_candidate_p): New.
	(operator_delete_candidate_p): Likewise.
	(mark_stmt_if_obviously_necessary): Use operator_new_candidate_p
	and operator_delete_candidate_p in order to detect operators
	that are not not clones.
	(mark_all_reaching_defs_necessary_1): Likewise.
	(propagate_necessity): Likewise.
	(eliminate_unnecessary_stmts): Likewise.
---
 gcc/gimple.c       | 12 ---------
 gcc/gimple.h       |  1 -
 gcc/tree-ssa-dce.c | 63 ++++++++++++++++++++++++++++------------------
 3 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/gcc/gimple.c b/gcc/gimple.c
index 633ef512a19..684b8831b4d 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2707,18 +2707,6 @@ gimple_builtin_call_types_compatible_p (const gimple *stmt, tree fndecl)
   return true;
 }
 
-/* Return true when STMT is operator delete call.  */
-
-bool
-gimple_call_operator_delete_p (const gcall *stmt)
-{
-  tree fndecl;
-
-  if ((fndecl = gimple_call_fndecl (stmt)) != NULL_TREE)
-    return DECL_IS_OPERATOR_DELETE_P (fndecl);
-  return false;
-}
-
 /* Return true when STMT is builtins call.  */
 
 bool
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 55f5d0d33d9..7a1e1f49099 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1548,7 +1548,6 @@ extern alias_set_type gimple_get_alias_set (tree);
 extern bool gimple_ior_addresses_taken (bitmap, gimple *);
 extern bool gimple_builtin_call_types_compatible_p (const gimple *, tree);
 extern combined_fn gimple_call_combined_fn (const gimple *);
-extern bool gimple_call_operator_delete_p (const gcall *);
 extern bool gimple_call_builtin_p (const gimple *);
 extern bool gimple_call_builtin_p (const gimple *, enum built_in_class);
 extern bool gimple_call_builtin_p (const gimple *, enum built_in_function);
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index afb7bd9dedc..66eb085b65f 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -114,6 +114,25 @@ static bool cfg_altered;
 /* When non-NULL holds map from basic block index into the postorder.  */
 static int *bb_postorder;
 
+/* Return true when FNDECL is a new operator and not a clone.  */
+
+static bool
+operator_new_candidate_p (tree fndecl)
+{
+  return (fndecl != NULL_TREE
+	  && DECL_IS_REPLACEABLE_OPERATOR_NEW_P (fndecl)
+	  && DECL_ABSTRACT_ORIGIN (fndecl) == NULL_TREE);
+}
+
+/* Return true when FNDECL is a delete operator and not a clone.  */
+
+static bool
+operator_delete_candidate_p (tree fndecl)
+{
+  return (fndecl != NULL_TREE
+	  && DECL_IS_OPERATOR_DELETE_P (fndecl)
+	  && DECL_ABSTRACT_ORIGIN (fndecl) == NULL_TREE);
+}
 
 /* True if we should treat any stmt with a vdef as necessary.  */
 
@@ -248,7 +267,7 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
 
 	if (callee != NULL_TREE
 	    && flag_allocation_dce
-	    && DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee))
+	    && operator_new_candidate_p (callee))
 	  return;
 
 	/* Most, but not all function calls are required.  Function calls that
@@ -613,8 +632,8 @@ mark_all_reaching_defs_necessary_1 (ao_ref *ref ATTRIBUTE_UNUSED,
 	  }
 
       if (callee != NULL_TREE
-	  && (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee)
-	      || DECL_IS_OPERATOR_DELETE_P (callee)))
+	  && (operator_new_candidate_p (callee)
+	      || operator_delete_candidate_p (callee)))
 	return false;
     }
 
@@ -800,21 +819,18 @@ propagate_necessity (bool aggressive)
 	     which feed this statement's uses as necessary.  */
 	  ssa_op_iter iter;
 	  tree use;
+	  tree fndecl;
 
 	  /* If this is a call to free which is directly fed by an
 	     allocation function do not mark that necessary through
 	     processing the argument.  */
 	  bool is_delete_operator
 	    = (is_gimple_call (stmt)
-	       && gimple_call_operator_delete_p (as_a <gcall *> (stmt)));
+	       && (fndecl = gimple_call_fndecl (as_a <gcall *> (stmt)))
+	       && operator_delete_candidate_p (fndecl));
 	  if (is_delete_operator
 	      || gimple_call_builtin_p (stmt, BUILT_IN_FREE))
 	    {
-	      /* It can happen that a user delete operator has the pointer
-		 argument optimized out already.  */
-	      if (gimple_call_num_args (stmt) == 0)
-		continue;
-
 	      tree ptr = gimple_call_arg (stmt, 0);
 	      gimple *def_stmt;
 	      tree def_callee;
@@ -827,7 +843,7 @@ propagate_necessity (bool aggressive)
 		       && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_ALIGNED_ALLOC
 			   || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
 			   || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
-		      || DECL_IS_REPLACEABLE_OPERATOR_NEW_P (def_callee)))
+		      || operator_new_candidate_p (def_callee)))
 		{
 		  /* Delete operators can have alignment and (or) size as next
 		     arguments.  When being a SSA_NAME, they must be marked
@@ -900,8 +916,8 @@ propagate_necessity (bool aggressive)
 		continue;
 
 	      if (callee != NULL_TREE
-		  && (DECL_IS_REPLACEABLE_OPERATOR_NEW_P (callee)
-		      || DECL_IS_OPERATOR_DELETE_P (callee)))
+		  && (operator_new_candidate_p (callee)
+		      || operator_delete_candidate_p (callee)))
 		continue;
 
 	      /* Calls implicitly load from memory, their arguments
@@ -1313,6 +1329,7 @@ eliminate_unnecessary_stmts (void)
       auto_bitmap debug_seen;
       for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi = psi)
 	{
+	  tree fndecl;
 	  stmt = gsi_stmt (gsi);
 
 	  psi = gsi;
@@ -1326,20 +1343,16 @@ eliminate_unnecessary_stmts (void)
 	  if (gimple_plf (stmt, STMT_NECESSARY)
 	      && (gimple_call_builtin_p (stmt, BUILT_IN_FREE)
 		  || (is_gimple_call (stmt)
-		      && gimple_call_operator_delete_p (as_a <gcall *> (stmt)))))
+		      && (fndecl = gimple_call_fndecl (as_a <gcall *> (stmt)))
+		      && operator_delete_candidate_p (fndecl))))
 	    {
-	      /* It can happen that a user delete operator has the pointer
-		 argument optimized out already.  */
-	      if (gimple_call_num_args (stmt) > 0)
+	      tree ptr = gimple_call_arg (stmt, 0);
+	      if (TREE_CODE (ptr) == SSA_NAME)
 		{
-		  tree ptr = gimple_call_arg (stmt, 0);
-		  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);
-		    }
+		  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);
 		}
 	    }
 
@@ -1394,7 +1407,7 @@ eliminate_unnecessary_stmts (void)
 			       && DECL_FUNCTION_CODE (call) != BUILT_IN_CALLOC
 			       && !ALLOCA_FUNCTION_CODE_P
 			       (DECL_FUNCTION_CODE (call))))
-			  && !DECL_IS_REPLACEABLE_OPERATOR_NEW_P (call))))
+			  && !operator_new_candidate_p (call))))
 		{
 		  something_changed = true;
 		  if (dump_file && (dump_flags & TDF_DETAILS))
-- 
2.22.0