VC++ compiler optimization switches

[BruceForstall]

eng\native\configureoptimization.cmake contains:

if(CLR_CMAKE_HOST_WIN32)                                                                        
    add_compile_options($<$<AND:$<COMPILE_LANGUAGE:C,CXX>,$<CONFIG:Debug>>:/Od>)                
    add_compile_options($<$<AND:$<COMPILE_LANGUAGE:C,CXX>,$<CONFIG:Checked>>:/O1>)              
    add_compile_options($<$<AND:$<COMPILE_LANGUAGE:C,CXX>,$<CONFIG:Release>>:/Ox>)              
    add_compile_options($<$<AND:$<COMPILE_LANGUAGE:C,CXX>,$<CONFIG:RelWithDebInfo>>:/O2>)       

I have some questions:

1. Why don't we use /O2 for all non-Debug builds?
2. Is RelWithDebInfo actually used? For what?

@dotnet/runtime-infrastructure

[ghost]

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

---

eng\native\configureoptimization.cmake contains:

if(CLR_CMAKE_HOST_WIN32)                                                                        
    add_compile_options($<$<AND:$<COMPILE_LANGUAGE:C,CXX>,$<CONFIG:Debug>>:/Od>)                
    add_compile_options($<$<AND:$<COMPILE_LANGUAGE:C,CXX>,$<CONFIG:Checked>>:/O1>)              
    add_compile_options($<$<AND:$<COMPILE_LANGUAGE:C,CXX>,$<CONFIG:Release>>:/Ox>)              
    add_compile_options($<$<AND:$<COMPILE_LANGUAGE:C,CXX>,$<CONFIG:RelWithDebInfo>>:/O2>)       

I have some questions:

1. Why don't we use /O2 for all non-Debug builds?
2. Is RelWithDebInfo actually used? For what?

@dotnet/runtime-infrastructure

Author:	BruceForstall
Assignees:	-
Labels:	area-Infrastructure-coreclr
Milestone:	-

[jkoritzinsky]

cc: @jkotas @janvorli

[jkotas]

It has been like that since we have open sourced: https://github.com/dotnet/coreclr/commits/release/2.1/src/pal/tools/windows-compiler-override.txt . I do not think we have history for this file prior to open sourcing.

The config for release is configuration is the only sensitive one, changing that would require performance testing. Feel free to clean up and unify the rest.

You may want to check impact of the checked build optimization settings on build performance. It is possible that /O1 was choosen as a trade-off between build and runtime performance.

You can also change https://github.com/dotnet/runtime/blob/main/src/libraries/Native/Windows/CMakeLists.txt to include the central optimization settings file while you are on it.

[BruceForstall]

cc @AndyAyersMS

[BruceForstall]

Well, it's messier than this. If I force a compiler error in a win-x64 release build, I see a command line with these optimization switches (this is a subset of the compiler switches):

/O2
/Ob2
/Ox
/Oi
/Oy-
/Gy

So we throw both /O2 and /Ox, which are mutually exclusive (which wins?). We also throw /Ob2 and /Oi (included in /Ox and /O2) and /Gy (included in /O2), but don't throw or disable /GF (string pooling), enabled in /O2 but not /Ox (making it important to know which one "wins"). I can't figure out where the /O2 and /Ob2 come from. Does ninja/cmake inject them somehow?

It looks like cmake does this. In artifacts\obj\coreclr\windows.x64.Release\CMakeCache.txt, I see:

//Flags used by the CXX compiler during RELEASE builds.
CMAKE_CXX_FLAGS_RELEASE:STRING=/O2 /Ob2 /DNDEBUG

Maybe we should clear out the cmake default values of this (and the other defaults:

CMAKE_CXX_FLAGS_DEBUG
CMAKE_CXX_FLAGS_MINSIZEREL
CMAKE_CXX_FLAGS_RELWITHDEBINFO

) to give us full control (and avoid ambiguity and warnings/errors from overriding switches).

[janvorli]

1. Why don't we use /O2 for all non-Debug builds?

As @jkotas said, that was the way it was set before we started the opensourcing of .NET. I am not sure if moving Checked to /O2 wouldn't make debugging experience much worse. I often use Checked builds to verify my changes using CoreCLR pri1 tests. the /O1 enables me to see a reasonable amount of locals to do basic debugging of problems so that I don't have to move to Debug build every time I want to debug an issue. But even the /O1 is missing plenty of locals / arguments and I often need to search them up the call stack. And Release builds that use /O2 are a real pain to debug.

2. Is RelWithDebInfo actually used? For what?

It is one of the standard CMake configurations (Debug, Release, RelWithDebInfo and MinSizeRel). We don't actively use it.

[janvorli]

I can't figure out where the /O2 and /Ob2 come from.

CMake has default settings of options for each compiler / build configuration. It contains these. I think it is a result of the change that @jkoritzinsky made in dotnet/coreclr#23897. We used to use CMAKE_USER_MAKE_RULES_OVERRIDE cmake variable to override the options, which is the way that was designed for that - CMake pulls in the file with overrides to cmake settings like the compiler options after it sets the defaults, but before the options are ever used. With the change, we've stopped using that mechanism and we have probably not realized that we somehow keep having the defaults, at least at some places.

However, MSVC doc says:

Compiler options are processed "left to right," and when a conflict is detected, the last (rightmost) option wins. The CL environment variable is processed before the command line, so in any conflicts between CL and the command line, the command line takes precedence.

See https://docs.microsoft.com/en-us/cpp/build/reference/compiling-a-c-cpp-program?view=msvc-160

[EgorBo]

Btw, -Os and -Oz save a lot of space when clang is used

but I never tested TP impact.

[BruceForstall]

I am not sure if moving Checked to /O2 wouldn't make debugging experience much worse.

For VC, /O1 is "optimize for size" and /O2 is "optimize for speed". I'd be surprised if the debugging experience was significantly different.

I often use Checked builds to verify my changes using CoreCLR pri1 tests.

I use Checked for almost everything (until I need to debug something), but I'd like it to be as fast as possible; I don't care about size.

CMake has default settings of options for each compiler / build configuration

I see we have set(CMAKE_CXX_FLAGS_CHECKED ""), probably because cmake doesn't have a concept of "Checked"? Maybe we could clear out the defaults here by set(CMAKE_CXX_FLAGS_RELEASE "") and same for Debug?

[BruceForstall]

When I switch "Checked" from -O1 to -O2, I see significant performance increases (for the JIT anyway):

1. 7% faster SuperPMI replay across all our collections
2. 14% instruction count reduction (measured using Intel's PIN tool) on a single SuperPMI replay of the benchmarks

The build is bigger: artifacts\bin\coreclr\windows.x64.Checked goes from 85,389,191 to 92,615,635 (presumably more aggressive inlining). E.g.,:

1. clrjit.dll: 3,528,704 => 3,947,008
2. coreclr.dll: 13,175,296 => 14,536,192

[janvorli]

I'd be surprised if the debugging experience was significantly different.

Hmm, so I wonder why debugging release / checked build is that much different experience. I have always believed that the optimization level is the reason for that.

[BruceForstall]

Hmm, so I wonder why debugging release / checked build is that much different experience. I have always believed that the optimization level is the reason for that.

It's possible that additional inlining makes debugging worse. However, my guess is that all the DEBUG code and contracts inhibits optimization, leading to slightly better debugging experience in Checked.

[BruceForstall]

I created a test PR to switch coreclr Windows Checked builds from -O1 to -O2 (#53888). Unfortunately, that leads to x86 test failures due to the CLR being very picky about low-level native compiler details (apparently).

[BruceForstall]

Windows non-x86 Checked builds have been switched to -O2. Windows x86 is blocked with this issue: #59845.

Windows Release builds should also be switched from -Ox to -O2.