From 8df75969ae7023b6ee527f9f9e0c8aaca02eee86 Mon Sep 17 00:00:00 2001 From: Han Shen Date: Mon, 10 Jul 2023 15:40:01 -0700 Subject: [CodeGen] Fine tune MachineFunctionSplitPass (MFS) for FSAFDO. The original MFS work D85368 shows good performance improvement with Instrumented FDO. However, AutoFDO or Flow-Sensitive AutoFDO (FSAFDO) does not show performance gain. This is mainly caused by a less accurate profile compared to the iFDO profile. For the past few months, we have been working to improve FSAFDO quality, like in D145171. Taking advantage of this improvement, MFS now shows performance improvements over FSAFDO profiles. That being said, 2 minor changes need to be made, 1) An FS-AutoFDO profile generation pass needs to be added right before MFS pass and an FSAFDO profile load pass is needed when FS-AutoFDO is enabled and the MFS flag is present. 2) MFS only applies to hot functions, because we believe (and experiment also shows) FS-AutoFDO is more accurate about functions that have plenty of samples than those with no or very few samples. With this improvement, we see a 1.2% performance improvement in clang benchmark, 0.9% QPS improvement in our internal search benchmark, and 3%-5% improvement in internal storage benchmark. This is #1 of the two patches that enables the improvement. Reviewed By: wenlei, snehasish, xur Differential Revision: https://reviews.llvm.org/D152399 --- llvm/lib/CodeGen/MachineFunctionSplitter.cpp | 52 ++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 11 deletions(-) (limited to 'llvm/lib/CodeGen/MachineFunctionSplitter.cpp') diff --git a/llvm/lib/CodeGen/MachineFunctionSplitter.cpp b/llvm/lib/CodeGen/MachineFunctionSplitter.cpp index 1b939a518f16..3a3b9a6e5e69 100644 --- a/llvm/lib/CodeGen/MachineFunctionSplitter.cpp +++ b/llvm/lib/CodeGen/MachineFunctionSplitter.cpp @@ -24,6 +24,8 @@ //===----------------------------------------------------------------------===// #include "llvm/ADT/SmallVector.h" +#include "llvm/Analysis/BlockFrequencyInfo.h" +#include "llvm/Analysis/BranchProbabilityInfo.h" #include "llvm/Analysis/EHUtils.h" #include "llvm/Analysis/ProfileSummaryInfo.h" #include "llvm/CodeGen/BasicBlockSectionUtils.h" @@ -94,16 +96,26 @@ static void setDescendantEHBlocksCold(MachineFunction &MF) { } } +static void finishAdjustingBasicBlocksAndLandingPads(MachineFunction &MF) { + auto Comparator = [](const MachineBasicBlock &X, const MachineBasicBlock &Y) { + return X.getSectionID().Type < Y.getSectionID().Type; + }; + llvm::sortBasicBlocksAndUpdateBranches(MF, Comparator); + llvm::avoidZeroOffsetLandingPad(MF); +} + static bool isColdBlock(const MachineBasicBlock &MBB, const MachineBlockFrequencyInfo *MBFI, - ProfileSummaryInfo *PSI) { + ProfileSummaryInfo *PSI, bool HasAccurateProfile) { std::optional Count = MBFI->getBlockProfileCount(&MBB); + // If using accurate profile, no count means cold. + // If no accurate profile, no count means "do not judge + // coldness". if (!Count) - return true; + return HasAccurateProfile; - if (PercentileCutoff > 0) { + if (PercentileCutoff > 0) return PSI->isColdCountNthPercentile(PercentileCutoff, *Count); - } return (*Count < ColdCountThreshold); } @@ -140,9 +152,28 @@ bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) { MachineBlockFrequencyInfo *MBFI = nullptr; ProfileSummaryInfo *PSI = nullptr; + // Whether this pass is using FSAFDO profile (not accurate) or IRPGO + // (accurate). HasAccurateProfile is only used when UseProfileData is true, + // but giving it a default value to silent any possible warning. + bool HasAccurateProfile = false; if (UseProfileData) { MBFI = &getAnalysis(); PSI = &getAnalysis().getPSI(); + // "HasAccurateProfile" is false for FSAFDO, true when using IRPGO + // (traditional instrumented FDO) or CSPGO profiles. + HasAccurateProfile = + PSI->hasInstrumentationProfile() || PSI->hasCSInstrumentationProfile(); + // If HasAccurateProfile is false, we only trust hot functions, + // which have many samples, and consider them as split + // candidates. On the other hand, if HasAccurateProfile (likeIRPGO), we + // trust both cold and hot functions. + if (!HasAccurateProfile && !PSI->isFunctionHotInCallGraph(&MF, *MBFI)) { + // Split all EH code and it's descendant statically by default. + if (SplitAllEHCode) + setDescendantEHBlocksCold(MF); + finishAdjustingBasicBlocksAndLandingPads(MF); + return true; + } } SmallVector LandingPads; @@ -152,7 +183,8 @@ bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) { if (MBB.isEHPad()) LandingPads.push_back(&MBB); - else if (UseProfileData && isColdBlock(MBB, MBFI, PSI) && !SplitAllEHCode) + else if (UseProfileData && + isColdBlock(MBB, MBFI, PSI, HasAccurateProfile) && !SplitAllEHCode) MBB.setSectionID(MBBSectionID::ColdSectionID); } @@ -161,9 +193,10 @@ bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) { setDescendantEHBlocksCold(MF); // We only split out eh pads if all of them are cold. else { + // Here we have UseProfileData == true. bool HasHotLandingPads = false; for (const MachineBasicBlock *LP : LandingPads) { - if (!isColdBlock(*LP, MBFI, PSI)) + if (!isColdBlock(*LP, MBFI, PSI, HasAccurateProfile)) HasHotLandingPads = true; } if (!HasHotLandingPads) { @@ -171,11 +204,8 @@ bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) { LP->setSectionID(MBBSectionID::ColdSectionID); } } - auto Comparator = [](const MachineBasicBlock &X, const MachineBasicBlock &Y) { - return X.getSectionID().Type < Y.getSectionID().Type; - }; - llvm::sortBasicBlocksAndUpdateBranches(MF, Comparator); - llvm::avoidZeroOffsetLandingPad(MF); + + finishAdjustingBasicBlocksAndLandingPads(MF); return true; } -- cgit v1.2.3