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

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 1 19:17:01 PST 2017


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/20170302/3b1e31fc/attachment.html>


More information about the llvm-commits mailing list