Fix PGO instrumentation for libcoreclr.so static libraries

[EgorBo]

Note

This PR was created with the assistance of GitHub Copilot.

Fixes #124141

On non-Windows, add_pgo(coreclr) used target_compile_options(PRIVATE) which only applied -fprofile-instr-generate/-fprofile-instr-use to the 2 source files directly compiled by the coreclr target (mscoree.cpp, exports.cpp). The runtime's real code lives in static libraries (cee_wks, utilcode, coreclrpal, etc.) that were linked in without instrumentation. This doesn't affect clrjit (all JIT sources compile directly into the shared library) or Windows (PGO uses link-time /LTCG flags).

Fix: add directory-scoped PGO compile flags in pgosupport.cmake so all C/C++ sources in the coreclr tree get the flags. The existing add_pgo() is unchanged — it still sets per-target LTO link flags. Assembly files are excluded via $<$<COMPILE_LANGUAGE:C,CXX>:...>.

Before (instrumented libcoreclr.so)

Section	Size
__llvm_prf_names	—
__llvm_prf_cnts	—
__llvm_prf_data	—
Estimated profiled functions	~30

After (instrumented libcoreclr.so)

Section	Size
__llvm_prf_names	283 KB
__llvm_prf_cnts	801 KB
__llvm_prf_data	1,094 KB
Estimated profiled functions	~20,000

No changes needed in dotnet-optimization — the training pipeline already collects all *.profraw files and merges them into coreclr.profdata.

[Copilot]

Pull request overview

This PR updates CoreCLR’s native build configuration so Profile Guided Optimization (PGO) compile flags apply broadly across the src/coreclr C/C++ compilation units on non-Windows builds, rather than only affecting the few sources compiled directly into a shared library target.

Changes:

• Adds directory-scoped -fprofile-instr-generate / -fprofile-instr-use compile options for non-Windows PGO builds (C/C++ only).
• Adds a directory-scoped link option for instrumentation builds to ensure profiling runtime symbols resolve when instrumented static libraries are linked into shared libraries.
• Looks for a global coreclr.profdata to enable PGO optimization flags across the CoreCLR tree when present.

[github-actions]

Code Review: Fix PGO instrumentation for libcoreclr.so static libraries

Summary

This PR adds directory-scoped PGO compile flags (add_compile_options/add_link_options) in pgosupport.cmake so that all C/C++ sources under src/coreclr/ — including static libraries like cee_wks, utilcode, coreclrpal — receive instrumentation/optimization flags on non-Windows. Previously, add_pgo(coreclr) only applied compile flags to the 2 source files directly compiled by the coreclr shared library target (mscoree.cpp, exports.cpp), yielding ~30 profiled functions instead of ~20,000.

Verdict: ✅ Approve (no blocking issues)

The fix is correct, well-scoped, well-commented, and directly addresses the root cause described in #124141. The approach is sound: directory-scope compile flags propagate into static library sub-targets, while add_pgo() continues to own the per-target LTO link flags.

---

Detailed Findings

⚠️ Warning: Directory scope covers more than libcoreclr.so constituents

Since pgosupport.cmake is include()d from src/coreclr/CMakeLists.txt (line 53), add_compile_options applies to all subdirectories: nativeaot/, tools/, ilasm/, ildasm/, hosts/, etc. — not just the static libraries that feed into libcoreclr.so. During instrumentation, this means NativeAOT and build-time tools will also emit profiling probes. During optimization, those targets will receive -fprofile-instr-use with coreclr.profdata, which won't have matching profile data for their functions (suppressed by -Wno-profile-instr-unprofiled, so no build failures).

This is likely harmless in practice — instrumented tools run once during training and the mismatch warnings are suppressed — but it's worth documenting the intent. If the intent is truly "only coreclr static lib constituents," a narrower scope (e.g., guarding with a variable set before specific add_subdirectory calls) would be more precise. As-is, it's a reasonable pragmatic choice.

💡 Suggestion: Duplicate compile flags on clrjit sources

JIT sources will now receive PGO flags from both the directory-scoped add_compile_options (new) and the per-target target_compile_options via add_pgo(clrjit) (existing, line 718 of jit/CMakeLists.txt). Compilers silently ignore duplicates, so this is harmless. For tidiness, add_pgo could skip adding compile flags when directory-scope flags are already active, but this is a follow-up cleanup, not a blocker.

✅ Correct: No -flto at directory scope

The new code intentionally omits -flto from add_compile_options. LTO flags must only appear on the final shared library link command (handled by add_pgo), not during compilation of static library objects. This is correct — adding -flto at directory scope would force LTO bitcode emission for all objects and could break static library archives.

✅ Correct: add_link_options(-fprofile-instr-generate) in instrumentation path

This ensures any shared library linked within the coreclr tree resolves the LLVM profiling runtime symbols introduced by its instrumented static library dependencies. For static/object library targets, link options are ignored by CMake, so no harm.

✅ Correct: Generator expression guards

$<$<COMPILE_LANGUAGE:C,CXX>:...> correctly excludes assembly (.S) files from receiving the flags, which would cause assembler errors.

✅ Correct: Profile path consistency

The hardcoded coreclr.profdata in the new directory-scope code matches what add_pgo already uses for non-Windows targets (line 33 of the original code).

✅ Correct: Guard conditions

The NOT WIN32, CMAKE_CXX_COMPILER_ID MATCHES "Clang", build-type, and sanitizer-exclusion guards all mirror the existing add_pgo logic correctly.

Existing Review Comments

The automated reviewer flagged three items: (1) missing Clang guard — this is actually present at line 81/92 (CMAKE_CXX_COMPILER_ID MATCHES "Clang"), so that comment is a false positive; (2) scope concern on NativeAOT — valid, noted above; (3) unquoted _PgoGlobalProfilePath — the path is quoted in the EXISTS check (line 91) and embedded in a generator expression string for compile options where CMake handles it correctly, so this is not a real issue for typical build paths.

---

Generated by Copilot code review

Generated by Code Review for issue #128412 · ● 1.6M · ◷

[EgorBo]

@hoyosjs @AndyAyersMS opinions? seems to be producing instrumented bits

NOTE: this only fixes libcoreclr, the System.* shared libs need additional changes, but I'd like to validate that this one works as expected first.

[AndyAyersMS]

Seems like a great idea. I wonder how hard it will be to observe the impact?

[EgorBo]

Seems like a great idea. I wonder how hard it will be to observe the impact?

I checked that it does produce a fully instrumented shared lib, so the next step would be to wait for an instrumented SDK to be produced & consumed by dotnet-optimization after this PR is merged. From my quick look there should be no actions needed in dotnet-optimization and it will eventually produce a profdata that includes coreclr (we will test it by inspecting pgo data coming with nuget after the roundtrip and the final optimized binary).