diff mbox

PR79697: Delete calls to strdup, strndup, realloc if there is no lhs

Message ID CAAgBjMnmjO1eCjTTFpz6mk9gjJJ8obZO7KUP_TtpPYQtDWGfoA@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni April 24, 2017, 7:18 a.m. UTC 
On 27 February 2017 at 00:10, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> On 26 February 2017 at 05:16, Martin Sebor <msebor@gmail.com> wrote:
>> On 02/25/2017 11:54 AM, Prathamesh Kulkarni wrote:
>>>
>>> On 25 February 2017 at 14:43, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>>
>>>> On Sat, 25 Feb 2017, Prathamesh Kulkarni wrote:
>>>>
>>>>> Hi,
>>>>> The attached patch deletes calls to strdup, strndup if it's
>>>>> return-value is unused,
>>>>> and same for realloc if the first arg is NULL.
>>>>> Bootstrap+tested on x86_64-unknown-linux-gnu.
>>>>> OK for GCC 8 ?
>>>>
>>>>
>>>>
>>>> Instead of specializing realloc(0,*) wherever we can perform the same
>>>> optimization as with malloc, wouldn't it be better to optimize:
>>>> realloc(0,n) -> malloc(n)
>>>> and let the malloc optimizations happen?
>>>
>>> Thanks for the suggestions. In the attached patch, realloc (0, n) is
>>> folded to malloc (n).
>>> Bootstrap+test in progress on x86_64-unknown-linux-gnu.
>>> Does the patch look OK ?
>>
>>
>> Although it's not part of the bug report, I wonder if a complete
>> patch should also extend the malloc/free DCE to str{,n}dup/free
>> calls and eliminate pairs like these:
>>
>>   void f (const char *s)
>>   {
>>     char *p = strdup (src);
>>     free (p);
>>   }
>>
>> (That seems to be just a matter of adding a couple of conditionals
>> for BUILT_IN_STR{,N}DUP in propagate_necessity).
> Hi Martin,
> Thanks for the suggestions, I have updated the patch accordingly.
> Does it look OK ?
Hi,
The attached patch for PR79697 passes bootstrap+test on x86_64 and
cross-tested on arm*-*-* and aarch64*-*-*.
As per the previous suggestions, the patch folds realloc (0, n) -> malloc (n).
Is it OK for trunk ?

Thanks,
Prathamesh
>>
>> Martin
>>
>> PS Another optimization, though one that's most likely outside
>> the scope of this patch, is to eliminate all of the following:
>>
>>   void f (const char *s, char *s2)
>>   {
>>     char *p = strdup (s);
>>     strcpy (p, s2);
>>     free (p);
>>   }
>>
>> as is done in:
>>
>>   void f (unsigned n, const char *s)
>>   {
>>     char *p = malloc (n);
>>     memcpy (p, s, n);
>>     free (p);
>>   }
>>
> Hmm, dse1 detects that the store to p in memcpy() is dead and deletes the call.
> cddce1 then removes calls to  malloc and free thus making the function empty.
> It doesn't work if memcpy is replaced by strcpy:
>
> void f (unsigned n, char *s2)
> {
>   char *p = __builtin_malloc (n);
>   __builtin_strcpy (p, s2);
>   __builtin_free (p);
> }
>
> I suppose strcpy should be special-cased too in dse similar to memcpy ?
>
> Thanks,
> Prathamesh
2017-04-24  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

	PR tree-optimization/79697
	* tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Check if callee
	is BUILT_IN_STRDUP, BUILT_IN_STRNDUP, BUILT_IN_REALLOC.
	(propagate_necessity): Check if def_callee is BUILT_IN_STRDUP or
	BUILT_IN_STRNDUP.
	* gimple-fold.c (gimple_fold_builtin_realloc): New function.
	(gimple_fold_builtin): Call gimple_fold_builtin_realloc.

testsuite/
	* gcc.dg/tree-ssa/pr79697.c: New test.
diff mbox

Patch

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index a75dd91..e6eceea 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -3251,6 +3251,28 @@  gimple_fold_builtin_acc_on_device (gimple_stmt_iterator *gsi, tree arg0)
   return true;
 }
 
+/* Fold realloc (0, n) -> malloc (n).  */
+
+static bool
+gimple_fold_builtin_realloc (gimple_stmt_iterator *gsi)
+{
+  gimple *stmt = gsi_stmt (*gsi);
+  tree arg = gimple_call_arg (stmt, 0);
+  tree size = gimple_call_arg (stmt, 1);
+
+  if (operand_equal_p (arg, null_pointer_node, 0))
+    {
+      tree fn_malloc = builtin_decl_implicit (BUILT_IN_MALLOC);
+      if (fn_malloc)
+	{
+	  gcall *repl = gimple_build_call (fn_malloc, 1, size);
+	  replace_call_with_call_and_fold (gsi, repl);
+	  return true;
+	}
+    }
+  return false;
+}
+
 /* Fold the non-target builtin at *GSI and return whether any simplification
    was made.  */
 
@@ -3409,6 +3431,9 @@  gimple_fold_builtin (gimple_stmt_iterator *gsi)
     case BUILT_IN_ACC_ON_DEVICE:
       return gimple_fold_builtin_acc_on_device (gsi,
 						gimple_call_arg (stmt, 0));
+    case BUILT_IN_REALLOC:
+      return gimple_fold_builtin_realloc (gsi);
+
     default:;
     }
 
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr79697.c b/gcc/testsuite/gcc.dg/tree-ssa/pr79697.c
new file mode 100644
index 0000000..d4f6473
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr79697.c
@@ -0,0 +1,33 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-gimple -fdump-tree-cddce-details -fdump-tree-optimized" } */
+
+void f(void)
+{
+  __builtin_strdup ("abc");
+}
+
+void g(void)
+{
+  __builtin_strndup ("abc", 3);
+}
+
+void h(void)
+{
+  __builtin_realloc (0, 10);
+}
+
+void k(void)
+{
+  char *p = __builtin_strdup ("abc");
+  __builtin_free (p);
+
+  char *q = __builtin_strndup ("abc", 3);
+  __builtin_free (q);
+}
+
+/* { dg-final { scan-tree-dump "Deleting : __builtin_strdup" "cddce1" } } */
+/* { dg-final { scan-tree-dump "Deleting : __builtin_strndup" "cddce1" } } */
+/* { dg-final { scan-tree-dump "__builtin_malloc" "gimple" } } */
+/* { dg-final { scan-tree-dump-not "__builtin_strdup" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__builtin_strndup" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__builtin_free" "optimized" } } */
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index 5ebe57b..e17659d 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -233,6 +233,8 @@  mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
 	    case BUILT_IN_CALLOC:
 	    case BUILT_IN_ALLOCA:
 	    case BUILT_IN_ALLOCA_WITH_ALIGN:
+	    case BUILT_IN_STRDUP:
+	    case BUILT_IN_STRNDUP:
 	      return;
 
 	    default:;
@@ -780,7 +782,9 @@  propagate_necessity (bool aggressive)
 		  && DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
 		  && (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_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC
+		      || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_STRDUP
+		      || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_STRNDUP))
 		{
 		  gimple *bounds_def_stmt;
 		  tree bounds;