Page MenuHomePhabricator

[CodeView] Add CodeView support for PGO debug information
AcceptedPublic

Authored by Holman on Tue, Apr 6, 3:16 PM.

Details

Summary

This change adds debug information about whether PGO is being used or not.

Microsoft performance tooling (e.g. xperf, WPA) uses this information to show whether functions are optimized with PGO or not, as well as whether PGO information is invalid.

This information is useful for validating whether training scenarios are providing good coverage of real world scenarios, showing if profile data is out of date, etc.

Diff Detail

Unit TestsFailed

TimeTest
1,680 msx64 windows > Clang.Modules::preprocess-module.cpp
Script: -- : 'RUN: at line 1'; rm -rf C:\ws\w32-1\llvm-project\premerge-checks\build\tools\clang\test\Modules\Output\preprocess-module.cpp.tmp

Event Timeline

Holman created this revision.Tue, Apr 6, 3:16 PM
Holman requested review of this revision.Tue, Apr 6, 3:16 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptTue, Apr 6, 3:16 PM

think this looks good overall; maybe it should also have an IR to codeview test?

rnk added a subscriber: rnk.Thu, Apr 8, 12:14 PM

IMO it's best to avoid adding fields to DICompileUnit if at all possible. It's the "god object" / "katamari damacy" of module debug info. Is there something about the IR module that indicates if PGO data is present or not? We could check that instead. I looked, but I wasn't able to find anything quickly.

Holman added a comment.Thu, Apr 8, 1:11 PM
In D99994#2677566, @rnk wrote:

IMO it's best to avoid adding fields to DICompileUnit if at all possible. It's the "god object" / "katamari damacy" of module debug info. Is there something about the IR module that indicates if PGO data is present or not? We could check that instead. I looked, but I wasn't able to find anything quickly.

Got it. It looks like MMI->getModule()->getProfileSummary() should have the info I need. I'll update to use that instead.

Holman updated this revision to Diff 336223.Thu, Apr 8, 2:07 PM

Get PGO info from Module instead of adding new field to debug info.

rnk added a comment.Thu, Apr 8, 2:12 PM

Seems reasonable, but we still need a small IR test.

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
807

nit, the more common style to name parameters looks like getProfileSummary(/*IsCS=*/false). Sort of more Python-y.

1434

I think the preferred spelling is GV.hasProfileData(), I think that's functionally equivalent.

Holman updated this revision to Diff 336274.Thu, Apr 8, 4:53 PM
Holman set the repository for this revision to rG LLVM Github Monorepo.

Add a test

Holman marked 2 inline comments as done.Thu, Apr 8, 4:54 PM
Holman updated this revision to Diff 336497.Fri, Apr 9, 9:42 AM

Fix clang-format issue.

rnk accepted this revision.Fri, Apr 9, 2:27 PM

lgtm, thanks!

This revision is now accepted and ready to land.Fri, Apr 9, 2:27 PM