[clang] 3ab7ef2 - Revert "[MemProf] Use new option/pass for profile feedback and matching"

JP Lehr via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 11 02:55:41 PDT 2023


Author: JP Lehr
Date: 2023-07-11T05:44:42-04:00
New Revision: 3ab7ef28eebf9019eb3d3c4efd7ebfd160106bb1

URL: https://github.com/llvm/llvm-project/commit/3ab7ef28eebf9019eb3d3c4efd7ebfd160106bb1
DIFF: https://github.com/llvm/llvm-project/commit/3ab7ef28eebf9019eb3d3c4efd7ebfd160106bb1.diff

LOG: Revert "[MemProf] Use new option/pass for profile feedback and matching"

This reverts commit b4a82b62258c5f650a1cccf5b179933e6bae4867.

Broke AMDGPU OpenMP Offload buildbot

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 fadfff48582eea..b0f22411e1ad20 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -282,9 +282,6 @@ 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 c5230d11baeddf..a34eab4064997a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1772,10 +1772,6 @@ 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 06af08023d1be9..458756509b3a3d 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -762,37 +762,31 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
     PGOOpt = PGOOptions(
         CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName()
                                                : CodeGenOpts.InstrProfileOutput,
-        "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr,
-        PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling);
+        "", "", 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, CodeGenOpts.MemoryProfileUsePath, VFS,
-        PGOOptions::IRUse, CSAction, CodeGenOpts.DebugInfoForProfiling);
+    PGOOpt =
+        PGOOptions(CodeGenOpts.ProfileInstrumentUsePath, "",
+                   CodeGenOpts.ProfileRemappingFile, VFS, PGOOptions::IRUse,
+                   CSAction, CodeGenOpts.DebugInfoForProfiling);
   } else if (!CodeGenOpts.SampleProfileFile.empty())
     // -fprofile-sample-use
     PGOOpt = PGOOptions(
         CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile,
-        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);
+        VFS, PGOOptions::SampleUse, PGOOptions::NoCSAction,
+        CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling);
   else if (CodeGenOpts.PseudoProbeForProfiling)
     // -fpseudo-probe-for-profiling
-    PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr,
-                        PGOOptions::NoAction, PGOOptions::NoCSAction,
+    PGOOpt = PGOOptions("", "", "", nullptr, PGOOptions::NoAction,
+                        PGOOptions::NoCSAction,
                         CodeGenOpts.DebugInfoForProfiling, true);
   else if (CodeGenOpts.DebugInfoForProfiling)
     // -fdebug-info-for-profiling
-    PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr,
-                        PGOOptions::NoAction, PGOOptions::NoCSAction, true);
+    PGOOpt = PGOOptions("", "", "", nullptr, PGOOptions::NoAction,
+                        PGOOptions::NoCSAction, true);
 
   // Check to see if we want to generate a CS profile.
   if (CodeGenOpts.hasProfileCSIRInstr()) {
@@ -814,8 +808,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
                      CodeGenOpts.InstrProfileOutput.empty()
                          ? getDefaultProfileGenName()
                          : CodeGenOpts.InstrProfileOutput,
-                     "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction,
-                     PGOOptions::CSIRInstr, CodeGenOpts.DebugInfoForProfiling);
+                     "", 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 5c7e248c286a1b..a9efab7ae07816 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4946,18 +4946,6 @@ 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 a8d13f3ec0210d..cb53eaac03e111 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 -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]
+// 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]
 
 char *foo() {
   return new char[10];

diff  --git a/clang/test/Driver/fmemprof.cpp b/clang/test/Driver/fmemprof.cpp
index 69ac153b042d90..8879580a50ba0b 100644
--- a/clang/test/Driver/fmemprof.cpp
+++ b/clang/test/Driver/fmemprof.cpp
@@ -8,12 +8,3 @@
 // 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 35670c457745a5..45a3b9a010f937 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 MemoryProfile,
+             std::string ProfileRemappingFile,
              IntrusiveRefCntPtr<vfs::FileSystem> FS,
              PGOAction Action = NoAction, CSPGOAction CSAction = NoCSAction,
              bool DebugInfoForProfiling = false,
@@ -40,7 +40,6 @@ 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 293133b29cd9f9..a05ed5f1b99710 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h
@@ -12,7 +12,6 @@
 #ifndef LLVM_TRANSFORMS_INSTRUMENTATION_MEMPROFILER_H
 #define LLVM_TRANSFORMS_INSTRUMENTATION_MEMPROFILER_H
 
-#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
@@ -21,10 +20,6 @@ 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.
 ///
@@ -48,16 +43,12 @@ class ModuleMemProfilerPass : public PassInfoMixin<ModuleMemProfilerPass> {
   static bool isRequired() { return true; }
 };
 
-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;
-};
+// 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);
 
 } // namespace llvm
 

diff  --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 29e28876760816..897380c81f7652 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -236,21 +236,20 @@ 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,
-                        /*MemoryProfile=*/"", FS, PGOOptions::SampleUse,
-                        PGOOptions::NoCSAction, true);
+    PGOOpt = PGOOptions(Conf.SampleProfile, "", Conf.ProfileRemapping, FS,
+                        PGOOptions::SampleUse, PGOOptions::NoCSAction, true);
   else if (Conf.RunCSIRInstr) {
-    PGOOpt = PGOOptions("", Conf.CSIRProfile, Conf.ProfileRemapping,
-                        /*MemoryProfile=*/"", FS, PGOOptions::IRUse,
-                        PGOOptions::CSIRInstr, Conf.AddFSDiscriminator);
+    PGOOpt = PGOOptions("", Conf.CSIRProfile, Conf.ProfileRemapping, FS,
+                        PGOOptions::IRUse, PGOOptions::CSIRInstr,
+                        Conf.AddFSDiscriminator);
   } else if (!Conf.CSIRProfile.empty()) {
-    PGOOpt = PGOOptions(Conf.CSIRProfile, "", Conf.ProfileRemapping,
-                        /*MemoryProfile=*/"", FS, PGOOptions::IRUse,
-                        PGOOptions::CSIRUse, Conf.AddFSDiscriminator);
+    PGOOpt = PGOOptions(Conf.CSIRProfile, "", Conf.ProfileRemapping, FS,
+                        PGOOptions::IRUse, PGOOptions::CSIRUse,
+                        Conf.AddFSDiscriminator);
     NoPGOWarnMismatch = !Conf.PGOWarnMismatch;
   } else if (Conf.AddFSDiscriminator) {
-    PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr,
-                        PGOOptions::NoAction, PGOOptions::NoCSAction, true);
+    PGOOpt = PGOOptions("", "", "", nullptr, PGOOptions::NoAction,
+                        PGOOptions::NoCSAction, true);
   }
   TM->setPGOOption(PGOOpt);
 

diff  --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index d0cbbcc0e310b9..aea38733872c54 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -1071,23 +1071,6 @@ 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 660cb2e974d781..37565408fbede3 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -1102,10 +1102,6 @@ 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 e10dc995c49305..5b0e3b8a496ccb 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -183,13 +183,6 @@ 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 04d50cc70d91c2..d11528ca6dbc26 100644
--- a/llvm/lib/Support/PGOOptions.cpp
+++ b/llvm/lib/Support/PGOOptions.cpp
@@ -13,13 +13,12 @@ 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), MemoryProfile(MemoryProfile),
-      Action(Action), CSAction(CSAction),
+      ProfileRemappingFile(ProfileRemappingFile), Action(Action),
+      CSAction(CSAction),
       DebugInfoForProfiling(DebugInfoForProfiling ||
                             (Action == SampleUse && !PseudoProbeForProfiling)),
       PseudoProbeForProfiling(PseudoProbeForProfiling), FS(std::move(FS)) {
@@ -37,18 +36,13 @@ PGOOptions::PGOOptions(std::string ProfileFile, std::string CSProfileGenFile,
   // a profile.
   assert(this->CSAction != CSIRUse || this->Action == IRUse);
 
-  // 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.
+  // If neither Action nor CSAction, DebugInfoForProfiling or
+  // PseudoProbeForProfiling needs to be true.
   assert(this->Action != NoAction || this->CSAction != NoCSAction ||
-         !this->MemoryProfile.empty() || this->DebugInfoForProfiling ||
-         this->PseudoProbeForProfiling);
+         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 ||
-                       !this->MemoryProfile.empty()));
+  assert(this->FS || !(this->Action == IRUse || this->CSAction == CSIRUse));
 }
 
 PGOOptions::PGOOptions(const PGOOptions &) = default;

diff  --git a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
index 789ed005d03ddf..416060a847d443 100644
--- a/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
@@ -38,7 +38,6 @@
 #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"
@@ -674,9 +673,9 @@ stackFrameIncludesInlinedCallStack(ArrayRef<Frame> ProfileCallStack,
   return InlCallStackIter == InlinedCallStack.end();
 }
 
-static void readMemprof(Module &M, Function &F,
-                        IndexedInstrProfReader *MemProfReader,
-                        const TargetLibraryInfo &TLI) {
+void llvm::readMemprof(Module &M, Function &F,
+                       IndexedInstrProfReader *MemProfReader,
+                       const TargetLibraryInfo &TLI) {
   auto &Ctx = M.getContext();
 
   auto FuncName = getPGOFuncName(F);
@@ -866,49 +865,3 @@ static void 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 82cabbd28a7aaa..1a67710f748e39 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -110,6 +110,7 @@
 #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"
@@ -1996,7 +1997,7 @@ static bool annotateAllFunctions(
     return false;
 
   // TODO: might need to change the warning once the clang option is finalized.
-  if (!PGOReader->isIRLevelProfile()) {
+  if (!PGOReader->isIRLevelProfile() && !PGOReader->hasMemoryProfile()) {
     Ctx.diagnose(DiagnosticInfoPGOProfile(
         ProfileFileName.data(), "Not an IR level instrumentation profile"));
     return false;
@@ -2041,6 +2042,14 @@ 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 409f9502add770..7296de87e5a634 100644
--- a/llvm/test/Transforms/PGOProfile/memprof.ll
+++ b/llvm/test/Transforms/PGOProfile/memprof.ll
@@ -23,36 +23,19 @@
 ; ALL-NOT: memprof record not found for function hash
 ; ALL-NOT: no profile data available for function
 
-;; 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
+;; 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
 ; There should not be any PGO metadata
 ; MEMPROFONLY-NOT: !prof
 
-;; 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
+;; 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
 ; There should not be any memprof related metadata
 ; PGOONLY-NOT: !memprof
 ; PGOONLY-NOT: !callsite
 
-;; 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
+;; 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
 
 ; ModuleID = 'memprof.cc'
 source_filename = "memprof.cc"

diff  --git a/llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll b/llvm/test/Transforms/PGOProfile/memprofmissingfunc.ll
index 9741f5e43dbaa6..068c8c91aa3f40 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='memprof-use<profile-filename=%t.memprofdata>' -pgo-warn-missing-function -S 2>&1 | FileCheck %s
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%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 6ae3f87099afd6..5465810bf6f840 100644
--- a/llvm/tools/opt/NewPMDriver.cpp
+++ b/llvm/tools/opt/NewPMDriver.cpp
@@ -176,9 +176,6 @@ 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,
@@ -339,21 +336,19 @@ bool llvm::runPassPipeline(
   std::optional<PGOOptions> P;
   switch (PGOKindFlag) {
   case InstrGen:
-    P = PGOOptions(ProfileFile, "", "", MemoryProfileFile, FS,
-                   PGOOptions::IRInstr);
+    P = PGOOptions(ProfileFile, "", "", FS, PGOOptions::IRInstr);
     break;
   case InstrUse:
-    P = PGOOptions(ProfileFile, "", ProfileRemappingFile, MemoryProfileFile, FS,
+    P = PGOOptions(ProfileFile, "", ProfileRemappingFile, FS,
                    PGOOptions::IRUse);
     break;
   case SampleUse:
-    P = PGOOptions(ProfileFile, "", ProfileRemappingFile, MemoryProfileFile, FS,
+    P = PGOOptions(ProfileFile, "", ProfileRemappingFile, FS,
                    PGOOptions::SampleUse);
     break;
   case NoPGO:
-    if (DebugInfoForProfiling || PseudoProbeForProfiling ||
-        !MemoryProfileFile.empty())
-      P = PGOOptions("", "", "", MemoryProfileFile, FS, PGOOptions::NoAction,
+    if (DebugInfoForProfiling || PseudoProbeForProfiling)
+      P = PGOOptions("", "", "", nullptr, PGOOptions::NoAction,
                      PGOOptions::NoCSAction, DebugInfoForProfiling,
                      PseudoProbeForProfiling);
     else
@@ -374,9 +369,8 @@ bool llvm::runPassPipeline(
         P->CSAction = PGOOptions::CSIRInstr;
         P->CSProfileGenFile = CSProfileGenFile;
       } else
-        P = PGOOptions("", CSProfileGenFile, ProfileRemappingFile,
-                       /*MemoryProfile=*/"", FS, PGOOptions::NoAction,
-                       PGOOptions::CSIRInstr);
+        P = PGOOptions("", CSProfileGenFile, ProfileRemappingFile, 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