diff mbox

Don't invalidate string length cache when not needed

Message ID 20130517090658.GJ14240@redhat.com
State New
Headers show

Commit Message

Marek Polacek May 17, 2013, 9:06 a.m. UTC 
On Thu, May 16, 2013 at 10:33:27PM +0200, Jakub Jelinek wrote:
> On Thu, May 16, 2013 at 06:44:03PM +0200, Marek Polacek wrote:
> > --- gcc/tree-ssa-strlen.c.mp	2013-05-15 14:11:20.079707492 +0200
> > +++ gcc/tree-ssa-strlen.c	2013-05-16 17:57:33.963150006 +0200
> > @@ -1693,8 +1693,10 @@ handle_char_store (gimple_stmt_iterator
> >  		}
> >  	      else
> >  		{
> > -		  si->writable = true;
> > -		  si->dont_invalidate = true;
> > +		  /* The string might be e.g. in the .rodata section.  */
> > +		  si->writable = false;
> 
> No, this really should be si->writable = true; (and comment not needed).
> Ok with that change.

Done.

> The thing is, while the string might not be known to be writable before,
> i.e. we can't optimize away this store, because supposedly it should
> trap, if we notice e.g. another write to the same location (writing zero
> there again), we can optimize that other write already, because we know
> that this store stored there something.
> 
> > +#include "strlenopt.h"
> > +
> > +__attribute__((noinline, noclone)) size_t
> > +fn1 (char *p, const char *r)
> > +{
> > +  size_t len1 = __builtin_strlen (r);
> > +  char *q = __builtin_strchr (p, '\0');
> > +  *q = '\0';
> > +  return len1 - __builtin_strlen (r); // This strlen should be optimized into len1.
> 
> With strlenopt.h include you can avoid using __builtin_ prefixes, all the
> builtins needed are prototyped in that header.

Ugh, sure, that's only a remnant from my testing.  Thanks for the
reviews.  Will commit this one finally.

2013-05-17  Marek Polacek  <polacek@redhat.com>

	* tree-ssa-strlen.c (handle_char_store): Don't invalidate
	cached length when doing non-zero store of storing '\0' to
	'\0'.

	* gcc.dg/strlenopt-25.c: New test.
	* gcc.dg/strlenopt-26.c: Likewise.


	Marek
diff mbox

Patch

--- gcc/tree-ssa-strlen.c.mp	2013-05-15 14:11:20.079707492 +0200
+++ gcc/tree-ssa-strlen.c	2013-05-17 10:59:27.461231167 +0200
@@ -1694,7 +1694,8 @@  handle_char_store (gimple_stmt_iterator
 	      else
 		{
 		  si->writable = true;
-		  si->dont_invalidate = true;
+		  gsi_next (gsi);
+		  return false;
 		}
 	    }
 	  else
@@ -1717,6 +1718,33 @@  handle_char_store (gimple_stmt_iterator
 	    si->endptr = ssaname;
 	  si->dont_invalidate = true;
 	}
+      /* If si->length is non-zero constant, we aren't overwriting '\0',
+	 and if we aren't storing '\0', we know that the length of the
+	 string and any other zero terminated string in memory remains
+	 the same.  In that case we move to the next gimple statement and
+	 return to signal the caller that it shouldn't invalidate anything.  
+
+	 This is benefical for cases like:
+
+	 char p[20];
+	 void foo (char *q)
+	 {
+	   strcpy (p, "foobar");
+	   size_t len = strlen (p);        // This can be optimized into 6
+	   size_t len2 = strlen (q);        // This has to be computed
+	   p[0] = 'X';
+	   size_t len3 = strlen (p);        // This can be optimized into 6
+	   size_t len4 = strlen (q);        // This can be optimized into len2
+	   bar (len, len2, len3, len4);
+        }
+	*/ 
+      else if (si != NULL && si->length != NULL_TREE
+	       && TREE_CODE (si->length) == INTEGER_CST
+	       && integer_nonzerop (gimple_assign_rhs1 (stmt)))
+	{
+	  gsi_next (gsi);
+	  return false;
+	}
     }
   else if (idx == 0 && initializer_zerop (gimple_assign_rhs1 (stmt)))
     {
--- gcc/testsuite/gcc.dg/strlenopt-25.c.mp	2013-05-15 17:15:18.702118637 +0200
+++ gcc/testsuite/gcc.dg/strlenopt-25.c	2013-05-15 18:26:27.881030317 +0200
@@ -0,0 +1,18 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 -fdump-tree-strlen" } */
+
+#include "strlenopt.h"
+
+int
+main ()
+{
+  char p[] = "foobar";
+  int len, len2;
+  len = strlen (p);
+  p[0] = 'O';
+  len2 = strlen (p);
+  return len - len2;
+}
+
+/* { dg-final { scan-tree-dump-times "strlen \\(" 0 "strlen" } } */
+/* { dg-final { cleanup-tree-dump "strlen" } } */
--- gcc/testsuite/gcc.dg/strlenopt-26.c.mp	2013-05-16 17:33:00.302060413 +0200
+++ gcc/testsuite/gcc.dg/strlenopt-26.c	2013-05-17 10:58:17.605015608 +0200
@@ -0,0 +1,25 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 -fdump-tree-strlen" } */
+
+#include "strlenopt.h"
+
+__attribute__((noinline, noclone)) size_t
+fn1 (char *p, const char *r)
+{
+  size_t len1 = strlen (r);
+  char *q = strchr (p, '\0');
+  *q = '\0';
+  return len1 - strlen (r); // This strlen should be optimized into len1.
+}
+
+int
+main (void)
+{
+  char p[] = "foobar";
+  const char *volatile q = "xyzzy";
+  fn1 (p, q);
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "strlen \\(" 1 "strlen" } } */
+/* { dg-final { cleanup-tree-dump "strlen" } } */