diff mbox

Don't invalidate string length cache when not needed

Message ID 20130516133140.GG14240@redhat.com
State New
Headers show

Commit Message

Marek Polacek May 16, 2013, 1:31 p.m. UTC 
On Wed, May 15, 2013 at 07:20:03PM +0200, Jakub Jelinek wrote:
> Well, if si->length is known constant != 0 (note, it is enough to
> test that it is non-zero and probably the code above this has
> tested that already?) and we are storing non-zero, then that
> should mean we are overwriting a non-zero value with another (or same)
> non-zero value.  Such operation shouldn't change the length of any strings
> at all, not just the ones related to the current strinfo.

Right.  Adjusted.

> That is either just setting all current strinfos to dont_invalidate, or
> perhaps faster doing gsi_next (gsi); return false; (that means the caller
> won't call maybe_invalidate - of course in that case you must not set
> any dont_invalidate on any strinfo on that stmt) and the caller of
> strlen_optimize_stmt will not do gsi_next (&gsi) - that is why you need
> to do it instead.

Cool, I took the gsi_next approach; seems to work nicely.  So, updated
version (it still doesn't handle p[1], p[2], etc.).

Regtested/bootstrapped on x86_64-linux.

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

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

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


	Marek

Comments

Jakub Jelinek May 16, 2013, 1:38 p.m. UTC | #1 
On Thu, May 16, 2013 at 03:31:40PM +0200, Marek Polacek wrote:
> Cool, I took the gsi_next approach; seems to work nicely.  So, updated
> version (it still doesn't handle p[1], p[2], etc.).
> 
> Regtested/bootstrapped on x86_64-linux.
> 
> 2013-05-16  Marek Polacek  <polacek@redhat.com>
> 
> 	* tree-ssa-strlen.c (handle_char_store): Don't invalidate
> 	cached length when doing non-zero store.
> 
> 	* gcc.dg/strlenopt-25.c: New test.
> 
> --- gcc/tree-ssa-strlen.c.mp	2013-05-15 14:11:20.079707492 +0200
> +++ gcc/tree-ssa-strlen.c	2013-05-16 14:44:06.496545662 +0200
> @@ -1717,6 +1717,13 @@ handle_char_store (gimple_stmt_iterator
>  	    si->endptr = ssaname;
>  	  si->dont_invalidate = true;
>  	}

Please add here a comment what it does and why, that if si->length
is non-zero constant, we know that the character at that spot is
not '\0' and when storing non-'\0' to that location, we can't affect
size of any strings at all.  Therefore we do the gsi_next + return false
to signal caller that it shouldn't invalidate anything.

Ok with that change, thanks.

	Jakub
diff mbox

Patch

--- gcc/tree-ssa-strlen.c.mp	2013-05-15 14:11:20.079707492 +0200
+++ gcc/tree-ssa-strlen.c	2013-05-16 14:44:06.496545662 +0200
@@ -1717,6 +1717,13 @@  handle_char_store (gimple_stmt_iterator
 	    si->endptr = ssaname;
 	  si->dont_invalidate = true;
 	}
+      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" } } */