[PATCH] D39885: Don't enable ICF by default

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In https://reviews.llvm.org/D39885#921452, @pcc wrote:

> > somewhat expensive
>
> Do you have numbers?


For chrome's content.dll:
default
real    0m42.833s
real    0m39.829s
real    0m40.380s
real    0m40.008s
real    0m39.462s
/OPT:NOICF
real    0m37.458s
real    0m39.222s
real    0m38.813s
real    0m38.316s
real    0m38.276s
/OPT:NOREF (implies noicf)
real    0m39.924s
real    0m39.251s
real    0m40.284s
real    0m42.717s
real    0m40.270s

>> hurts debuggability
> 
> According to https://msdn.microsoft.com/en-us/library/bxwfs976.aspx link.exe disables ICF by default when linking with /debug. We could do the same.

Yeah, let's do what they do.

>> technically non-conforming.
> 
> True, but from a practical perspective, programs that have been linked with link.exe are unlikely to break as a result of the default.
> 
>> disabling /OPT:REF by default
> 
> We might actually consider doing that in /debug builds because it should provide a better debugging experience.

I suppose users might be confused if they can't find something unreferenced in their binary. I hadn't considered that.


https://reviews.llvm.org/D39885