[PATCH] Remove "localize global" optimization

[Chandler Carruth]

On Mon, Oct 7, 2013 at 11:28 AM, Evan Cheng <evan.cheng at apple.com> wrote:

> Any change that has the potential to impact performance widely requires
> this investigation *before* the patch is committed.


Evan, while I completely agree with you on this specific issue, I don't
fully agree with this general claim... It's close to what I follow and
encourage others to follow, but with some important differences.

For changes which have the potential to impact performance significantly
for a few benchmarks, but are reasonably small and isolated changes to make
(such as, adding a new collection of simplifications to instcombine, or
switching the canonical form from A to B in the IR), I think it's often
sufficient to measure performance of the test suite on your architecture
(provided it is reasonably representative, x86 or arm tend to be
sufficient) and commit. We have on-commit benchmarking from many companies
that will catch regressions and let us back out the change as needed.

For changes which have really far reaching impacts on performance and we
expect essentially a mixture of improvements and regressions across a wide
range of benchmarks, I think we *have* to commit the change so that people
can evaluate it, but we *have* to keep it behind a flag so that people can
run A/B tests that isolate the impact of the change. Examples of these
types of changes that have thus far all been carried out in this way:
1) SROA -- it changed the canonical form for many inputs by scalarizing
them further. feedback from others helped isolate several bugs
2) Vectorization -- we had a flag to enable it for a while before actually
making it on-by-default. it helped isolate a couple of bugs
3) Switch to MI sched -- its under a flag right now, and I've been able to
run lots of benchmarks with it

Note that both of these strategies still have people commit the change
prior to widespread testing on a wide variety of hardware and benchmarks. I
think that's actually a good thing. It ensures we continue to make
incremental progress on the codebase independently of the (often slow) time
cycle of running benchmarks.

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