[PATCH] Global merge on constants

[Duncan Sands]

Hi Quentin,

On 15/03/13 23:14, Quentin Colombet wrote:
> Hi Anton, Hi Duncan,
>
> We discussed offline with Bill on how we can improve the compile time of the
> proposed patch.
> What came out of this discussion is filtering out globals that are used in
> inline asm or intrinsic is useless.

I agree.  In my experience most inline asm that uses a global G and wants it
as a label will just use "G" directly in the asm rather than taking G as an
input.  Thus searching uses would never find it anyway.  I think you should
simply not merge globals which are in the llvm.used array, as "G" should be
for such asm to be correct.

  Therefore, we can focus on removing only the
> ones used in landing pad instruction (which is fast as the landing pad
> instructions are terminator instructions).
>
> What do you think?

It makes sense to me.

Ciao, Duncan.

>
> Thanks,
>
> -Quentin
>
> On Mar 15, 2013, at 1:46 PM, Quentin Colombet <qcolombet at apple.com
> <mailto:qcolombet at apple.com>> wrote:
>
>> Bill,
>>
>> Here are the numbers for SPEC.
>>
>> I’ve measure 3 different configurations:
>> 1. the compiler without the proposed patch (Reference)
>> 2. the compiler with the proposed patch but global merge disabled on constants
>> (i.e., default behavior after the patch applied)
>> 3. the compiler with the proposed patch and global merge enabled on constants
>> (i.e., with command line option global-merge-on-const).
>>
>> I have attached 2 main comparisons: 1 vs 2 and 1 vs 3 (identify with
>> ‘with_const’ for the latter).
>> This translates in 4 files because for each comparison I made a difference
>> between the tests with a compilation error I hit (“cannot find [some system
>> headers]”, these errors are the same between all the configurations) and the
>> tests without.
>>
>> SPEC_failsXXX report numbers with the sysroot include error.
>> SPECXXX report numbers without any errors.
>>
>> On average, the default behavior of the compiler does not impact the compile
>> time. When enabled, the constant processing has on average a negative impact
>> of 2%.
>>
>> -Quentin
>> <SPEC_fails-compile_time_with_const.txt>
>> <SPEC-compile_time_with_const.txt>
>> <SPEC_fails-compile_time.txt>
>> <SPEC-compile_time.txt>
>>
>> On Mar 15, 2013, at 11:33 AM, Quentin Colombet <qcolombet at apple.com
>> <mailto:qcolombet at apple.com>> wrote:
>>
>>> On Mar 15, 2013, at 10:49 AM, Bill Wendling <wendling at apple.com
>>> <mailto:wendling at apple.com>> wrote:
>>>
>>>> On Mar 15, 2013, at 9:51 AM, Quentin Colombet <qcolombet at apple.com
>>>> <mailto:qcolombet at apple.com>> wrote:
>>>>
>>>>> Hi Bill,
>>>>>
>>>>> I’ve attached the measurement made on compile time for O3.
>>>>> In that file, the Reference column is llvm r177120 without the proposed
>>>>> patch and Test is  the same compiler with the proposed patch and constant
>>>>> merging enabled.
>>>>> Both compilers have been generated without assertions and with
>>>>> optimizations (i.e., release mode).
>>>>>
>>>>> The speedup column reports a ratio: Test/Reference, thus the bigger the better.
>>>>>
>>>>> On average, the impact is limited (0.99 speedup, thus 1% slow-down). The
>>>>> maximum and minimum reported numbers are, in my opinion, not much relevant
>>>>> as the related tests (SingleSource/UnitTests/Vector/build and
>>>>> MultiSource/Benchmarks/MiBench/network-dijkstra/network-dijkstr) are
>>>>> compiled very quickly and the measurement method is not that accurate.
>>>>> Moreover, we have to keep in mind that this patch adds a check for a
>>>>> currently bogus behavior with global variables maked as “used”.
>>>>>
>>>>> Feel free to comment!
>>>>>
>>>> Hrm. I was hoping that the results would be no change. The slowdowns are
>>>> small, but then so are the test cases. Would you be able to run some of the
>>>> SPEC tests to see if it shows up there?
>>> Yes, sure.
>>>
>>>>
>>>> Sorry to ask you to do this. I know it's a pain. You may want to talk with
>>>> Nadav to see what he thinks. If he isn't concerned, then I will back off and
>>>> you can commit the new patch. :)
>>> Never mind, it’s just a command to run :).
>>>
>>>>
>>>> -bw
>>>
>>> -Quentin
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>