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 <pola...@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.
--- 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" } } */
Marek
