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

[Peter Collingbourne via Phabricator via llvm-commits]

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