[PATCH] Remove "localize global" optimization

[Nick Lewycky]

On 7 October 2013 17:09, Chandler Carruth <chandlerc at google.com> wrote:

>
> On Mon, Oct 7, 2013 at 4:27 PM, Jim Grosbach <grosbach at apple.com> wrote:
>
>> Good thoughts, Chandler. Do I understand you correctly that you agree
>> with Evan’s statement with the modification of s/committed/enabled by
>> default/?
>
>
> Essentially, although I tried to be a bit more clear about which buckets
> different optimizations fall into. For example, in this case, I don't think
> this change would would have fallen into the bucket where you really need
> to give some significant heads up prior to enabling by default.
>
> I think the problem is this case was very different -- the people making
> the change *knew* there were a small number of important benchmarks that
> regressed, and failing to address that prior to enabling or committing
> something is quite different.
>

In order for this optimization to fire at -O2, you need to not use local
variables and instead write them as global variables and only use them from
a single function. A single source-level function, because this
optimization occurs before inlining.

I wasn't aware of a small number of important tests. I'd heard from
somebody once upon a time that there's a single test in SPEC which did
this, but that's all I know. I don't know which benchmark in SPEC, or
whether it exists in the current version of SPEC. I judged, based on the
workings of the optz'n, that it was a silly optimization and unlikely to be
very important, doubly so considering that it risks miscompiling if it ever
fires-- a strong suggestion that it never fires.

It wasn't until Evan's complaint that I learned that this pattern exists in
two tests in the nightly test suite, which was a real shock to me. But
please don't conflate that with what we knew before it was committed.

I do know this micro-opt miscompiles. Perhaps it only got the performance
it did because it went down wrong code paths, ultimately emitting the right
text following wrong reasoning to get there. Unless someone proves
otherwise, why should we be bound by the performance this test used to get?

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131007/cc3a112e/attachment.html>