[llvm] 546ec64 - Restore "[MemProf] Use new option/pass for profile feedback and matching"
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 11 13:16:37 PDT 2023
Author: Teresa Johnson
Date: 2023-07-11T13:16:20-07:00
New Revision: 546ec641b4b1bbbf9e66a53983b635fe85d365e6
URL: https://github.com/llvm/llvm-project/commit/546ec641b4b1bbbf9e66a53983b635fe85d365e6
DIFF: https://github.com/llvm/llvm-project/commit/546ec641b4b1bbbf9e66a53983b635fe85d365e6.diff
LOG: Restore "[MemProf] Use new option/pass for profile feedback and matching"
This restores commit b4a82b62258c5f650a1cccf5b179933e6bae4867, reverted
in 3ab7ef28eebf9019eb3d3c4efd7ebfd160106bb1 because it was thought to
cause a bot failure, which ended up being unrelated to this patch set.
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 llvm-commits
mailing list