On Thu, Nov 13, 2014 at 3:32 PM, Richard Biener
<richard.guent...@gmail.com> wrote:
> 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.
Err - please fix the changelog entry wording to "A clobber does
not zero inintiaize"
> 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
