[clang] b4a82b6 - [MemProf] Use new option/pass for profile feedback and matching
Teresa Johnson via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 10 16:43:07 PDT 2023
Author: Teresa Johnson
Date: 2023-07-10T16:42:56-07:00
New Revision: b4a82b62258c5f650a1cccf5b179933e6bae4867
URL: https://github.com/llvm/llvm-project/commit/b4a82b62258c5f650a1cccf5b179933e6bae4867
DIFF: https://github.com/llvm/llvm-project/commit/b4a82b62258c5f650a1cccf5b179933e6bae4867.diff
LOG: [MemProf] Use new option/pass for profile feedback and matching
Previously the MemProf profile was expected to be in the same profile
file as a normal PGO profile, passed via the usual -fprofile-use=
option, and was matched in the same pass. To simplify profile
preparation, since the raw MemProf profile requires the binary for
symbolization and may be simpler to index separately from the raw PGO
profile, and also to enable providing a MemProf profile for a SamplePGO
build, separate out the MemProf feedback option and matching pass.
This patch adds the -fmemory-profile-use=${file} option, and the
provided file is passed down to LLVM and ultimately used in a new
MemProfUsePass which performs the matching of just the memory profile
contents of that file.
Note that a single profile file containing both normal PGO and MemProf
profile data is still supported, and the relevant profile data is
matched by the appropriate matching pass(es) based on which option(s)
the profile is provided with (the same profile file can be supplied to
both feedback options).
Differential Revision: https://reviews.llvm.org/D154856
Added:
Modified:
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/memprof.cpp
clang/test/Driver/fmemprof.cpp
llvm/include/llvm/Support/PGOOptions.h
llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/Passes/PassBuilder.cpp
llvm/lib/Passes/PassBuilderPipelines.cpp
llvm/lib/Passes/PassRegistry.def
llvm/lib/Support/PGOOptions.cpp
llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
llvm/test/Transforms/PGOProfile/memprof.ll
llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll
llvm/tools/opt/NewPMDriver.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index b0f22411e1ad20..fadfff48582eea 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -282,6 +282,9 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// Name of the profile file to use as output for with -fmemory-profile.
std::string MemoryProfileOutput;
+ /// Name of the profile file to use as input for -fmemory-profile-use.
+ std::string MemoryProfileUsePath;
+
/// Name of the profile file to use as input for -fprofile-instr-use
std::string ProfileInstrumentUsePath;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index a34eab4064997a..c5230d11baeddf 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1772,6 +1772,10 @@ defm memory_profile : OptInCC1FFlag<"memory-profile", "Enable", "Disable", " hea
def fmemory_profile_EQ : Joined<["-"], "fmemory-profile=">,
Group<f_Group>, Flags<[CC1Option]>, MetaVarName<"<directory>">,
HelpText<"Enable heap memory profiling and dump results into <directory>">;
+def fmemory_profile_use_EQ : Joined<["-"], "fmemory-profile-use=">,
+ Group<f_Group>, Flags<[CC1Option, CoreOption]>, MetaVarName<"<pathname>">,
+ HelpText<"Use memory profile for profile-guided memory optimization">,
+ MarshallingInfoString<CodeGenOpts<"MemoryProfileUsePath">>;
// Begin sanitizer flags. These should all be core options exposed in all driver
// modes.
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 458756509b3a3d..06af08023d1be9 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -762,31 +762,37 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
PGOOpt = PGOOptions(
CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName()
: CodeGenOpts.InstrProfileOutput,
- "", "", nullptr, PGOOptions::IRInstr, PGOOptions::NoCSAction,
- CodeGenOpts.DebugInfoForProfiling);
+ "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr,
+ PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling);
else if (CodeGenOpts.hasProfileIRUse()) {
// -fprofile-use.
auto CSAction = CodeGenOpts.hasProfileCSIRUse() ? PGOOptions::CSIRUse
: PGOOptions::NoCSAction;
- PGOOpt =
- PGOOptions(CodeGenOpts.ProfileInstrumentUsePath, "",
- CodeGenOpts.ProfileRemappingFile, VFS, PGOOptions::IRUse,
- CSAction, CodeGenOpts.DebugInfoForProfiling);
+ PGOOpt = PGOOptions(
+ CodeGenOpts.ProfileInstrumentUsePath, "",
+ CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS,
+ PGOOptions::IRUse, CSAction, CodeGenOpts.DebugInfoForProfiling);
} else if (!CodeGenOpts.SampleProfileFile.empty())
// -fprofile-sample-use
PGOOpt = PGOOptions(
CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile,
- VFS, PGOOptions::SampleUse, PGOOptions::NoCSAction,
- CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling);
+ CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::SampleUse,
+ PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling,
+ CodeGenOpts.PseudoProbeForProfiling);
+ else if (!CodeGenOpts.MemoryProfileUsePath.empty())
+ // -fmemory-profile-use (without any of the above options)
+ PGOOpt = PGOOptions("", "", "", CodeGenOpts.MemoryProfileUsePath, VFS,
+ PGOOptions::NoAction, PGOOptions::NoCSAction,
+ CodeGenOpts.DebugInfoForProfiling);
else if (CodeGenOpts.PseudoProbeForProfiling)
// -fpseudo-probe-for-profiling
- PGOOpt = PGOOptions("", "", "", nullptr, PGOOptions::NoAction,
- PGOOptions::NoCSAction,
+ PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr,
+ PGOOptions::NoAction, PGOOptions::NoCSAction,
CodeGenOpts.DebugInfoForProfiling, true);
else if (CodeGenOpts.DebugInfoForProfiling)
// -fdebug-info-for-profiling
- PGOOpt = PGOOptions("", "", "", nullptr, PGOOptions::NoAction,
- PGOOptions::NoCSAction, true);
+ PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr,
+ PGOOptions::NoAction, PGOOptions::NoCSAction, true);
// Check to see if we want to generate a CS profile.
if (CodeGenOpts.hasProfileCSIRInstr()) {
@@ -808,8 +814,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
CodeGenOpts.InstrProfileOutput.empty()
? getDefaultProfileGenName()
: CodeGenOpts.InstrProfileOutput,
- "", nullptr, PGOOptions::NoAction, PGOOptions::CSIRInstr,
- CodeGenOpts.DebugInfoForProfiling);
+ "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction,
+ PGOOptions::CSIRInstr, CodeGenOpts.DebugInfoForProfiling);
}
if (TM)
TM->setPGOOption(PGOOpt);
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index a9efab7ae07816..5c7e248c286a1b 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4946,6 +4946,18 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
!MemProfArg->getOption().matches(options::OPT_fno_memory_profile))
MemProfArg->render(Args, CmdArgs);
+ if (auto *MemProfUseArg =
+ Args.getLastArg(options::OPT_fmemory_profile_use_EQ)) {
+ if (MemProfArg)
+ D.Diag(diag::err_drv_argument_not_allowed_with)
+ << MemProfUseArg->getAsString(Args) << MemProfArg->getAsString(Args);
+ if (auto *PGOInstrArg = Args.getLastArg(options::OPT_fprofile_generate,
+ options::OPT_fprofile_generate_EQ))
+ D.Diag(diag::err_drv_argument_not_allowed_with)
+ << MemProfUseArg->getAsString(Args) << PGOInstrArg->getAsString(Args);
+ MemProfUseArg->render(Args, CmdArgs);
+ }
+
// Embed-bitcode option.
// Only white-listed flags below are allowed to be embedded.
if (C.getDriver().embedBitcodeInObject() && !IsUsingLTO &&
diff --git a/clang/test/CodeGen/memprof.cpp b/clang/test/CodeGen/memprof.cpp
index cb53eaac03e111..a8d13f3ec0210d 100644
--- a/clang/test/CodeGen/memprof.cpp
+++ b/clang/test/CodeGen/memprof.cpp
@@ -16,8 +16,8 @@
// Profile use:
// Ensure Pass PGOInstrumentationUse is invoked with the memprof-only profile.
-// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t.memprofdata %s -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=USE
-// USE: Running pass: PGOInstrumentationUse on [module]
+// RUN: %clang_cc1 -O2 -fmemory-profile-use=%t.memprofdata %s -fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=USE
+// USE: Running pass: MemProfUsePass on [module]
char *foo() {
return new char[10];
diff --git a/clang/test/Driver/fmemprof.cpp b/clang/test/Driver/fmemprof.cpp
index 8879580a50ba0b..69ac153b042d90 100644
--- a/clang/test/Driver/fmemprof.cpp
+++ b/clang/test/Driver/fmemprof.cpp
@@ -8,3 +8,12 @@
// DIR: ld{{.*}}libclang_rt.memprof{{.*}}libclang_rt.memprof_cxx
// OFF-NOT: "-fmemory-profile"
// OFF-NOT: libclang_rt.memprof
+
+// RUN: %clangxx -target x86_64-linux-gnu -fmemory-profile-use=foo %s -### 2>&1 | FileCheck %s --check-prefix=USE
+// USE: "-cc1" {{.*}} "-fmemory-profile-use=foo"
+
+// RUN: %clangxx -target x86_64-linux-gnu -fmemory-profile -fmemory-profile-use=foo %s -### 2>&1 | FileCheck %s --check-prefix=CONFLICTWITHMEMPROFINSTR
+// CONFLICTWITHMEMPROFINSTR: error: invalid argument '-fmemory-profile-use=foo' not allowed with '-fmemory-profile'
+
+// RUN: %clangxx -target x86_64-linux-gnu -fprofile-generate -fmemory-profile-use=foo %s -### 2>&1 | FileCheck %s --check-prefix=CONFLICTWITHPGOINSTR
+// CONFLICTWITHPGOINSTR: error: invalid argument '-fmemory-profile-use=foo' not allowed with '-fprofile-generate'
diff --git a/llvm/include/llvm/Support/PGOOptions.h b/llvm/include/llvm/Support/PGOOptions.h
index 45a3b9a010f937..35670c457745a5 100644
--- a/llvm/include/llvm/Support/PGOOptions.h
+++ b/llvm/include/llvm/Support/PGOOptions.h
@@ -28,7 +28,7 @@ struct PGOOptions {
enum PGOAction { NoAction, IRInstr, IRUse, SampleUse };
enum CSPGOAction { NoCSAction, CSIRInstr, CSIRUse };
PGOOptions(std::string ProfileFile, std::string CSProfileGenFile,
- std::string ProfileRemappingFile,
+ std::string ProfileRemappingFile, std::string MemoryProfile,
IntrusiveRefCntPtr<vfs::FileSystem> FS,
PGOAction Action = NoAction, CSPGOAction CSAction = NoCSAction,
bool DebugInfoForProfiling = false,
@@ -40,6 +40,7 @@ struct PGOOptions {
std::string ProfileFile;
std::string CSProfileGenFile;
std::string ProfileRemappingFile;
+ std::string MemoryProfile;
PGOAction Action;
CSPGOAction CSAction;
bool DebugInfoForProfiling;
diff --git a/llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h b/llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h
index a05ed5f1b99710..293133b29cd9f9 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h
@@ -12,6 +12,7 @@
#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_MEMPROFILER_H
#define LLVM_TRANSFORMS_INSTRUMENTATION_MEMPROFILER_H
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/IR/PassManager.h"
namespace llvm {
@@ -20,6 +21,10 @@ class FunctionPass;
class Module;
class ModulePass;
+namespace vfs {
+class FileSystem;
+} // namespace vfs
+
/// Public interface to the memory profiler pass for instrumenting code to
/// profile memory accesses.
///
@@ -43,12 +48,16 @@ class ModuleMemProfilerPass : public PassInfoMixin<ModuleMemProfilerPass> {
static bool isRequired() { return true; }
};
-// TODO: Remove this declaration and make readMemprof static once the matching
-// is moved into its own pass.
-class IndexedInstrProfReader;
-class TargetLibraryInfo;
-void readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
- const TargetLibraryInfo &TLI);
+class MemProfUsePass : public PassInfoMixin<MemProfUsePass> {
+public:
+ explicit MemProfUsePass(std::string MemoryProfileFile,
+ IntrusiveRefCntPtr<vfs::FileSystem> FS = nullptr);
+ PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
+
+private:
+ std::string MemoryProfileFileName;
+ IntrusiveRefCntPtr<vfs::FileSystem> FS;
+};
} // namespace llvm
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 897380c81f7652..29e28876760816 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -236,20 +236,21 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
auto FS = vfs::getRealFileSystem();
std::optional<PGOOptions> PGOOpt;
if (!Conf.SampleProfile.empty())
- PGOOpt = PGOOptions(Conf.SampleProfile, "", Conf.ProfileRemapping, FS,
- PGOOptions::SampleUse, PGOOptions::NoCSAction, true);
+ PGOOpt = PGOOptions(Conf.SampleProfile, "", Conf.ProfileRemapping,
+ /*MemoryProfile=*/"", FS, PGOOptions::SampleUse,
+ PGOOptions::NoCSAction, true);
else if (Conf.RunCSIRInstr) {
- PGOOpt = PGOOptions("", Conf.CSIRProfile, Conf.ProfileRemapping, FS,
- PGOOptions::IRUse, PGOOptions::CSIRInstr,
- Conf.AddFSDiscriminator);
+ PGOOpt = PGOOptions("", Conf.CSIRProfile, Conf.ProfileRemapping,
+ /*MemoryProfile=*/"", FS, PGOOptions::IRUse,
+ PGOOptions::CSIRInstr, Conf.AddFSDiscriminator);
} else if (!Conf.CSIRProfile.empty()) {
- PGOOpt = PGOOptions(Conf.CSIRProfile, "", Conf.ProfileRemapping, FS,
- PGOOptions::IRUse, PGOOptions::CSIRUse,
- Conf.AddFSDiscriminator);
+ PGOOpt = PGOOptions(Conf.CSIRProfile, "", Conf.ProfileRemapping,
+ /*MemoryProfile=*/"", FS, PGOOptions::IRUse,
+ PGOOptions::CSIRUse, Conf.AddFSDiscriminator);
NoPGOWarnMismatch = !Conf.PGOWarnMismatch;
} else if (Conf.AddFSDiscriminator) {
- PGOOpt = PGOOptions("", "", "", nullptr, PGOOptions::NoAction,
- PGOOptions::NoCSAction, true);
+ PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr,
+ PGOOptions::NoAction, PGOOptions::NoCSAction, true);
}
TM->setPGOOption(PGOOpt);
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index aea38733872c54..d0cbbcc0e310b9 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -1071,6 +1071,23 @@ Expected<bool> parseMemorySSAPrinterPassOptions(StringRef Params) {
"MemorySSAPrinterPass");
}
+Expected<std::string> parseMemProfUsePassOptions(StringRef Params) {
+ std::string Result;
+ while (!Params.empty()) {
+ StringRef ParamName;
+ std::tie(ParamName, Params) = Params.split(';');
+
+ if (ParamName.consume_front("profile-filename=")) {
+ Result = ParamName.str();
+ } else {
+ return make_error<StringError>(
+ formatv("invalid MemProfUse pass parameter '{0}' ", ParamName).str(),
+ inconvertibleErrorCode());
+ }
+ }
+ return Result;
+}
+
} // namespace
/// Tests whether a pass name starts with a valid prefix for a default pipeline
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 37565408fbede3..660cb2e974d781 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -1102,6 +1102,10 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
PGOOpt->CSAction == PGOOptions::CSIRInstr)
MPM.addPass(PGOInstrumentationGenCreateVar(PGOOpt->CSProfileGenFile));
+ if (PGOOpt && Phase != ThinOrFullLTOPhase::ThinLTOPostLink &&
+ !PGOOpt->MemoryProfile.empty())
+ MPM.addPass(MemProfUsePass(PGOOpt->MemoryProfile, PGOOpt->FS));
+
// Synthesize function entry counts for non-PGO compilation.
if (EnableSyntheticCounts && !PGOOpt)
MPM.addPass(SyntheticCountsPropagation());
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 5b0e3b8a496ccb..e10dc995c49305 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -183,6 +183,13 @@ MODULE_PASS_WITH_PARAMS("embed-bitcode",
},
parseEmbedBitcodePassOptions,
"thinlto;emit-summary")
+MODULE_PASS_WITH_PARAMS("memprof-use",
+ "MemProfUsePass",
+ [](std::string Opts) {
+ return MemProfUsePass(Opts);
+ },
+ parseMemProfUsePassOptions,
+ "profile-filename=S")
#undef MODULE_PASS_WITH_PARAMS
#ifndef CGSCC_ANALYSIS
diff --git a/llvm/lib/Support/PGOOptions.cpp b/llvm/lib/Support/PGOOptions.cpp
index d11528ca6dbc26..04d50cc70d91c2 100644
--- a/llvm/lib/Support/PGOOptions.cpp
+++ b/llvm/lib/Support/PGOOptions.cpp
@@ -13,12 +13,13 @@ using namespace llvm;
PGOOptions::PGOOptions(std::string ProfileFile, std::string CSProfileGenFile,
std::string ProfileRemappingFile,
+ std::string MemoryProfile,
IntrusiveRefCntPtr<vfs::FileSystem> FS, PGOAction Action,
CSPGOAction CSAction, bool DebugInfoForProfiling,
bool PseudoProbeForProfiling)
: ProfileFile(ProfileFile), CSProfileGenFile(CSProfileGenFile),
- ProfileRemappingFile(ProfileRemappingFile), Action(Action),
- CSAction(CSAction),
+ ProfileRemappingFile(ProfileRemappingFile), MemoryProfile(MemoryProfile),
+ Action(Action), CSAction(CSAction),
DebugInfoForProfiling(DebugInfoForProfiling ||
(Action == SampleUse && !PseudoProbeForProfiling)),
PseudoProbeForProfiling(PseudoProbeForProfiling), FS(std::move(FS)) {
@@ -36,13 +37,18 @@ PGOOptions::PGOOptions(std::string ProfileFile, std::string CSProfileGenFile,
// a profile.
assert(this->CSAction != CSIRUse || this->Action == IRUse);
- // If neither Action nor CSAction, DebugInfoForProfiling or
- // PseudoProbeForProfiling needs to be true.
+ // Cannot optimize with MemProf profile during IR instrumentation.
+ assert(this->MemoryProfile.empty() || this->Action != PGOOptions::IRInstr);
+
+ // If neither Action nor CSAction nor MemoryProfile are set,
+ // DebugInfoForProfiling or PseudoProbeForProfiling needs to be true.
assert(this->Action != NoAction || this->CSAction != NoCSAction ||
- this->DebugInfoForProfiling || this->PseudoProbeForProfiling);
+ !this->MemoryProfile.empty() || this->DebugInfoForProfiling ||
+ this->PseudoProbeForProfiling);
// If we need to use the profile, the VFS cannot be nullptr.
- assert(this->FS || !(this->Action == IRUse || this->CSAction == CSIRUse));
+ assert(this->FS || !(this->Action == IRUse || this->CSAction == CSIRUse ||
+ !this->MemoryProfile.empty()));
}
PGOOptions::PGOOptions(const PGOOptions &) = default;
diff --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index 416060a847d443..789ed005d03ddf 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -38,6 +38,7 @@
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/HashBuilder.h"
+#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/TargetParser/Triple.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
@@ -673,9 +674,9 @@ stackFrameIncludesInlinedCallStack(ArrayRef<Frame> ProfileCallStack,
return InlCallStackIter == InlinedCallStack.end();
}
-void llvm::readMemprof(Module &M, Function &F,
- IndexedInstrProfReader *MemProfReader,
- const TargetLibraryInfo &TLI) {
+static void readMemprof(Module &M, Function &F,
+ IndexedInstrProfReader *MemProfReader,
+ const TargetLibraryInfo &TLI) {
auto &Ctx = M.getContext();
auto FuncName = getPGOFuncName(F);
@@ -865,3 +866,49 @@ void llvm::readMemprof(Module &M, Function &F,
}
}
}
+
+MemProfUsePass::MemProfUsePass(std::string MemoryProfileFile,
+ IntrusiveRefCntPtr<vfs::FileSystem> FS)
+ : MemoryProfileFileName(MemoryProfileFile), FS(FS) {
+ if (!FS)
+ this->FS = vfs::getRealFileSystem();
+}
+
+PreservedAnalyses MemProfUsePass::run(Module &M, ModuleAnalysisManager &AM) {
+ LLVM_DEBUG(dbgs() << "Read in memory profile:");
+ auto &Ctx = M.getContext();
+ auto ReaderOrErr = IndexedInstrProfReader::create(MemoryProfileFileName, *FS);
+ if (Error E = ReaderOrErr.takeError()) {
+ handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
+ Ctx.diagnose(
+ DiagnosticInfoPGOProfile(MemoryProfileFileName.data(), EI.message()));
+ });
+ return PreservedAnalyses::all();
+ }
+
+ std::unique_ptr<IndexedInstrProfReader> MemProfReader =
+ std::move(ReaderOrErr.get());
+ if (!MemProfReader) {
+ Ctx.diagnose(DiagnosticInfoPGOProfile(
+ MemoryProfileFileName.data(), StringRef("Cannot get MemProfReader")));
+ return PreservedAnalyses::all();
+ }
+
+ if (!MemProfReader->hasMemoryProfile()) {
+ Ctx.diagnose(DiagnosticInfoPGOProfile(MemoryProfileFileName.data(),
+ "Not a memory profile"));
+ return PreservedAnalyses::all();
+ }
+
+ auto &FAM = AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
+
+ for (auto &F : M) {
+ if (F.isDeclaration())
+ continue;
+
+ const TargetLibraryInfo &TLI = FAM.getResult<TargetLibraryAnalysis>(F);
+ readMemprof(M, F, MemProfReader.get(), TLI);
+ }
+
+ return PreservedAnalyses::none();
+}
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 1a67710f748e39..82cabbd28a7aaa 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -110,7 +110,6 @@
#include "llvm/Transforms/Instrumentation.h"
#include "llvm/Transforms/Instrumentation/BlockCoverageInference.h"
#include "llvm/Transforms/Instrumentation/CFGMST.h"
-#include "llvm/Transforms/Instrumentation/MemProfiler.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/MisExpect.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
@@ -1997,7 +1996,7 @@ static bool annotateAllFunctions(
return false;
// TODO: might need to change the warning once the clang option is finalized.
- if (!PGOReader->isIRLevelProfile() && !PGOReader->hasMemoryProfile()) {
+ if (!PGOReader->isIRLevelProfile()) {
Ctx.diagnose(DiagnosticInfoPGOProfile(
ProfileFileName.data(), "Not an IR level instrumentation profile"));
return false;
@@ -2042,14 +2041,6 @@ static bool annotateAllFunctions(
}
PGOUseFunc Func(F, &M, TLI, ComdatMembers, BPI, BFI, PSI, IsCS,
InstrumentFuncEntry, HasSingleByteCoverage);
- // Read and match memprof first since we do this via debug info and can
- // match even if there is an IR mismatch detected for regular PGO below.
- if (PGOReader->hasMemoryProfile())
- readMemprof(M, F, PGOReader.get(), TLI);
-
- if (!PGOReader->isIRLevelProfile())
- continue;
-
if (HasSingleByteCoverage) {
Func.populateCoverage(PGOReader.get());
continue;
diff --git a/llvm/test/Transforms/PGOProfile/memprof.ll b/llvm/test/Transforms/PGOProfile/memprof.ll
index 7296de87e5a634..409f9502add770 100644
--- a/llvm/test/Transforms/PGOProfile/memprof.ll
+++ b/llvm/test/Transforms/PGOProfile/memprof.ll
@@ -23,19 +23,36 @@
; ALL-NOT: memprof record not found for function hash
; ALL-NOT: no profile data available for function
-;; Feed back memprof-only profile
-; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.memprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,MEMPROFONLY
+;; Using a memprof-only profile for memprof-use should only give memprof metadata
+; RUN: opt < %s -passes='memprof-use<profile-filename=%t.memprofdata>' -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,MEMPROFONLY
; There should not be any PGO metadata
; MEMPROFONLY-NOT: !prof
-;; Feed back pgo-only profile
-; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.pgoprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=PGO,ALL,PGOONLY
+;; Test the same thing but by passing the memory profile through to a default
+;; pipeline via -memory-profile-file=, which should cause the necessary field
+;; of the PGOOptions structure to be populated with the profile filename.
+; RUN: opt < %s -passes='default<O2>' -memory-profile-file=%t.memprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,MEMPROFONLY
+
+;; Using a pgo+memprof profile for memprof-use should only give memprof metadata
+; RUN: opt < %s -passes='memprof-use<profile-filename=%t.pgomemprofdata>' -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,MEMPROFONLY
+
+;; Using a pgo-only profile for memprof-use should give an error
+; RUN: not opt < %s -passes='memprof-use<profile-filename=%t.pgoprofdata>' -S 2>&1 | FileCheck %s --check-prefixes=MEMPROFWITHPGOONLY
+; MEMPROFWITHPGOONLY: Not a memory profile
+
+;; Using a memprof-only profile for pgo-instr-use should give an error
+; RUN: not opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.memprofdata -S 2>&1 | FileCheck %s --check-prefixes=PGOWITHMEMPROFONLY
+; PGOWITHMEMPROFONLY: Not an IR level instrumentation profile
+
+;; Using a pgo+memprof profile for pgo-instr-use should only give pgo metadata
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.pgomemprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=PGO,ALL,PGOONLY
; There should not be any memprof related metadata
; PGOONLY-NOT: !memprof
; PGOONLY-NOT: !callsite
-;; Feed back pgo+memprof-only profile
-; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.pgomemprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,PGO,ALL
+;; Using a pgo+memprof profile for both memprof-use and pgo-instr-use should
+;; give both memprof and pgo metadata.
+; RUN: opt < %s -passes='pgo-instr-use,memprof-use<profile-filename=%t.pgomemprofdata>' -pgo-test-profile-file=%t.pgomemprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,PGO
; ModuleID = 'memprof.cc'
source_filename = "memprof.cc"
diff --git a/llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll b/llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll
index 068c8c91aa3f40..9741f5e43dbaa6 100644
--- a/llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll
+++ b/llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll
@@ -11,7 +11,7 @@
; RUN: llvm-profdata merge %S/Inputs/memprof.memprofraw --profiled-binary %S/Inputs/memprof.exe -o %t.memprofdata
-; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.memprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s
+; RUN: opt < %s -passes='memprof-use<profile-filename=%t.memprofdata>' -pgo-warn-missing-function -S 2>&1 | FileCheck %s
; CHECK: memprof record not found for function hash {{.*}} _Z16funcnotinprofilev
diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp
index 5465810bf6f840..6ae3f87099afd6 100644
--- a/llvm/tools/opt/NewPMDriver.cpp
+++ b/llvm/tools/opt/NewPMDriver.cpp
@@ -176,6 +176,9 @@ static cl::opt<PGOKind>
"Use sampled profile to guide PGO.")));
static cl::opt<std::string> ProfileFile("profile-file",
cl::desc("Path to the profile."), cl::Hidden);
+static cl::opt<std::string>
+ MemoryProfileFile("memory-profile-file",
+ cl::desc("Path to the memory profile."), cl::Hidden);
static cl::opt<CSPGOKind> CSPGOKindFlag(
"cspgo-kind", cl::init(NoCSPGO), cl::Hidden,
@@ -336,19 +339,21 @@ bool llvm::runPassPipeline(
std::optional<PGOOptions> P;
switch (PGOKindFlag) {
case InstrGen:
- P = PGOOptions(ProfileFile, "", "", FS, PGOOptions::IRInstr);
+ P = PGOOptions(ProfileFile, "", "", MemoryProfileFile, FS,
+ PGOOptions::IRInstr);
break;
case InstrUse:
- P = PGOOptions(ProfileFile, "", ProfileRemappingFile, FS,
+ P = PGOOptions(ProfileFile, "", ProfileRemappingFile, MemoryProfileFile, FS,
PGOOptions::IRUse);
break;
case SampleUse:
- P = PGOOptions(ProfileFile, "", ProfileRemappingFile, FS,
+ P = PGOOptions(ProfileFile, "", ProfileRemappingFile, MemoryProfileFile, FS,
PGOOptions::SampleUse);
break;
case NoPGO:
- if (DebugInfoForProfiling || PseudoProbeForProfiling)
- P = PGOOptions("", "", "", nullptr, PGOOptions::NoAction,
+ if (DebugInfoForProfiling || PseudoProbeForProfiling ||
+ !MemoryProfileFile.empty())
+ P = PGOOptions("", "", "", MemoryProfileFile, FS, PGOOptions::NoAction,
PGOOptions::NoCSAction, DebugInfoForProfiling,
PseudoProbeForProfiling);
else
@@ -369,8 +374,9 @@ bool llvm::runPassPipeline(
P->CSAction = PGOOptions::CSIRInstr;
P->CSProfileGenFile = CSProfileGenFile;
} else
- P = PGOOptions("", CSProfileGenFile, ProfileRemappingFile, FS,
- PGOOptions::NoAction, PGOOptions::CSIRInstr);
+ P = PGOOptions("", CSProfileGenFile, ProfileRemappingFile,
+ /*MemoryProfile=*/"", FS, PGOOptions::NoAction,
+ PGOOptions::CSIRInstr);
} else /* CSPGOKindFlag == CSInstrUse */ {
if (!P) {
errs() << "CSInstrUse needs to be together with InstrUse";
More information about the cfe-commits
mailing list