On Thu, Nov 13, 2014 at 7:12 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Thu, Nov 13, 2014 at 06:46:23AM -0800, Teresa Johnson wrote:
>> >> --- 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;
>> >
>> > Wrong formatting.
>>
>> Sorry, not sure I understand why? My mailer does tend to eat spaces,
>> but it is indented the correct amount and I think has the right spaces
>> within the line.
>
> I meant that you are using spaces instead of tabs and the unsigned line
> above it being one char off suggested to me visually it has the tab in there
> (apparently it is eaten too). Use MUA that doesn't eat it, or convince your
> employer that tabs are important in e-mails? ;)
>
> If you commit tabs in there, it is fine with me.
Changed the spaces to tabs to match the surrounding lines and
committed to trunk (r217505).
>
>> Ok, I have held off on my commit for now. I considered fixing this in
>> tree-ssa-strlen, but thought this was a potentially larger problem
>> with initializer_zerop, where it shouldn't be considering a clobber to
>> be a zero init and might be affecting other callers as well.
>
> As I said, I think your tree.c change is useful, but perhaps too risky
> for the release branches, because we don't know what all it will affect?
>
>> If we make the change to initializer_zerop is it still useful to
>> change tree-strlen?
>
> I think it is better to be clear on it that right now we want clobbers
> to clobber the memory for strlen pass purposes, maybe in the future we want
> to perform some optimizations for them as Richard suggested in the PR
> (though, extending the lifetime in that case by removing the clobber
> shouldn't be done unconditionally (i.e. limited to when we'd actually want
> to benefit from the known length) and assuming we are close to the clobber
> (i.e. it is fine to extend it a little bit, but not extend the lifetime
> across multi-megabyte function e.g.).
> 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.
Thanks,
Teresa
>
> Jakub
--
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
