This is an archive of the discontinued LLVM Phabricator instance.

[SampleFDO][NFC] Detach SampleProfileLoader from SampleCoverageTracker
ClosedPublic

Authored by xur on Feb 1 2021, 2:26 PM.

Details

Summary

his is split from patch: https://reviews.llvm.org/D95698

This patch detaches SampleProfileLoader from SampleCoverageTracker. We plan to move SampleProfileLoader to a template class. This would keep
SampleCoverageTracker as a class. Also make callsiteIsHot() as a file static function.

Diff Detail

Event Timeline

xur created this revision.Feb 1 2021, 2:26 PM
xur requested review of this revision.Feb 1 2021, 2:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2021, 2:26 PM

Why making callSiteIsHot a file static func?

xur added a comment.Feb 1 2021, 3:11 PM

It's only used in this file, by SampleProfileLoader and SampleProfileTracker class. I could make it a class static method (in SampleProfileLoader), But I don't see an advantage of that -- just with longer class qualifier.

wmi added a comment.Feb 1 2021, 4:01 PM

This patch detaches SampleProfileLoader from SampleCoverageTracker. This would keep SampleCoverageTracker as a class.

It means you don't want SampleCoverageTracker to be a template. Will the FS profile annotation in machine code level also need some coverage tracker?

Machine level profile loader will use the SampleCoverage Tracker. This
class will be kept in the templated header. Machine level profile loader
and IR level profile loader will be using the exactly the same class (not
templatelized).

wmi added a comment.Feb 1 2021, 4:43 PM
In D95823#2535255, @xur wrote:

Machine level profile loader will use the SampleCoverage Tracker. This
class will be kept in the templated header. Machine level profile loader
and IR level profile loader will be using the exactly the same class (not
templatelized).

I see. During profile annotation in machine IR level, for FunctionSamples only using base discriminator, will they be used for annotation again or their annotation will be skipped? I am asking because I am wondering what the coverage report will look like for SampleCoverageTracker used in machine IR.

wmi accepted this revision.Feb 2 2021, 4:42 PM

LGTM.

This revision is now accepted and ready to land.Feb 2 2021, 4:42 PM