On Thu, Nov 13, 2014 at 6:32 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> 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 for catching that - copied from the original patch.
Will commit to trunk now then to gcc-4_9 after regression testing with
the branch.
Thanks,
Teresa
>
>> 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
--
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
