On Thu, Nov 13, 2014 at 3:20 PM, Teresa Johnson <tejohn...@google.com> wrote: > Here is the new patch. Bootstrapped and tested on > x86_64-unknown-linux-gnu. OK for trunk?
Ok for trunk and branches. Thanks, Richard. > Thanks, > Teresa > > 2014-11-13 <tejohn...@google.com> > > gcc: > PR tree-optimization/63841 > * tree.c (initializer_zerop): A constructor with no elements > does not zero initialize. > > gcc/testsuite: > PR tree-optimization/63841 > * g++.dg/tree-ssa/pr63841.C: New test. > > Index: tree.c > =================================================================== > --- tree.c (revision 217190) > +++ tree.c (working copy) > @@ -10330,6 +10330,8 @@ initializer_zerop (const_tree init) > { > unsigned HOST_WIDE_INT idx; > > + if (TREE_CLOBBER_P (init)) > + return false; > FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), idx, elt) > if (!initializer_zerop (elt)) > return false; > 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()); > + > + return good != bad; > +} > > On Wed, Nov 12, 2014 at 9:40 PM, Andrew Pinski <pins...@gmail.com> wrote: >> On Wed, Nov 12, 2014 at 9:38 PM, Teresa Johnson <tejohn...@google.com> wrote: >>> On Wed, Nov 12, 2014 at 9:30 PM, Andrew Pinski <pins...@gmail.com> wrote: >>>> On Wed, Nov 12, 2014 at 9:25 PM, Teresa Johnson <tejohn...@google.com> >>>> wrote: >>>>> Added testcase. Here is the new patch: >>>>> >>>>> 2014-11-12 <tejohn...@google.com> >>>>> >>>>> gcc: >>>>> PR tree-optimization/63841 >>>>> * tree.c (initializer_zerop): A constructor with no elements >>>>> does not zero initialize. >>>> >>>> Actually an empty constructor does zero initialize. A clobber does not. >>> >>> Ok, thanks, I wasn't sure. >>> >>>> >>>> The check you want is TREE_CLOBBER_P instead. >>> >>> So in initializer_zerop, a constructor that is a TREE_CLOBBER_P would >>> return false, right? I will make that change and retest. >> >> Yes that is correct. Note TREE_CLOBBER_P already checks if it is an >> constructor. >> >> Thanks, >> Andrew >> >>> >>> Thanks, >>> Teresa >>> >>>> >>>> Thanks, >>>> Andrew >>>> >>>> >>>>> >>>>> gcc/testsuite: >>>>> * g++.dg/tree-ssa/pr63841.C: New test. >>>>> >>>>> Index: tree.c >>>>> =================================================================== >>>>> --- tree.c (revision 217190) >>>>> +++ tree.c (working copy) >>>>> @@ -10330,6 +10330,8 @@ initializer_zerop (const_tree init) >>>>> { >>>>> unsigned HOST_WIDE_INT idx; >>>>> >>>>> + if (!CONSTRUCTOR_NELTS (init)) >>>>> + return false; >>>>> FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), idx, elt) >>>>> if (!initializer_zerop (elt)) >>>>> return false; >>>>> 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()); >>>>> + >>>>> + return good != bad; >>>>> +} >>>>> >>>>> On Wed, Nov 12, 2014 at 2:17 PM, Xinliang David Li <davi...@google.com> >>>>> wrote: >>>>>> missing test case? >>>>>> >>>>>> David >>>>>> >>>>>> On Wed, Nov 12, 2014 at 2:13 PM, Teresa Johnson <tejohn...@google.com> >>>>>> wrote: >>>>>>> This patch fixes an issue where tree-strlen was incorrectly removing a >>>>>>> store of 0 into a string because it thought a prior CLOBBER (which is >>>>>>> an empty constructor with no elements) was zero-initializing the >>>>>>> string. >>>>>>> >>>>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu. OK for trunk? >>>>>>> >>>>>>> Thanks, >>>>>>> Teresa >>>>>>> >>>>>>> 2014-11-12 <tejohn...@google.com> >>>>>>> >>>>>>> PR tree-optimization/63841 >>>>>>> * tree.c (initializer_zerop): A constructor with no elements >>>>>>> does not zero initialize. >>>>>>> >>>>>>> Index: tree.c >>>>>>> =================================================================== >>>>>>> --- tree.c (revision 217190) >>>>>>> +++ tree.c (working copy) >>>>>>> @@ -10330,6 +10330,8 @@ initializer_zerop (const_tree init) >>>>>>> { >>>>>>> unsigned HOST_WIDE_INT idx; >>>>>>> >>>>>>> + if (!CONSTRUCTOR_NELTS (init)) >>>>>>> + return false; >>>>>>> FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), idx, elt) >>>>>>> if (!initializer_zerop (elt)) >>>>>>> return false; >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 >>>>> >>>>> >>>>> >>>>> -- >>>>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 >>> >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413