This is an archive of the discontinued LLVM Phabricator instance.

Differential D111847

[CSSPGO] Turn off PseudoProbeUpdatePass for non-FDO builds.
ClosedPublic

Authored by hoy on Oct 14 2021, 3:33 PM.

Details

Reviewers
wenlei
wlei
Commits
rG42ad7e1bc908: [CSSPGO] Turn off PseudoProbeUpdatePass for non-FDO builds.
Summary

PseudoProbeUpdatePass is used to distribute sample counts among dulplicated probes. It doesn't make sense for it to run without a sample profile. The pass takes 1% of the build time.

Diff Detail

Event Timeline

hoy created this revision.Oct 14 2021, 3:33 PM
Herald added subscribers: modimo, wenlei, hiraditya. · View Herald TranscriptOct 14 2021, 3:33 PM
hoy requested review of this revision.Oct 14 2021, 3:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 14 2021, 3:33 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
hoy added reviewers: wenlei, wlei.Oct 14 2021, 3:34 PM
wenlei accepted this revision.Oct 14 2021, 3:36 PM

lgtm.

llvm/lib/Passes/PassBuilderPipelines.cpp
1261

linter might complain the length of the line.

This revision is now accepted and ready to land.Oct 14 2021, 3:36 PM
hoy added inline comments.Oct 14 2021, 4:01 PM
llvm/lib/Passes/PassBuilderPipelines.cpp
1261

Interestingly, linter is actually fine with this. The lint build job just passed. My local linting doesn't do anything about it. I guess the linting rule has changed?

Harbormaster completed remote builds in B128971: Diff 379867.Oct 14 2021, 4:18 PM
hoy updated this revision to Diff 379890.Oct 14 2021, 5:12 PM

Fixing linter issue.

wlei accepted this revision.Oct 14 2021, 5:16 PM

LGTM, thanks!

This revision was landed with ongoing or failed builds.Oct 14 2021, 5:29 PM
Closed by commit rG42ad7e1bc908: [CSSPGO] Turn off PseudoProbeUpdatePass for non-FDO builds. (authored by hoy). · Explain Why
This revision was automatically updated to reflect the committed changes.
hoy added a commit: rG42ad7e1bc908: [CSSPGO] Turn off PseudoProbeUpdatePass for non-FDO builds..
Harbormaster completed remote builds in B128988: Diff 379890.Oct 14 2021, 6:10 PM

Revision Contents

PathSize
llvm/
lib/
Passes/
4 lines

Diff 379867

llvm/lib/Passes/PassBuilderPipelines.cpp

Show First 20 Lines • Show All 1,196 Lines • ▼ Show 20 LinesPassBuilder::buildPerModuleDefaultPipeline(OptimizationLevel Level,
// Add the core simplification pipeline. // Add the core simplification pipeline.
MPM.addPass(buildModuleSimplificationPipeline( MPM.addPass(buildModuleSimplificationPipeline(
Level, LTOPreLink ? ThinOrFullLTOPhase::FullLTOPreLink Level, LTOPreLink ? ThinOrFullLTOPhase::FullLTOPreLink
: ThinOrFullLTOPhase::None)); : ThinOrFullLTOPhase::None));
// Now add the optimization pipeline. // Now add the optimization pipeline.
MPM.addPass(buildModuleOptimizationPipeline(Level, LTOPreLink)); MPM.addPass(buildModuleOptimizationPipeline(Level, LTOPreLink));
if (PGOOpt && PGOOpt->PseudoProbeForProfiling) if (PGOOpt && PGOOpt->PseudoProbeForProfiling && PGOOpt->Action == PGOOptions::SampleUse)
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  if (PGOOpt && PGOOpt->PseudoProbeForProfiling && PGOOpt->Action == PGOOptions::SampleUse)
+  if (PGOOpt && PGOOpt->PseudoProbeForProfiling &&
+      PGOOpt->Action == PGOOptions::SampleUse)
Lint: Pre-merge checks: clang-format: please reformat the code ``` - if (PGOOpt && PGOOpt->PseudoProbeForProfiling &&…
MPM.addPass(PseudoProbeUpdatePass()); MPM.addPass(PseudoProbeUpdatePass());
// Emit annotation remarks. // Emit annotation remarks.
addAnnotationRemarksPass(MPM); addAnnotationRemarksPass(MPM);
if (LTOPreLink) if (LTOPreLink)
addRequiredLTOPreLinkPasses(MPM); addRequiredLTOPreLinkPasses(MPM);
Show All 39 LinesPassBuilder::buildThinLTOPreLinkDefaultPipeline(OptimizationLevel Level) {
// Reduce the size of the IR as much as possible. // Reduce the size of the IR as much as possible.
MPM.addPass(GlobalOptPass()); MPM.addPass(GlobalOptPass());
// Module simplification splits coroutines, but does not fully clean up // Module simplification splits coroutines, but does not fully clean up
// coroutine intrinsics. To ensure ThinLTO optimization passes don't trip up // coroutine intrinsics. To ensure ThinLTO optimization passes don't trip up
// on these, we schedule the cleanup here. // on these, we schedule the cleanup here.
MPM.addPass(createModuleToFunctionPassAdaptor(CoroCleanupPass())); MPM.addPass(createModuleToFunctionPassAdaptor(CoroCleanupPass()));
if (PGOOpt && PGOOpt->PseudoProbeForProfiling) if (PGOOpt && PGOOpt->PseudoProbeForProfiling && PGOOpt->Action == PGOOptions::SampleUse)
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  if (PGOOpt && PGOOpt->PseudoProbeForProfiling && PGOOpt->Action == PGOOptions::SampleUse)
+  if (PGOOpt && PGOOpt->PseudoProbeForProfiling &&
+      PGOOpt->Action == PGOOptions::SampleUse)
Lint: Pre-merge checks: clang-format: please reformat the code ``` - if (PGOOpt && PGOOpt->PseudoProbeForProfiling &&…
wenleiUnsubmitted
Not Done
ReplyInline Actions

linter might complain the length of the line.

wenlei: linter might complain the length of the line.
hoyAuthorUnsubmitted
Done
ReplyInline Actions

Interestingly, linter is actually fine with this. The lint build job just passed. My local linting doesn't do anything about it. I guess the linting rule has changed?

hoy: Interestingly, linter is actually fine with this. The lint build job just passed. My local…
MPM.addPass(PseudoProbeUpdatePass()); MPM.addPass(PseudoProbeUpdatePass());
// Handle OptimizerLastEPCallbacks added by clang on PreLink. Actual // Handle OptimizerLastEPCallbacks added by clang on PreLink. Actual
// optimization is going to be done in PostLink stage, but clang can't // optimization is going to be done in PostLink stage, but clang can't
// add callbacks there in case of in-process ThinLTO called by linker. // add callbacks there in case of in-process ThinLTO called by linker.
for (auto &C : OptimizerLastEPCallbacks) for (auto &C : OptimizerLastEPCallbacks)
C(MPM, Level); C(MPM, Level);
▲ Show 20 Lines • Show All 466 Lines • Show Last 20 Lines