PR85787: Extend malloc_candidate_p to handle multiple phis.

Message ID CAAgBjMmdps8GYfKX9-6BTSBZ_7ci++xJZ8sfB1=DNyqzEUwX+g@mail.gmail.com
State New
Headers show
Series
  • PR85787: Extend malloc_candidate_p to handle multiple phis.
Related show

Commit Message

Prathamesh Kulkarni Aug. 28, 2018, 11:26 a.m.
H
The attached patch extends malloc_candidate_p to handle multiple phis.
There's a lot of noise in the patch because I moved most of
malloc_candidate_p into
new function malloc_candidate_p_1. The only real change is following hunk:

+           gimple *arg_def = SSA_NAME_DEF_STMT (arg);
+           if (is_a<gphi *> (arg_def))
+             {
+               if (!malloc_candidate_p_1 (fun, arg, phi, ipa))
+                   DUMP_AND_RETURN ("nested phi fail")
+               continue;
+             }
+

Which checks recursively that the phi argument is used only within
comparisons against 0
and the phi.

Bootstrapped+tested on x86_64-unknown-linux-gnu.
OK to commit ?

Thanks,
Prathamesh
2018-08-28  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

	PR tree-optimization/85787
	* ipa-pure-const.c (malloc_candidate_p_1): Move most of malloc_candidate_p
	into this function and add support for detecting multiple phis.
	(DUMP_AND_RETURN): Move from malloc_candidate_p into top-level macro.

Comments

Prathamesh Kulkarni Sept. 5, 2018, 10:02 a.m. | #1
On 28 August 2018 at 16:56, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> H

> The attached patch extends malloc_candidate_p to handle multiple phis.

> There's a lot of noise in the patch because I moved most of

> malloc_candidate_p into

> new function malloc_candidate_p_1. The only real change is following hunk:

>

> +           gimple *arg_def = SSA_NAME_DEF_STMT (arg);

> +           if (is_a<gphi *> (arg_def))

> +             {

> +               if (!malloc_candidate_p_1 (fun, arg, phi, ipa))

> +                   DUMP_AND_RETURN ("nested phi fail")

> +               continue;

> +             }

> +

>

> Which checks recursively that the phi argument is used only within

> comparisons against 0

> and the phi.

>

> Bootstrapped+tested on x86_64-unknown-linux-gnu.

> OK to commit ?

ping https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01757.html

Thanks,
Prathamesh
>

> Thanks,

> Prathamesh
Jeff Law Sept. 14, 2018, 5:19 p.m. | #2
On 8/28/18 5:26 AM, Prathamesh Kulkarni wrote:
> H

> The attached patch extends malloc_candidate_p to handle multiple phis.

> There's a lot of noise in the patch because I moved most of

> malloc_candidate_p into

> new function malloc_candidate_p_1. The only real change is following hunk:

> 

> +           gimple *arg_def = SSA_NAME_DEF_STMT (arg);

> +           if (is_a<gphi *> (arg_def))

> +             {

> +               if (!malloc_candidate_p_1 (fun, arg, phi, ipa))

> +                   DUMP_AND_RETURN ("nested phi fail")

> +               continue;

> +             }

> +

> 

> Which checks recursively that the phi argument is used only within

> comparisons against 0

> and the phi.

> 

> Bootstrapped+tested on x86_64-unknown-linux-gnu.

> OK to commit ?

> 

> Thanks,

> Prathamesh

> 

> 

> pr85787-1.txt

> 

> 2018-08-28  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

> 

> 	PR tree-optimization/85787

> 	* ipa-pure-const.c (malloc_candidate_p_1): Move most of malloc_candidate_p

> 	into this function and add support for detecting multiple phis.

> 	(DUMP_AND_RETURN): Move from malloc_candidate_p into top-level macro.

> 

OK.
jeff
Prathamesh Kulkarni Oct. 4, 2018, 11:08 a.m. | #3
On Fri, 14 Sep 2018 at 22:49, Jeff Law <law@redhat.com> wrote:
>

> On 8/28/18 5:26 AM, Prathamesh Kulkarni wrote:

> > H

> > The attached patch extends malloc_candidate_p to handle multiple phis.

> > There's a lot of noise in the patch because I moved most of

> > malloc_candidate_p into

> > new function malloc_candidate_p_1. The only real change is following hunk:

> >

> > +           gimple *arg_def = SSA_NAME_DEF_STMT (arg);

> > +           if (is_a<gphi *> (arg_def))

> > +             {

> > +               if (!malloc_candidate_p_1 (fun, arg, phi, ipa))

> > +                   DUMP_AND_RETURN ("nested phi fail")

> > +               continue;

> > +             }

> > +

> >

> > Which checks recursively that the phi argument is used only within

> > comparisons against 0

> > and the phi.

> >

> > Bootstrapped+tested on x86_64-unknown-linux-gnu.

> > OK to commit ?

> >

> > Thanks,

> > Prathamesh

> >

> >

> > pr85787-1.txt

> >

> > 2018-08-28  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

> >

> >       PR tree-optimization/85787

> >       * ipa-pure-const.c (malloc_candidate_p_1): Move most of malloc_candidate_p

> >       into this function and add support for detecting multiple phis.

> >       (DUMP_AND_RETURN): Move from malloc_candidate_p into top-level macro.

> >

> OK.

Hi Jeff,
Thanks for the review, and sorry for the delay.
Committed it as r264838 after re-bootstrap+test on x86_64 with
--enable-languages=all,ada,go.

Thanks,
Prathamesh
> jeff

>

>

Patch

diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
index a9a8863d907..66c81be23ec 100644
--- a/gcc/ipa-pure-const.c
+++ b/gcc/ipa-pure-const.c
@@ -869,14 +869,6 @@  check_retval_uses (tree retval, gimple *stmt)
       and return_stmt (and likewise a phi arg has immediate use only within comparison
       or the phi stmt).  */
 
-static bool
-malloc_candidate_p (function *fun, bool ipa)
-{
-  basic_block exit_block = EXIT_BLOCK_PTR_FOR_FN (fun);
-  edge e;
-  edge_iterator ei;
-  cgraph_node *node = cgraph_node::get_create (fun->decl);
-
 #define DUMP_AND_RETURN(reason)  \
 {  \
   if (dump_file && (dump_flags & TDF_DETAILS))  \
@@ -885,6 +877,96 @@  malloc_candidate_p (function *fun, bool ipa)
   return false;  \
 }
 
+static bool
+malloc_candidate_p_1 (function *fun, tree retval, gimple *ret_stmt, bool ipa)
+{
+  cgraph_node *node = cgraph_node::get_create (fun->decl);
+
+  if (!check_retval_uses (retval, ret_stmt))
+    DUMP_AND_RETURN("Return value has uses outside return stmt"
+		    " and comparisons against 0.")
+
+  gimple *def = SSA_NAME_DEF_STMT (retval);
+
+  if (gcall *call_stmt = dyn_cast<gcall *> (def))
+    {
+      tree callee_decl = gimple_call_fndecl (call_stmt);
+      if (!callee_decl)
+	return false;
+
+      if (!ipa && !DECL_IS_MALLOC (callee_decl))
+	DUMP_AND_RETURN("callee_decl does not have malloc attribute for"
+			" non-ipa mode.")
+
+      cgraph_edge *cs = node->get_edge (call_stmt);
+      if (cs)
+	{
+	  ipa_call_summary *es = ipa_call_summaries->get_create (cs);
+	  es->is_return_callee_uncaptured = true;
+	}
+    }
+
+    else if (gphi *phi = dyn_cast<gphi *> (def))
+      {
+	bool all_args_zero = true;
+	for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i)
+	  {
+	    tree arg = gimple_phi_arg_def (phi, i);
+	    if (integer_zerop (arg))
+	      continue;
+
+	    all_args_zero = false;
+	    if (TREE_CODE (arg) != SSA_NAME)
+	      DUMP_AND_RETURN ("phi arg is not SSA_NAME.");
+	    if (!check_retval_uses (arg, phi))
+	      DUMP_AND_RETURN ("phi arg has uses outside phi"
+				 " and comparisons against 0.")
+
+	    gimple *arg_def = SSA_NAME_DEF_STMT (arg);
+	    if (is_a<gphi *> (arg_def))
+	      {
+		if (!malloc_candidate_p_1 (fun, arg, phi, ipa))
+		    DUMP_AND_RETURN ("nested phi fail")
+		continue;
+	      }
+
+	    gcall *call_stmt = dyn_cast<gcall *> (arg_def);
+	    if (!call_stmt)
+	      DUMP_AND_RETURN ("phi arg is a not a call_stmt.")
+
+	    tree callee_decl = gimple_call_fndecl (call_stmt);
+	    if (!callee_decl)
+	      return false;
+	    if (!ipa && !DECL_IS_MALLOC (callee_decl))
+	      DUMP_AND_RETURN("callee_decl does not have malloc attribute"
+			      " for non-ipa mode.")
+
+	    cgraph_edge *cs = node->get_edge (call_stmt);
+	    if (cs)
+	      {
+		ipa_call_summary *es = ipa_call_summaries->get_create (cs);
+		es->is_return_callee_uncaptured = true;
+	      }
+	  }
+
+	if (all_args_zero)
+	  DUMP_AND_RETURN ("Return value is a phi with all args equal to 0.")
+      }
+
+    else
+      DUMP_AND_RETURN("def_stmt of return value is not a call or phi-stmt.")
+
+  return true;
+}
+
+static bool
+malloc_candidate_p (function *fun, bool ipa)
+{
+  basic_block exit_block = EXIT_BLOCK_PTR_FOR_FN (fun);
+  edge e;
+  edge_iterator ei;
+  cgraph_node *node = cgraph_node::get_create (fun->decl);
+
   if (EDGE_COUNT (exit_block->preds) == 0
       || !flag_delete_null_pointer_checks)
     return false;
@@ -905,80 +987,17 @@  malloc_candidate_p (function *fun, bool ipa)
 	  || TREE_CODE (TREE_TYPE (retval)) != POINTER_TYPE)
 	DUMP_AND_RETURN("Return value is not SSA_NAME or not a pointer type.")
 
-      if (!check_retval_uses (retval, ret_stmt))
-	DUMP_AND_RETURN("Return value has uses outside return stmt"
-			" and comparisons against 0.")
-
-      gimple *def = SSA_NAME_DEF_STMT (retval);
-      if (gcall *call_stmt = dyn_cast<gcall *> (def))
-	{
-	  tree callee_decl = gimple_call_fndecl (call_stmt);
-	  if (!callee_decl)
-	    return false;
-
-	  if (!ipa && !DECL_IS_MALLOC (callee_decl))
-	    DUMP_AND_RETURN("callee_decl does not have malloc attribute for"
-			    " non-ipa mode.")
-
-	  cgraph_edge *cs = node->get_edge (call_stmt);
-	  if (cs)
-	    {
-	      ipa_call_summary *es = ipa_call_summaries->get_create (cs);
-	      es->is_return_callee_uncaptured = true;
-	    }
-	}
-
-      else if (gphi *phi = dyn_cast<gphi *> (def))
-	{
-	  bool all_args_zero = true;
-	  for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i)
-	    {
-	      tree arg = gimple_phi_arg_def (phi, i);
-	      if (integer_zerop (arg))
-		continue;
-
-	      all_args_zero = false;
-	      if (TREE_CODE (arg) != SSA_NAME)
-		DUMP_AND_RETURN ("phi arg is not SSA_NAME.");
-	      if (!check_retval_uses (arg, phi))
-		DUMP_AND_RETURN ("phi arg has uses outside phi"
-				 " and comparisons against 0.")
-
-	      gimple *arg_def = SSA_NAME_DEF_STMT (arg);
-	      gcall *call_stmt = dyn_cast<gcall *> (arg_def);
-	      if (!call_stmt)
-		return false;
-	      tree callee_decl = gimple_call_fndecl (call_stmt);
-	      if (!callee_decl)
-		return false;
-	      if (!ipa && !DECL_IS_MALLOC (callee_decl))
-		DUMP_AND_RETURN("callee_decl does not have malloc attribute"
-				" for non-ipa mode.")
-
-	      cgraph_edge *cs = node->get_edge (call_stmt);
-	      if (cs)
-		{
-		  ipa_call_summary *es = ipa_call_summaries->get_create (cs);
-		  es->is_return_callee_uncaptured = true;
-		}
-	    }
-
-	  if (all_args_zero)
-	    DUMP_AND_RETURN ("Return value is a phi with all args equal to 0.");
-	}
-
-      else
-	DUMP_AND_RETURN("def_stmt of return value is not a call or phi-stmt.")
+      if (!malloc_candidate_p_1 (fun, retval, ret_stmt, ipa))
+	return false;
     }
 
   if (dump_file && (dump_flags & TDF_DETAILS))
     fprintf (dump_file, "\nFound %s to be candidate for malloc attribute\n",
 	     IDENTIFIER_POINTER (DECL_NAME (fun->decl)));
   return true;
-
-#undef DUMP_AND_RETURN
 }
 
+#undef DUMP_AND_RETURN
 
 /* This is the main routine for finding the reference patterns for
    global variables within a function FN.  */
diff --git a/gcc/testsuite/gcc.dg/ipa/propmalloc-4.c b/gcc/testsuite/gcc.dg/ipa/propmalloc-4.c
new file mode 100644
index 00000000000..4c40d63f63d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/propmalloc-4.c
@@ -0,0 +1,56 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-local-pure-const-details" } */
+
+void *foo(int cond1, int cond2, int cond3)
+{
+  void *ret;
+  void *a;
+  void *b;
+
+  if (cond1)
+    a = __builtin_malloc (10);
+  else
+    a = __builtin_malloc (20);
+
+  if (cond2)
+    b = __builtin_malloc (30);
+  else
+    b = __builtin_malloc (40);
+
+  if (cond3)
+    ret = a;
+  else
+    ret = b;
+
+  return ret;
+}
+
+void *foo2(int cond1, int cond2, int cond3)
+{
+  void *ret;
+  void *a;
+  void *b;
+  void bar(void *, void *);
+
+  if (cond1)
+    a = __builtin_malloc (10);
+  else
+    a = __builtin_malloc (20);
+
+  if (cond2)
+    b = __builtin_malloc (30);
+  else
+    b = __builtin_malloc (40);
+
+  bar (a, b);
+
+  if (cond3)
+    ret = a;
+  else
+    ret = b;
+
+  return ret;
+}
+
+/* { dg-final { scan-tree-dump "Function found to be malloc: foo" "local-pure-const1" } } */
+/* { dg-final { scan-tree-dump-not "Function found to be malloc: foo2" "local-pure-const1" } } */