[PATCH] D15784: Enable 2 warnings on MSVC, turn on StringPooling & intrinsic functions

[Alexander Riccio via llvm-commits]

ariccio added a comment.

Do I have to submit a comment to "submit" an //inline// comment??


================
Comment at: cmake/modules/HandleLLVMOptions.cmake:369
@@ +368,3 @@
+  # Eliminate Duplicate Strings
+  append("/GF" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+
----------------
aaron.ballman wrote:
> >> I think this one is reasonable, but I worry about it requiring use of /bigobj.
> > MSVC didn't complain about it in the debug build, so my guess is that this isn't a problem, but I'll compile a release build - optimizations might somehow pack more strings into a single image? - and see if it causes problems there. Can you think of any situation that might actually include more than 65,536 strings?
> 
> From MSDN:
> 
> > The /GF compiler option creates an addressable section for each unique string. And by default, an object file can contain up to 65,536 addressable sections. If your program contains more than 65,536 strings, use the /bigobj compiler option to create more sections.
> 
> The compiler has a lot of unique strings, which means that this option will create a lot of addressable sections. My gut feeling (not based on any research) is that our duplicate string literals are generally going to be short strings, and not repeated overly often. I would guess that our unique string literals dwarf the duplicates, and this is unlikely to have a huge size savings, but may push us over the bigobj limit.
> 
> >> What are the size differences for our executables with this option disabled vs enabled?
> > Ok, I'm embarrassed to say it, but I don't have any data for this. LLVM just takes too long to compile for me to check the executable size for such a small patch. Is that reasonable, or should I actually compare them?
> 
> You should actually compare them. When changing the compiler switches like this, you want to make sure that (1) the projects all still compile (perhaps there are code changes required to enable these flags), and (2) that check-all still runs cleanly (code could have been unintentionally relying on a particular optimization strategy, there are optimizer bugs, etc). This also lets you get the size statistics as well.
Before anything: (1), and (2) still pass - on my machine, two check-all tests fail before and after, because Python.exe is in a path that contains spaces.

> This also lets you get the size statistics as well.

Any tips on how to get the size stats? Surely, there's a better way to do it than manually?




http://reviews.llvm.org/D15784