Skip to content

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

Open
mergify[bot] wants to merge 1 commit intoign-cmake2from
mergify/bp/ign-cmake2/pr-532
Open

Make Whole Program Optimization (WPO) optional on MSVC (backport #532)#536
mergify[bot] wants to merge 1 commit intoign-cmake2from
mergify/bp/ign-cmake2/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/ign-cmake2/pr-532
Your branch is up to date with 'origin/ign-cmake2'.

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/IgnSetCompilerFlags.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/ign-cmake2/pr-532 branch from 833da47 to b81cba3 Compare March 25, 2026 17:12
@azeey azeey removed the conflicts label Mar 25, 2026
set(MSVC_RELEASE_FLAGS "${MSVC_DEBUG_FLAGS} /GL")

# 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.

Same comment as #533 (comment). @j-rivero mind taking a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏯 fortress Ignition Fortress

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

1 participant