[PATCH] Remove "localize global" optimization

Alexey Samsonov samsonov at google.com
Wed Oct 2 11:20:45 PDT 2013


On Wed, Oct 2, 2013 at 9:52 PM, Bob Wilson <bob.wilson at apple.com> wrote:

>
> On Oct 2, 2013, at 10:08 AM, Bob Wilson <bob.wilson at apple.com> wrote:
>
>
> On Oct 2, 2013, at 8:33 AM, Alexey Samsonov <samsonov at google.com> wrote:
>
>
> On Tue, Oct 1, 2013 at 10:39 PM, Bob Wilson <bob.wilson at apple.com> wrote:
>
>>
>> On Oct 1, 2013, at 10:20 AM, Nick Lewycky <nlewycky at gmail.com> wrote:
>>
>> >
>> >  The optimization was "important" for some SPEC test, but I think we're
>> past caring about that.
>>
>> Not necessarily.  Has anyone measured the impact on SPEC and other
>> benchmarks?
>>
>
> I'm going to submit this anyway.
>
>
> That's not very friendly.  You may not care about SPEC but that doesn't
> mean that no one else cares.
>
> See Reid's comment in http://llvm-reviews.chandlerc.com/D1754 - looks
> like this optimization has incorrect assumptions about C programs.
>
>
> The only comment I see from Reid there is this: "I missed any earlier
> discussion about this. Don't we want to keep this optimization, even if
> it's a real corner case that fires once in a blue moon? isLeakCheckerRoot()
> doesn't check any flags to see if any of the sanitizers are on."
>
>
> OK, I see the problem, and I can't think of a way to fix it.
>

> But, if this hurts SPEC results, we may need to investigate alternative
> ways to get the same results.  Since you're the one pushing this, it would
> have been nice if you contributed to that effort by at least providing the
> performance data.  You could still do that now.  Please do.
>

Yes, the comment didn't appear in Phabricator, only the review thread:

"The C++ standard sez [basic.start.main]p6: "The function main shall not be
used (3.2) within a program."  The C standard has no such language, so this
optimization isn't really valid for C.  IIRC this caused PNaCl some
problems."

I will run the SPEC benchmarks and measure the effects of this change.
Sorry for not doing this as a first step.

-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131002/7b2f0bc9/attachment.html>


More information about the llvm-commits mailing list