diff mbox

handle bzero/bcopy in DSE and aliasing (PR 80933, 80934)

Message ID c10a89fa-e8cb-b2e5-ad02-be8a6d5e7a1f@gmail.com
State New
Headers show

Commit Message

Martin Sebor June 1, 2017, 8:52 p.m. UTC
While testing some otherwise unrelated enhancements in these areas
I noticed that calls to bzero and bcopy are not being handled as
efficiently as equivalent calls to memset and memcpy.  Specifically,
redundant calls are not eliminated and the functions appear to be
treated as if they allowed their pointer arguments to escape.  This
turned out to be due to the missing handling of the former two built
ins by the DSE and aliasing passes.

The attached patch adds this handling so the cases I noted in the two
PRs are now handled.

Tested on x86_64-linux.

Martin

Comments

Richard Biener June 2, 2017, 11:12 a.m. UTC | #1
On Thu, Jun 1, 2017 at 10:52 PM, Martin Sebor <msebor@gmail.com> wrote:
> While testing some otherwise unrelated enhancements in these areas
> I noticed that calls to bzero and bcopy are not being handled as
> efficiently as equivalent calls to memset and memcpy.  Specifically,
> redundant calls are not eliminated and the functions appear to be
> treated as if they allowed their pointer arguments to escape.  This
> turned out to be due to the missing handling of the former two built
> ins by the DSE and aliasing passes.
>
> The attached patch adds this handling so the cases I noted in the two
> PRs are now handled.

+  /* The number of the size argument to one of the built-in functions
+     below.  */
+  unsigned sizargno = 2;
   /* Handle those builtin functions explicitly that do not act as
      escape points.  See tree-ssa-structalias.c:find_func_aliases
      for the list of builtins we might need to handle here.  */
@@ -2030,8 +2034,13 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref)
       && gimple_call_builtin_p (call, BUILT_IN_NORMAL))
     switch (DECL_FUNCTION_CODE (callee))
       {
-       /* All the following functions clobber memory pointed to by
-          their first argument.  */
+        case BUILT_IN_BZERO:
+         sizargno = 1;
+         /* Fall through.  */
+
+         /* With the exception of the bzero function above, all of
+            the following clobber memory pointed to by their first
+            argument.  */
        case BUILT_IN_STRCPY:
        case BUILT_IN_STRNCPY:
        case BUILT_IN_MEMCPY:
@@ -2062,9 +2071,9 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref)
               is strlen (dest) + n + 1 instead of n, resp.
               n + 1 at dest + strlen (dest), but strlen (dest) isn't
               known.  */
-           if (gimple_call_num_args (call) == 3
+           if (gimple_call_num_args (call) > sizargno
                && DECL_FUNCTION_CODE (callee) != BUILT_IN_STRNCAT)
-             size = gimple_call_arg (call, 2);
+             size = gimple_call_arg (call, sizargno);
            ao_ref_init_from_ptr_and_size (&dref,
                                           gimple_call_arg (call, 0),
                                           size);

please insted do

        if (DECL_FUNCTION_CODE (callee) == BUILT_IN_BZERO)
          size = gimple_call_Arg (call, 1);
        else if (gimple_call_num_args (call) == 3
...

instead of the above changes.  Likewise in stmt_kills_ref_p.

Likewise in initialize_ao_ref_for_dse I see no value in uglifying the code
but instead do

   case BUILT_IN_BCOPY:
      {
        ao_ref_init_from_ptr_and_size (write, gimple_call_arg (stmt,
1), gimple_call_arg (stmt, 2));
        return true;
      }

and similar for BZERO.

Similar in maybe_trim_memstar_call, for decrement_count simply pass in
the tree *
instead of the argno (and you can get rid of the stmt as well.

+     1) Bzero and memset.  */
+  bool memset_p = false;
   if (is_gimple_reg_type (vr->type)
-      && gimple_call_builtin_p (def_stmt, BUILT_IN_MEMSET)
-      && integer_zerop (gimple_call_arg (def_stmt, 1))
-      && tree_fits_uhwi_p (gimple_call_arg (def_stmt, 2))
+      && (((memset_p = gimple_call_builtin_p (def_stmt, BUILT_IN_MEMSET))
+          && integer_zerop (gimple_call_arg (def_stmt, 1))
+          && tree_fits_uhwi_p (gimple_call_arg (def_stmt, 2)))
+         || gimple_call_builtin_p (def_stmt, BUILT_IN_BZERO))

you miss the tree_fits_uhwi_p check for bzero.

Note I'd be _much_ more sympathetic to simply canonicalizing all of
bzero and bcopy
to memset / memmove and be done with all the above complexity.

Richard.

> Tested on x86_64-linux.
>
> Martin
Bernhard Reutner-Fischer June 4, 2017, 3:36 p.m. UTC | #2
On 2 June 2017 13:12:41 CEST, Richard Biener <richard.guenther@gmail.com> wrote:

>Note I'd be _much_ more sympathetic to simply canonicalizing all of
>bzero and bcopy
>to memset / memmove and be done with all the above complexity.

Indeed and even more so since SUSv3 marked it LEGACY and both were removed in SUSv4.
thanks,
Jeff Law June 6, 2017, 4:54 p.m. UTC | #3
On 06/04/2017 09:36 AM, Bernhard Reutner-Fischer wrote:
> On 2 June 2017 13:12:41 CEST, Richard Biener <richard.guenther@gmail.com> wrote:
> 
>> Note I'd be _much_ more sympathetic to simply canonicalizing all of
>> bzero and bcopy
>> to memset / memmove and be done with all the above complexity.
> 
> Indeed and even more so since SUSv3 marked it LEGACY and both were removed in SUSv4.
> thanks,
Likewise.
jeff
diff mbox

Patch

PR tree-optimization/80934 - bzero should be assumed not to escape pointer argument
PR tree-optimization/80933 - redundant bzero/bcopy calls not eliminated

gcc/ChangeLog:

	PR tree-optimization/80933
	* tree-ssa-alias.c (ref_maybe_used_by_call_p_1): Handle bzero.
	(call_may_clobber_ref_p_1): Likewise.
	(stmt_kills_ref_p): Likewise.
	* tree-ssa-dse.c (initialize_ao_ref_for_dse): Handle bcopy and bzero.
	(decrement_count): Add an argument.
	(maybe_trim_memstar_call): Handle bcopy.
	(dse_dom_walker::dse_optimize_stmt): Likewise.
	* tree-ssa-sccvn.c (vn_reference_lookup_3): Handle bzero.
	* tree-ssa-structalias.c (find_func_aliases_for_builtin_call): Likewise.
	(find_func_clobbers): Likewise.

gcc/testsuite/ChangeLog:

	PR tree-optimization/80933
	* gcc.dg/tree-ssa/ssa-dse-30.c: New test.
	* gcc.dg/tree-ssa/alias-36.c: Likewise.

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/alias-36.c b/gcc/testsuite/gcc.dg/tree-ssa/alias-36.c
new file mode 100644
index 0000000..61b601a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/alias-36.c
@@ -0,0 +1,28 @@ 
+/* PR tree-optimization/80934 - bzero should be assumed not to escape
+   pointer argument
+   { dg-do compile }
+   { dg-options "-O2 -fdump-tree-alias" } */
+
+void foobar (void);
+
+void f (void);
+
+void g (void)
+{
+  char d[32];
+  __builtin_memset (d, 0, sizeof d);
+  f ();
+  if (*d != 0)
+    foobar ();
+}
+
+void h (void)
+{
+  char d[32];
+  __builtin_bzero (d, sizeof d);
+  f ();
+  if (*d != 0)
+    foobar ();
+}
+
+/* { dg-final { scan-tree-dump-not "memset|foobar|bzero" "alias" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-30.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-30.c
new file mode 100644
index 0000000..9d2c920
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-30.c
@@ -0,0 +1,30 @@ 
+/* PR tree-optimization/80933 - redundant bzero/bcopy calls not eliminated
+   { dg-do compile }
+   { dg-options "-O2 -fdump-tree-dse1" } */
+
+void sink (void*);
+
+void test_bcopy (const void *s)
+{
+  char d[33];
+
+  /* The bcopy calls are expanded inline in EVRP, before DSE runs,
+     so this test doesn't actually verify that DSE does its job.  */
+  __builtin_bcopy (s, d, sizeof d);
+  __builtin_bcopy (s, d, sizeof d);
+
+  sink (d);
+}
+
+void test_bzero (void)
+{
+  char d[33];
+
+  __builtin_bzero (d, sizeof d);
+  __builtin_bzero (d, sizeof d);
+
+  sink (d);
+}
+
+/* { dg-final { scan-tree-dump-times "builtin_bzero" 1 "dse1" } } */
+/* { dg-final { scan-tree-dump-not "builtin_bcopy" "dse1" } } */
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 74ee2b0..8c4b289f 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -1783,6 +1783,7 @@  ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *ref)
 	case BUILT_IN_ALLOCA_WITH_ALIGN:
 	case BUILT_IN_STACK_SAVE:
 	case BUILT_IN_STACK_RESTORE:
+	case BUILT_IN_BZERO:
 	case BUILT_IN_MEMSET:
 	case BUILT_IN_TM_MEMSET:
 	case BUILT_IN_MEMSET_CHK:
@@ -2023,6 +2024,9 @@  call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref)
 
   callee = gimple_call_fndecl (call);
 
+  /* The number of the size argument to one of the built-in functions
+     below.  */
+  unsigned sizargno = 2;
   /* Handle those builtin functions explicitly that do not act as
      escape points.  See tree-ssa-structalias.c:find_func_aliases
      for the list of builtins we might need to handle here.  */
@@ -2030,8 +2034,13 @@  call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref)
       && gimple_call_builtin_p (call, BUILT_IN_NORMAL))
     switch (DECL_FUNCTION_CODE (callee))
       {
-	/* All the following functions clobber memory pointed to by
-	   their first argument.  */
+        case BUILT_IN_BZERO:
+	  sizargno = 1;
+	  /* Fall through.  */
+
+	  /* With the exception of the bzero function above, all of
+	     the following clobber memory pointed to by their first
+	     argument.  */
 	case BUILT_IN_STRCPY:
 	case BUILT_IN_STRNCPY:
 	case BUILT_IN_MEMCPY:
@@ -2062,9 +2071,9 @@  call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref)
 	       is strlen (dest) + n + 1 instead of n, resp.
 	       n + 1 at dest + strlen (dest), but strlen (dest) isn't
 	       known.  */
-	    if (gimple_call_num_args (call) == 3
+	    if (gimple_call_num_args (call) > sizargno
 		&& DECL_FUNCTION_CODE (callee) != BUILT_IN_STRNCAT)
-	      size = gimple_call_arg (call, 2);
+	      size = gimple_call_arg (call, sizargno);
 	    ao_ref_init_from_ptr_and_size (&dref,
 					   gimple_call_arg (call, 0),
 					   size);
@@ -2512,6 +2521,10 @@  stmt_kills_ref_p (gimple *stmt, ao_ref *ref)
 
   if (is_gimple_call (stmt))
     {
+      /* The number of the size argument to one of the built-in functions
+	 below.  */
+      unsigned sizargno = 2;
+
       tree callee = gimple_call_fndecl (stmt);
       if (callee != NULL_TREE
 	  && gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
@@ -2527,6 +2540,10 @@  stmt_kills_ref_p (gimple *stmt, ao_ref *ref)
 	      break;
 	    }
 
+	  case BUILT_IN_BZERO:
+	    sizargno = 1;
+	    /* Fall through.  */
+
 	  case BUILT_IN_MEMCPY:
 	  case BUILT_IN_MEMPCPY:
 	  case BUILT_IN_MEMMOVE:
@@ -2543,7 +2560,7 @@  stmt_kills_ref_p (gimple *stmt, ao_ref *ref)
 	      if (ref->max_size == -1)
 		return false;
 	      tree dest = gimple_call_arg (stmt, 0);
-	      tree len = gimple_call_arg (stmt, 2);
+	      tree len = gimple_call_arg (stmt, sizargno);
 	      if (!tree_fits_shwi_p (len))
 		return false;
 	      tree rbase = ref->base;
diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index 70c8b07..359ef8c 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -92,22 +92,40 @@  initialize_ao_ref_for_dse (gimple *stmt, ao_ref *write)
   /* It's advantageous to handle certain mem* functions.  */
   if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
     {
+      /* The zero-based argument number of the destination pointer and
+	 the byte count of the raw memory call.  */
+      unsigned dstargno;
+      unsigned sizargno;
+
       switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt)))
 	{
+  	  case BUILT_IN_BCOPY:
+	    dstargno = 1;
+	    sizargno = 2;
+	    break;
+
+	  case BUILT_IN_BZERO:
+	    dstargno = 0;
+	    sizargno = 1;
+	    break;
+
 	  case BUILT_IN_MEMCPY:
 	  case BUILT_IN_MEMMOVE:
 	  case BUILT_IN_MEMSET:
-	    {
-	      tree size = NULL_TREE;
-	      if (gimple_call_num_args (stmt) == 3)
-		size = gimple_call_arg (stmt, 2);
-	      tree ptr = gimple_call_arg (stmt, 0);
-	      ao_ref_init_from_ptr_and_size (write, ptr, size);
-	      return true;
-	    }
-	  default:
+	    dstargno = 0;
+	    sizargno = 2;
 	    break;
+
+	  default:
+	    return false;
 	}
+
+      tree size = NULL_TREE;
+      if (gimple_call_num_args (stmt) > sizargno)
+	size = gimple_call_arg (stmt, sizargno);
+      tree ptr = gimple_call_arg (stmt, dstargno);
+      ao_ref_init_from_ptr_and_size (write, ptr, size);
+      return true;
     }
   else if (is_gimple_assign (stmt))
     {
@@ -349,9 +367,9 @@  maybe_trim_constructor_store (ao_ref *ref, sbitmap live, gimple *stmt)
 /* STMT is a memcpy, memmove or memset.  Decrement the number of bytes
    copied/set by DECREMENT.  */
 static void
-decrement_count (gimple *stmt, int decrement)
+decrement_count (gimple *stmt, int decrement, unsigned argno)
 {
-  tree *countp = gimple_call_arg_ptr (stmt, 2);
+  tree *countp = gimple_call_arg_ptr (stmt, argno);
   gcc_assert (TREE_CODE (*countp) == INTEGER_CST);
   *countp = wide_int_to_tree (TREE_TYPE (*countp), (TREE_INT_CST_LOW (*countp)
 						    - decrement));
@@ -391,8 +409,19 @@  increment_start_addr (gimple *stmt, tree *where, int increment)
 static void
 maybe_trim_memstar_call (ao_ref *ref, sbitmap live, gimple *stmt)
 {
+  /* The zero-based argument number of the destination and soutce pointers,
+     and the byte count of the raw memory call.  */
+  unsigned dstargno = 0;
+  unsigned srcargno = 1;
+  unsigned sizargno = 2;
+
   switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt)))
     {
+    case BUILT_IN_BCOPY:
+      dstargno = 1;
+      srcargno = 0;
+      /* Fall through.  */
+
     case BUILT_IN_MEMCPY:
     case BUILT_IN_MEMMOVE:
       {
@@ -401,20 +430,24 @@  maybe_trim_memstar_call (ao_ref *ref, sbitmap live, gimple *stmt)
 
 	/* Tail trimming is easy, we can just reduce the count.  */
         if (tail_trim)
-	  decrement_count (stmt, tail_trim);
+	  decrement_count (stmt, tail_trim, sizargno);
 
 	/* Head trimming requires adjusting all the arguments.  */
         if (head_trim)
           {
-	    tree *dst = gimple_call_arg_ptr (stmt, 0);
+	    tree *dst = gimple_call_arg_ptr (stmt, dstargno);
 	    increment_start_addr (stmt, dst, head_trim);
-	    tree *src = gimple_call_arg_ptr (stmt, 1);
+	    tree *src = gimple_call_arg_ptr (stmt, srcargno);
 	    increment_start_addr (stmt, src, head_trim);
-	    decrement_count (stmt, head_trim);
+	    decrement_count (stmt, head_trim, sizargno);
 	  }
         break;
       }
 
+    case BUILT_IN_BZERO:
+      sizargno = 1;
+      /* Fall through.  */
+
     case BUILT_IN_MEMSET:
       {
 	int head_trim, tail_trim;
@@ -422,14 +455,14 @@  maybe_trim_memstar_call (ao_ref *ref, sbitmap live, gimple *stmt)
 
 	/* Tail trimming is easy, we can just reduce the count.  */
         if (tail_trim)
-	  decrement_count (stmt, tail_trim);
+	  decrement_count (stmt, tail_trim, sizargno);
 
 	/* Head trimming requires adjusting all the arguments.  */
         if (head_trim)
           {
 	    tree *dst = gimple_call_arg_ptr (stmt, 0);
 	    increment_start_addr (stmt, dst, head_trim);
-	    decrement_count (stmt, head_trim);
+	    decrement_count (stmt, head_trim, sizargno);
 	  }
 	break;
       }
@@ -707,8 +740,17 @@  dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator *gsi)
      some builtin calls.  */
   if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
     {
+      /* The zero-based argument number of the byte count of the raw
+	 memory call.  */
+      unsigned sizargno = 2;
+
       switch (DECL_FUNCTION_CODE (gimple_call_fndecl (stmt)))
 	{
+	  case BUILT_IN_BZERO:
+	    sizargno = 1;
+	    /* Fall through.  */
+
+	  case BUILT_IN_BCOPY:
 	  case BUILT_IN_MEMCPY:
 	  case BUILT_IN_MEMMOVE:
 	  case BUILT_IN_MEMSET:
@@ -716,7 +758,7 @@  dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator *gsi)
 	      /* Occasionally calls with an explicit length of zero
 		 show up in the IL.  It's pointless to do analysis
 		 on them, they're trivially dead.  */
-	      tree size = gimple_call_arg (stmt, 2);
+	      tree size = gimple_call_arg (stmt, sizargno);
 	      if (integer_zerop (size))
 		{
 		  delete_dead_call (gsi);
diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c
index c140c35..3a9541d 100644
--- a/gcc/tree-ssa-sccvn.c
+++ b/gcc/tree-ssa-sccvn.c
@@ -1882,11 +1882,13 @@  vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *vr_,
 
   /* def_stmt may-defs *ref.  See if we can derive a value for *ref
      from that definition.
-     1) Memset.  */
+     1) Bzero and memset.  */
+  bool memset_p = false;
   if (is_gimple_reg_type (vr->type)
-      && gimple_call_builtin_p (def_stmt, BUILT_IN_MEMSET)
-      && integer_zerop (gimple_call_arg (def_stmt, 1))
-      && tree_fits_uhwi_p (gimple_call_arg (def_stmt, 2))
+      && (((memset_p = gimple_call_builtin_p (def_stmt, BUILT_IN_MEMSET))
+	   && integer_zerop (gimple_call_arg (def_stmt, 1))
+	   && tree_fits_uhwi_p (gimple_call_arg (def_stmt, 2)))
+	  || gimple_call_builtin_p (def_stmt, BUILT_IN_BZERO))
       && TREE_CODE (gimple_call_arg (def_stmt, 0)) == ADDR_EXPR)
     {
       tree ref2 = TREE_OPERAND (gimple_call_arg (def_stmt, 0), 0);
@@ -1895,9 +1897,10 @@  vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *vr_,
       bool reverse;
       base2 = get_ref_base_and_extent (ref2, &offset2, &size2, &maxsize2,
 				       &reverse);
-      size2 = tree_to_uhwi (gimple_call_arg (def_stmt, 2)) * 8;
-      if ((unsigned HOST_WIDE_INT)size2 / 8
-	  == tree_to_uhwi (gimple_call_arg (def_stmt, 2))
+      /* The byte count is argument 2 for bzero and 3 for memset.  */
+      tree sizarg = gimple_call_arg (def_stmt, 1 + memset_p);
+      size2 = tree_to_uhwi (sizarg) * 8;
+      if ((unsigned HOST_WIDE_INT)size2 / 8 == tree_to_uhwi (sizarg)
 	  && maxsize2 != -1
 	  && operand_equal_p (base, base2, 0)
 	  && offset2 <= offset
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index a4abd28..b2ea8c2 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -4337,10 +4337,13 @@  find_func_aliases_for_builtin_call (struct function *fn, gcall *t)
 	  process_all_all_constraints (lhsc, rhsc);
 	  return true;
 	}
+      case BUILT_IN_BZERO:
       case BUILT_IN_MEMSET:
       case BUILT_IN_MEMSET_CHK:
       case BUILT_IN_TM_MEMSET:
 	{
+	  bool zero_p = DECL_FUNCTION_CODE (fndecl) == BUILT_IN_BZERO;
+
 	  tree res = gimple_call_lhs (t);
 	  tree dest = gimple_call_arg (t, 0);
 	  unsigned i;
@@ -4356,7 +4359,8 @@  find_func_aliases_for_builtin_call (struct function *fn, gcall *t)
 	  get_constraint_for_ptr_offset (dest, NULL_TREE, &lhsc);
 	  do_deref (&lhsc);
 	  if (flag_delete_null_pointer_checks
-	      && integer_zerop (gimple_call_arg (t, 1)))
+	      && (zero_p
+		  || integer_zerop (gimple_call_arg (t, 1))))
 	    {
 	      ac.type = ADDRESSOF;
 	      ac.var = nothing_id;
@@ -5164,6 +5168,7 @@  find_func_clobbers (struct function *fn, gimple *origt)
 	    }
 	  /* The following function clobbers memory pointed to by
 	     its argument.  */
+	  case BUILT_IN_BZERO:
 	  case BUILT_IN_MEMSET:
 	  case BUILT_IN_MEMSET_CHK:
 	  case BUILT_IN_POSIX_MEMALIGN: