[PATCH] Global merge on constants

Mon Mar 18 13:28:41 PDT 2013

Thanks for the feedbacks Duncan.

Here is the updated version of the patch as well as the updated compile time numbers.
The updated patch just checks for the “used” attribute and the uses in landing pad instructions. More particularly, it looks for all landing pad instructions in the module and marks internally the involved global variables as “must keep”.

For the compile time numbers, on average performing the new checks plus merging the constants (SPEC-compile_time_with_const.txt) does not have any noticeable impact on the compile time now.

Cheers, 
-Quentin

On Mar 16, 2013, at 12:57 AM, Duncan Sands <baldrick at free.fr> wrote:

> 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130318/561b8d0a/attachment.html>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: SPEC-compile_time_with_const.txt
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130318/561b8d0a/attachment.txt>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130318/561b8d0a/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: GlobalMergeOnConst.patch
Type: application/octet-stream
Size: 8144 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130318/561b8d0a/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130318/561b8d0a/attachment-0002.html>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: SPEC-compile_time.txt
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130318/561b8d0a/attachment-0001.txt>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130318/561b8d0a/attachment-0003.html>