Migrate PGOMemOptSizeOpt to use new OptimizationRemarkEmitter Pass

Summary:
Fixes PR33790.

This patch still needs a yaml-style test, which I shall write tomorrow

Reviewers: anemet

Reviewed By: anemet

Subscribers: anemet, llvm-commits

Differential Revision: https://reviews.llvm.org/D35981

llvm-svn: 309497

4 files changed, 61 insertions, 14 deletions

diff --git a/llvm/include/llvm/IR/DiagnosticInfo.h b/llvm/include/llvm/IR/DiagnosticInfo.h
index c0bfe681e05..2a38071be5e 100644
--- a/llvm/include/llvm/IR/DiagnosticInfo.h
+++ b/llvm/include/llvm/IR/DiagnosticInfo.h
@@ -419,6 +419,7 @@ public:
     explicit Argument(StringRef Str = "") : Key("String"), Val(Str) {}
     Argument(StringRef Key, const Value *V);
     Argument(StringRef Key, const Type *T);
+    Argument(StringRef Key, StringRef S);
     Argument(StringRef Key, int N);
     Argument(StringRef Key, int64_t N);
     Argument(StringRef Key, unsigned N);
diff --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp
index 6feeb2911e3..eeb6ce3dd50 100644
--- a/llvm/lib/IR/DiagnosticInfo.cpp
+++ b/llvm/lib/IR/DiagnosticInfo.cpp
@@ -218,6 +218,9 @@ DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, const Type *T)
   OS << *T;
 }
 
+DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, StringRef S)
+    : Key(Key), Val(S.str()) {}
+
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, int N)
     : Key(Key), Val(itostr(N)) {}
 
diff --git a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
index 0bc9ddfbe4d..d91567bc4d9 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
@@ -21,16 +21,16 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/GlobalsModRef.h"
+#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/DerivedTypes.h"
-#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/InstVisitor.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
-#include "llvm/IR/InstVisitor.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/Type.h"
@@ -110,6 +110,7 @@ private:
   bool runOnFunction(Function &F) override;
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<BlockFrequencyInfoWrapperPass>();
+    AU.addRequired<OptimizationRemarkEmitterWrapperPass>();
     AU.addPreserved<GlobalsAAWrapperPass>();
   }
 };
@@ -131,8 +132,9 @@ FunctionPass *llvm::createPGOMemOPSizeOptLegacyPass() {
 namespace {
 class MemOPSizeOpt : public InstVisitor<MemOPSizeOpt> {
 public:
-  MemOPSizeOpt(Function &Func, BlockFrequencyInfo &BFI)
-      : Func(Func), BFI(BFI), Changed(false) {
+  MemOPSizeOpt(Function &Func, BlockFrequencyInfo &BFI,
+               OptimizationRemarkEmitter &ORE)
+      : Func(Func), BFI(BFI), ORE(ORE), Changed(false) {
     ValueDataArray =
         llvm::make_unique<InstrProfValueData[]>(MemOPMaxVersion + 2);
     // Get the MemOPSize range information from option MemOPSizeRange,
@@ -166,6 +168,7 @@ public:
 private:
   Function &Func;
   BlockFrequencyInfo &BFI;
+  OptimizationRemarkEmitter &ORE;
   bool Changed;
   std::vector<MemIntrinsic *> WorkList;
   // Start of the previse range.
@@ -376,23 +379,27 @@ bool MemOPSizeOpt::perform(MemIntrinsic *MI) {
   DEBUG(dbgs() << *DefaultBB << "\n");
   DEBUG(dbgs() << *MergeBB << "\n");
 
-  emitOptimizationRemark(Func.getContext(), "memop-opt", Func,
-                         MI->getDebugLoc(),
-                         Twine("optimize ") + getMIName(MI) + " with count " +
-                             Twine(SumForOpt) + " out of " + Twine(TotalCount) +
-                             " for " + Twine(Version) + " versions");
+  {
+    using namespace ore;
+    ORE.emit(OptimizationRemark(DEBUG_TYPE, "memopt-opt", MI)
+             << "optimized " << NV("Intrinsic", StringRef(getMIName(MI)))
+             << " with count " << NV("Count", SumForOpt) << " out of "
+             << NV("Total", TotalCount) << " for " << NV("Versions", Version)
+             << " versions");
+  }
 
   return true;
 }
 } // namespace
 
-static bool PGOMemOPSizeOptImpl(Function &F, BlockFrequencyInfo &BFI) {
+static bool PGOMemOPSizeOptImpl(Function &F, BlockFrequencyInfo &BFI,
+                                OptimizationRemarkEmitter &ORE) {
   if (DisableMemOPOPT)
     return false;
 
   if (F.hasFnAttribute(Attribute::OptimizeForSize))
     return false;
-  MemOPSizeOpt MemOPSizeOpt(F, BFI);
+  MemOPSizeOpt MemOPSizeOpt(F, BFI, ORE);
   MemOPSizeOpt.perform();
   return MemOPSizeOpt.isChanged();
 }
@@ -400,7 +407,8 @@ static bool PGOMemOPSizeOptImpl(Function &F, BlockFrequencyInfo &BFI) {
 bool PGOMemOPSizeOptLegacyPass::runOnFunction(Function &F) {
   BlockFrequencyInfo &BFI =
       getAnalysis<BlockFrequencyInfoWrapperPass>().getBFI();
-  return PGOMemOPSizeOptImpl(F, BFI);
+  auto &ORE = getAnalysis<OptimizationRemarkEmitterWrapperPass>().getORE();
+  return PGOMemOPSizeOptImpl(F, BFI, ORE);
 }
 
 namespace llvm {
@@ -409,7 +417,8 @@ char &PGOMemOPSizeOptID = PGOMemOPSizeOptLegacyPass::ID;
 PreservedAnalyses PGOMemOPSizeOpt::run(Function &F,
                                        FunctionAnalysisManager &FAM) {
   auto &BFI = FAM.getResult<BlockFrequencyAnalysis>(F);
-  bool Changed = PGOMemOPSizeOptImpl(F, BFI);
+  auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(F);
+  bool Changed = PGOMemOPSizeOptImpl(F, BFI, ORE);
   if (!Changed)
     return PreservedAnalyses::all();
   auto PA = PreservedAnalyses();
diff --git a/llvm/test/Transforms/PGOProfile/memop_size_opt.ll b/llvm/test/Transforms/PGOProfile/memop_size_opt.ll
index e11f235a48e..0d2edd40e9c 100644
--- a/llvm/test/Transforms/PGOProfile/memop_size_opt.ll
+++ b/llvm/test/Transforms/PGOProfile/memop_size_opt.ll
@@ -1,5 +1,6 @@
 ; RUN: opt < %s -passes=pgo-memop-opt -pgo-memop-count-threshold=90 -pgo-memop-percent-threshold=15 -S | FileCheck %s --check-prefix=MEMOP_OPT
-; RUN: opt < %s -pgo-memop-opt -pgo-memop-count-threshold=90 -pgo-memop-percent-threshold=15 -S | FileCheck %s --check-prefix=MEMOP_OPT
+; RUN: opt < %s -pgo-memop-opt -pgo-memop-count-threshold=90 -pgo-memop-percent-threshold=15 -pass-remarks-with-hotness -pass-remarks-output=%t.opt.yaml -S | FileCheck %s --check-prefix=MEMOP_OPT
+; RUN: FileCheck %s -input-file=%t.opt.yaml --check-prefix=YAML
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -114,3 +115,36 @@ declare void @llvm.lifetime.start(i64, i8* nocapture)
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32, i1)
 
 declare void @llvm.lifetime.end(i64, i8* nocapture)
+
+; YAML:      --- !Passed
+; YAML-NEXT: Pass:            pgo-memop-opt
+; YAML-NEXT: Name:            memopt-opt
+; YAML-NEXT: Function:        foo
+; YAML-NEXT: Hotness:         0
+; YAML-NEXT: Args:
+; YAML-NEXT:   - String:          'optimized '
+; YAML-NEXT:   - Intrinsic:       memcpy
+; YAML-NEXT:   - String:          ' with count '
+; YAML-NEXT:   - Count:           '99'
+; YAML-NEXT:   - String:          ' out of '
+; YAML-NEXT:   - Total:           '556'
+; YAML-NEXT:   - String:          ' for '
+; YAML-NEXT:   - Versions:        '1'
+; YAML-NEXT:   - String:          ' versions'
+; YAML-NEXT: ...
+; YAML-NEXT: --- !Passed
+; YAML-NEXT: Pass:            pgo-memop-opt
+; YAML-NEXT: Name:            memopt-opt
+; YAML-NEXT: Function:        foo
+; YAML-NEXT: Hotness:         0
+; YAML-NEXT: Args:
+; YAML-NEXT:   - String:          'optimized '
+; YAML-NEXT:   - Intrinsic:       memcpy
+; YAML-NEXT:   - String:          ' with count '
+; YAML-NEXT:   - Count:           '99'
+; YAML-NEXT:   - String:          ' out of '
+; YAML-NEXT:   - Total:           '556'
+; YAML-NEXT:   - String:          ' for '
+; YAML-NEXT:   - Versions:        '1'
+; YAML-NEXT:   - String:          ' versions'
+; YAML-NEXT: ...