[PATCH] D30363: COFF ICF: Merge only functions. Do not merge read-only data.

[Rui Ueyama via llvm-commits]

That note is mentioned in the context of whole program optimization.
Doesn't it just say that whole program analysis can know whether an address
of a variable is taken or not?

On Wed, Mar 1, 2017 at 7:17 PM, David Majnemer <david.majnemer at gmail.com>
wrote:

> I think I found out why: https://blogs.msdn.microsoft.com/vcblog/2013/09/
> 11/introducing-gw-compiler-switch/
>
> "*Please note*, the ICF optimization will only be applied for identical
> COMDATs where their address is not taken, and they are read only. If a data
> is not address taken, then breaking address uniqueness by ICF won’t lead to
> any observable difference, thus it is valid and conformant to the standard."
>
> On Wed, Mar 1, 2017 at 3:40 PM Peter Collingbourne via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> pcc added a comment.
>>
>> In https://reviews.llvm.org/D30363#689809, @rnk wrote:
>>
>> > In https://reviews.llvm.org/D30363#687837, @pcc wrote:
>> >
>> > > Reid, I think you mentioned that you saw this problem only with LLD,
>> do you have any more details?
>> >
>> >
>> > The one instance that I remember was http://crbug.com/635943. I think
>> there was one other issue much earlier on that didn't require a
>> restructuring.
>>
>>
>> Standalone reproducer:
>>
>>   C:\src\icftest>type icftest.cc
>>   #include <stdio.h>
>>
>>   extern const wchar_t foo[];
>>   extern const wchar_t bar[];
>>
>>   const wchar_t foo[] = L"";
>>   const wchar_t bar[] = L"";
>>
>>   int main() {
>>     printf("%p %p\n", &foo, &bar);
>>   }
>>
>>   C:\src\icftest>cl /Gw /O2 /c icftest.cc
>>   Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24215.1 for x64
>>   Copyright (C) Microsoft Corporation.  All rights reserved.
>>
>>   icftest.cc
>>
>>   C:\src\icftest>link icftest.obj /opt:icf
>>   Microsoft (R) Incremental Linker Version 14.00.24215.1
>>   Copyright (C) Microsoft Corporation.  All rights reserved.
>>
>>
>>   C:\src\icftest>icftest
>>   000000013F2C12C0 000000013F2C12C4
>>
>>   C:\src\icftest>..\llvm-project\ra\bin\lld-link icftest.obj /opt:icf
>>
>>   C:\src\icftest>icftest
>>   000000013FC02088 000000013FC02088
>>
>> So it does appear that MSVC's behaviour is not as documented at least
>> some of the time. In that case I think I'd be fine with this change with an
>> explanatory comment.
>>
>>
>> https://reviews.llvm.org/D30363
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170306/2a2ab4e0/attachment.html>