On Thu, Nov 13, 2014 at 12:55 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Thu, Nov 13, 2014 at 12:51:05PM -0800, Teresa Johnson wrote:
>> On Thu, Nov 13, 2014 at 7:39 AM, Teresa Johnson <tejohn...@google.com> wrote:
>> > On Thu, Nov 13, 2014 at 7:12 AM, Jakub Jelinek <ja...@redhat.com> wrote:
>> >> And for release branches I'd really prefer tree-ssa-strlen.c change.
>> >
>> > Ok, I started testing the initializer_zerop change on the 4_9 branch,
>> > will also test the strlen fix and send that patch for review here when
>> > it completes.
>>
>> Here is the more conservative patch for 4_9. Bootstrapped and tested
>> on x86_64-unknown-linux-gnu. Ok for gcc-4_9 branch?
>
> Ok, thanks. But I have a comment regarding the test below:
>
>> 2014-11-13 Teresa Johnson <tejohn...@google.com>
>>
>> gcc:
>> PR tree-optimization/63841
>> * tree-ssa-strlen.c (strlen_optimize_stmt): Ignore clobbers.
>>
>> 2014-11-13 Teresa Johnson <tejohn...@google.com>
>>
>> gcc/testsuite:
>> PR tree-optimization/63841
>> * testsuite/g++.dg/tree-ssa/pr63841.C: New test.
>>
>> Index: tree-ssa-strlen.c
>> ===================================================================
>> --- tree-ssa-strlen.c (revision 217503)
>> +++ tree-ssa-strlen.c (working copy)
>> @@ -1856,7 +1856,7 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi)
>> break;
>> }
>> }
>> - else if (is_gimple_assign (stmt))
>> + else if (is_gimple_assign (stmt) && !gimple_clobber_p (stmt))
>> {
>> tree lhs = gimple_assign_lhs (stmt);
>>
>> Index: testsuite/g++.dg/tree-ssa/pr63841.C
>> ===================================================================
>> --- testsuite/g++.dg/tree-ssa/pr63841.C (revision 0)
>> +++ testsuite/g++.dg/tree-ssa/pr63841.C (working copy)
>> @@ -0,0 +1,38 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O2" } */
>> +
>> +#include <cstdio>
>> +#include <string>
>> +
>> +std::string __attribute__ ((noinline)) comp_test_write() {
>> + std::string data;
>> +
>> + for (int i = 0; i < 2; ++i) {
>> + char b = 1 >> (i * 8);
>> + data.append(&b, 1);
>> + }
>> +
>> + return data;
>> +}
>> +
>> +std::string __attribute__ ((noinline)) comp_test_write_good() {
>> + std::string data;
>> +
>> + char b;
>> + for (int i = 0; i < 2; ++i) {
>> + b = 1 >> (i * 8);
>> + data.append(&b, 1);
>> + }
>> +
>> + return data;
>> +}
>> +
>> +int main() {
>> + std::string good = comp_test_write_good();
>> + printf("expected: %hx\n", *(short*)good.c_str());
>> +
>> + std::string bad = comp_test_write();
>> + printf("got: %hx\n", *(short*)bad.c_str());
>
> Supposedly the printfs should have been removed and the #include <cstdio>
> isn't needed then either. No need to clutter the test output and log files.
> On the other side, tests should abort (); or __builtin_abort (); on failure,
> not just return non-zero.
Ok, sounds good. Here is the new patch. Confirmed that the test case
passes with the fix, aborts without it.
If this looks good I will also update the trunk version of the test.
Thanks,
Teresa
2014-11-13 Teresa Johnson <tejohn...@google.com>
gcc:
PR tree-optimization/63841
* tree-ssa-strlen.c (strlen_optimize_stmt): Ignore clobbers.
2014-11-13 Teresa Johnson <tejohn...@google.com>
gcc/testsuite:
PR tree-optimization/63841
* testsuite/g++.dg/tree-ssa/pr63841.C: New test.
Index: tree-ssa-strlen.c
===================================================================
--- tree-ssa-strlen.c (revision 217503)
+++ tree-ssa-strlen.c (working copy)
@@ -1856,7 +1856,7 @@ strlen_optimize_stmt (gimple_stmt_iterator *gsi)
break;
}
}
- else if (is_gimple_assign (stmt))
+ else if (is_gimple_assign (stmt) && !gimple_clobber_p (stmt))
{
tree lhs = gimple_assign_lhs (stmt);
Index: testsuite/g++.dg/tree-ssa/pr63841.C
===================================================================
--- testsuite/g++.dg/tree-ssa/pr63841.C (revision 0)
+++ testsuite/g++.dg/tree-ssa/pr63841.C (working copy)
@@ -0,0 +1,35 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include <string>
+
+std::string __attribute__ ((noinline)) comp_test_write() {
+ std::string data;
+
+ for (int i = 0; i < 2; ++i) {
+ char b = 1 >> (i * 8);
+ data.append(&b, 1);
+ }
+
+ return data;
+}
+
+std::string __attribute__ ((noinline)) comp_test_write_good() {
+ std::string data;
+
+ char b;
+ for (int i = 0; i < 2; ++i) {
+ b = 1 >> (i * 8);
+ data.append(&b, 1);
+ }
+
+ return data;
+}
+
+int main() {
+ std::string good = comp_test_write_good();
+ std::string bad = comp_test_write();
+
+ if (good != bad)
+ __builtin_abort ();
+}
>
>> +
>> + return good != bad;
>> +}
>
> Jakub
--
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
