Migrate PGOMemOptSizeOpt to use new OptimizationRemarkEmitter Pass

[lenary]

Summary

Fixes PR33790.

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

[lenary]

Two quick notes for feedback, I shall get started on working on a test.

lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp

30

This change was added by git clang-format, probably incorrectly.

384

Not sure this is the right pair of names for this remark, maybe I should use getMIName(MI) (which returns "memset"/"memcpy"/"memmove"/"unknown") for the second string as well?

[anemet]

LGTM, I suppose there is already a -pass-remark test for this. Of course it would be great to also have a YAML test.

lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp

30

It probably orders uppercase before lowercase. This is fine. I usually land changes like this in a commit separate from the functional change but it's fine either way in this case.

384

No, I think it's fine the way it is. I think that it's better to have static strings for the remark name/identifier.

One nice improvement would be to pass the intrinsic name as an attribute so that we'd get "Intrinsic: memset" in YAML with NV("Intrinsic", getMIName(MI)).

I don't think we have an override for the Argument ctor that would take a StringRef. You may need to add that. This is also fine as a follow-on.

[lenary]

• Address Review Feedback, Add Test

lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringRef.h"
 #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"
 #include "llvm/Pass.h"
 #include "llvm/PassRegistry.h"
 #include "llvm/PassSupport.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Casting.h"
public:
   }
 
   StringRef getPassName() const override { return "PGOMemOPSize"; }
 
 private:
   bool runOnFunction(Function &F) override;
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<BlockFrequencyInfoWrapperPass>();
+    AU.addRequired<OptimizationRemarkEmitterWrapperPass>();
     AU.addPreserved<GlobalsAAWrapperPass>();
   }
 };
 } // end anonymous namespace
 
 char PGOMemOPSizeOptLegacyPass::ID = 0;
 INITIALIZE_PASS_BEGIN(PGOMemOPSizeOptLegacyPass, "pgo-memop-opt",
                       "Optimize memory intrinsic using its size value profile",
                       false, false)
 INITIALIZE_PASS_DEPENDENCY(BlockFrequencyInfoWrapperPass)
 INITIALIZE_PASS_END(PGOMemOPSizeOptLegacyPass, "pgo-memop-opt",
                     "Optimize memory intrinsic using its size value profile",
                     false, false)
 
 FunctionPass *llvm::createPGOMemOPSizeOptLegacyPass() {
   return new PGOMemOPSizeOptLegacyPass();
 }
 
 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,
     getMemOPSizeRangeFromOption(MemOPSizeRange, PreciseRangeStart,
                                 PreciseRangeLast);
   }
   bool isChanged() const { return Changed; }
   void perform() {
void visitMemIntrinsic(MemIntrinsic &MI) {
     if (dyn_cast<ConstantInt>(Length))
       return;
     WorkList.push_back(&MI);
   }
 
 private:
   Function &Func;
   BlockFrequencyInfo &BFI;
+  OptimizationRemarkEmitter &ORE;
   bool Changed;
   std::vector<MemIntrinsic *> WorkList;
   // Start of the previse range.
   int64_t PreciseRangeStart;
   // Last value of the previse range.
   int64_t PreciseRangeLast;
   // The space to read the profile annotation.
   std::unique_ptr<InstrProfValueData[]> ValueDataArray;
for (uint64_t SizeId : SizeIds) {
     DEBUG(dbgs() << *CaseBB << "\n");
   }
   setProfMetadata(Func.getParent(), SI, CaseCounts, MaxCount);
 
   DEBUG(dbgs() << *BB << "\n");
   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 " << 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();
 }
 
 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 {
 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();
   PA.preserve<GlobalsAA>();
   return PA;
 }
 } // namespace llvm