Skip to content

Make Whole Program Optimization (WPO) optional on MSVC (backport #532)#533

Merged
j-rivero merged 1 commit intogz-cmake3from
mergify/bp/gz-cmake3/pr-532
Mar 26, 2026
Merged

Make Whole Program Optimization (WPO) optional on MSVC (backport #532)#533
j-rivero merged 1 commit intogz-cmake3from
mergify/bp/gz-cmake3/pr-532

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Mar 25, 2026

🦟 Bug fix

Summary

This PR introduces an option to toggle Whole Program Optimization (WPO) on MSVC. On template heavy libraries such as gz-physics, WPO causes a large memory spike causing the system to do a lot of swapping thereby dramatically increasing the build time. The current build time for gz-physics PRs on the main branch is 1h 17m (https://build.osrfoundation.org/job/gz_physics-pr-cnlwin/232/timings/). With WPO disabled, it is reduced to 29m 🎉 (https://build.osrfoundation.org/job/gz_physics-pr-cnlwin/234/timings).

This PR also fixes a few other bugs in the way flags are set on CMAKE variables:

  • Fixed missing LTCG flags: Corrects an issue where the /LTCG linker flag was only being applied to RelWithDebInfo shared libraries. It is now correctly applied across all Release and RelWithDebInfo target types (executables, shared libraries, and static libraries) when WPO is enabled. This fixes the warnings in most of our windows builds, e.g.:
    Environment.obj : MSIL .netmodule or module compiled with /GL found; restarting link with /LTCG; add /LTCG to the link command line to improve linker performance
    
  • Added /bigobj to minimal MSVC flags: Included the /bigobj flag in MSVC_MINIMAL_FLAGS. This prevents C1128: number of sections exceeded object file format limit errors, which I believe now occurs because WPO is properly set on all targets (see https://build.osrfoundation.org/job/gz_sim-pr-cnlwin/668/).

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the fix (as needed)
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

Generated-by: Gemini 3.0 Pro

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

Backports: If this is a backport, please use Rebase and Merge instead.


This is an automatic backport of pull request #532 done by Mergify.

@mergify mergify bot requested review from j-rivero and scpeters as code owners March 25, 2026 16:19
@mergify mergify bot added the conflicts label Mar 25, 2026
@mergify
Copy link
Contributor Author

mergify bot commented Mar 25, 2026

Cherry-pick of a3c8e29 has failed:

On branch mergify/bp/gz-cmake3/pr-532
Your branch is up to date with 'origin/gz-cmake3'.

You are currently cherry-picking commit a3c8e29.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   cmake/GzSetCompilerFlags.cmake

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

* Add GZ_MSVC_WPO option to toggle Whole Program Optimization

This adds a \GZ_MSVC_WPO\ option (default ON) that controls the
injection of \/GL\ and \/LTCG\ on MSVC. It also fixes a bug where
\/LTCG\ was only applied to RelWithDebInfo shared linker flags, and not
to Release or other target types. \/INCREMENTAL:NO\ is appropriately
restricted to RelWithDebInfo.

Generated-By: Gemini 3.1 Pro
Signed-off-by: Addisu Z. Taddese <addisuzt@intrinsic.ai>

* Add /bigobj flag to MSVC_MINIMAL_FLAGS

Increases the number of sections in an object file to prevent
C1128 errors when building complex templated code with Whole
Program Optimization (/GL) enabled.

Generated-By: Gemini 3.0 Pro

---------

Signed-off-by: Addisu Z. Taddese <addisuzt@intrinsic.ai>
Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>
(cherry picked from commit a3c8e29)
Signed-off-by: Addisu Z. Taddese <addisuzt@intrinsic.ai>
@azeey azeey force-pushed the mergify/bp/gz-cmake3/pr-532 branch from 53f9b38 to a156dc8 Compare March 25, 2026 17:05
@azeey azeey removed the conflicts label Mar 25, 2026
# UNDEBUG: Undefine NDEBUG so that assertions can be triggered
set(MSVC_RELWITHDEBINFO_FLAGS "${MSVC_RELEASE_FLAGS} /UNDEBUG")
# UNDEBUG: Undefine NDEBUG so that assertions can be triggered
set(MSVC_RELWITHDEBINFO_FLAGS "${MSVC_RELEASE_FLAGS} /UNDEBUG")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#532 does not have /UNDEBUG, so this line caused a conflict. When resolving it, I chose to keep the /UNDEBUG because removing it would probably cause a behavior change. The PR that removes it was added to main before gz-cmake4 was released (see #418).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good decision to me, let's keep it.

@azeey
Copy link
Contributor

azeey commented Mar 25, 2026

@j-rivero could you give this a look since I had to resolve conflicts?

@github-project-automation github-project-automation bot moved this from Inbox to In review in Core development Mar 26, 2026
@j-rivero j-rivero merged commit 336bf25 into gz-cmake3 Mar 26, 2026
9 checks passed
@j-rivero j-rivero deleted the mergify/bp/gz-cmake3/pr-532 branch March 26, 2026 11:44
@github-project-automation github-project-automation bot moved this from In review to Done in Core development Mar 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎵 harmonic Gazebo Harmonic

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants