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
