hans marked 7 inline comments as done. hans added a comment. > Thoughts on "As far as I can tell we do not make > /https://reviews.llvm.org/owners/package/1/ and > /https://reviews.llvm.org/owners/package/2/ imply /Gs -- we keep it at the > default value of 4096 (?) Fixing this probably means increasing chrome's size > (?)."?
The -mstack-probe-size option and /Gs was added later than the /O options (in r226601) so that's why it wasn't hooked up to /https://reviews.llvm.org/owners/package/1/ and /https://reviews.llvm.org/owners/package/2/ originally. But yes, making /https://reviews.llvm.org/owners/package/1/ and /https://reviews.llvm.org/owners/package/2/ imply /Gs seems like a bad idea. That would mean emitting __chkstk in every function, increasing both size and slowing down execution. Even MSVC's docs say "/Gs0 activates stack probes for every function call that requires storage for local variables. This can have a negative impact on performance." (and "If the /Gs option is specified without a size argument, it is the same as specifying /Gs0"). Actually, trying this out with MSVC, I don't see any __chkstk calls with /https://reviews.llvm.org/owners/package/1/, or with eg. /Gs1 for that matter: a.cc: int f(int x) { volatile int a[128] = {0}; return a[0]; cl /c /O1 a.cc && dumpbin /disasm a.obj ... ?f@@YAHH@Z (int __cdecl f(int)): 00000000: 55 push ebp 00000001: 8B EC mov ebp,esp 00000003: 81 EC 00 02 00 00 sub esp,200h 00000009: 68 00 02 00 00 push 200h 0000000E: 8D 85 00 FE FF FF lea eax,[ebp-200h] 00000014: 6A 00 push 0 00000016: 50 push eax 00000017: E8 00 00 00 00 call _memset 0000001C: 8B 85 00 FE FF FF mov eax,dword ptr [ebp-200h] 00000022: 83 C4 0C add esp,0Ch 00000025: 8B E5 mov esp,ebp 00000027: 5D pop ebp 00000028: C3 ret Maybe MSVC started ignoring /Gs but didn't update the docs? https://reviews.llvm.org/D52266 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
