[llvm-commits] [llvm] r169079 [1/2] - in /llvm/trunk: lib/Transforms/IPO/GlobalOpt.cpp test/Transforms/GlobalOpt/big-memory-footprint.ll

Andrew Trick atrick at apple.com
Sun Dec 2 13:59:39 PST 2012


On Dec 1, 2012, at 6:45 PM, Nick Lewycky <nicholas at mxc.ca> wrote:

> Zhou Sheng wrote:
>> Author: sheng
>> Date: Fri Nov 30 22:38:53 2012
>> New Revision: 169079
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=169079&view=rev
>> Log:
>> The patch is to improve the memory footprint of pass GlobalOpt.
>> Also check in a case to repeat the issue, on which 'opt -globalopt' consumes 1.6GB memory.
>> The big memory footprint cause is that current GlobalOpt one by one hoists and stores the leaf element constant into the global array, in each iteration, it recreates the global array initializer constant and leave the old initializer alone. This may result in many obsolete constants left.
>> For example:  we have global array @rom = global [16 x i32] zeroinitializer
>> After the first element value is hoisted and installed:   @rom = global [16 x i32] [ 1, 0, 0, ... ]
>> After the second element value is installed:  @rom = global [16 x 32] [ 1, 2, 0, 0, ... ]        // here the previous initializer is obsolete
>> ...
>> When the transform is done, we have 15 obsolete initializers left useless.
> 
> It's great to have extra details like how you have a testcase on which globalopt takes 1.6GB, but you failed to explain the change you made. The change improves the memory footprint, but what is the change?
> 
> I think I understand it after reading the code, but please remember that about commit messages in the future.

Some of this explanation should also take the form of code comments--including your justification for the SetVector requirement that you explained to Kuba on this list.

Your test case doesn't look like a unit test that should be run by lit. I don't think we want an 8k IR test checked in, and I tend to think lit tests should complete in milliseconds.

It might suffice to checkin a very small test that you happen to know exercises the corner cases of your code. You can only CHECK that GlobalOpt still worked as expected, but it's better than nothing.

-Andy




More information about the llvm-commits mailing list