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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a subscriber: rnk.
pcc added a comment.

In https://reviews.llvm.org/D30363#687719, @majnemer wrote:

> MSVC won't merge read only data COMDATs? Microsoft's documentation has the following:
>
> > When /OPT:REF is enabled either explicitly or by default, a limited form of /OPT:ICF is enabled that only folds identical functions. If you want /OPT:REF but not /OPT:ICF, you must specify either /OPT:REF,NOICF or /OPT:NOICF.
>
>
>
> > The linker behaves differently when /OPT:REF is specified—and ICF is in effect by default—than it does when /OPT:REF,ICF is specified explicitly. The form of ICF that's enabled with /OPT:REF alone does not fold read-only data—this includes .rdata, .pdata, and .xdata. Therefore, fewer functions are folded when images are produced for x64 because functions in these modules are more dependent on read-only data—for example, .pdata and .xdata. To get full ICF folding behavior, explicitly specify /OPT:ICF.
>
> It sounds like Microsoft's default behavior is /OPT:REF. With that flag, they will fold identical functions. However, they provide access to full folding using /OPT:REF,ICF
>
> Perhaps we should provide the functionality via a similar flag?


Maybe, but I'd slightly prefer to encode the information in the object files rather than introducing another flag.

Although it looks like Chromium Windows builds are already using /OPT:REF and /OPT:ICF:
https://cs.chromium.org/chromium/src/build/config/win/BUILD.gn?type=cs&q=%22opt:icf%22&sq=package:chromium&l=102
(but only when building with Clang or fastlink because /PROFILE implies /OPT:NOICF contrary to what the comment says (!))

So now I'm confused as to how we were running into problems before.


https://reviews.llvm.org/D30363