[OptDiag] Updating Remarks in SampleProfile

Updating remark API to newer OptimizationDiagnosticInfo API. This
allows remarks to show up in diagnostic yaml file, and enables use
of opt-viewer tool.

Hotness information for remarks (L505 and L751) do not display hotness
information, most likely due to profile information not being
propagated yet. Unsure if this is the desired outcome.

Patch by Tarun Rajendran.

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

llvm-svn: 310763

7 files changed, 101 insertions, 37 deletions

diff --git a/llvm/include/llvm/IR/DiagnosticInfo.h b/llvm/include/llvm/IR/DiagnosticInfo.h
index 2a38071be5e..024b81a91cf 100644
--- a/llvm/include/llvm/IR/DiagnosticInfo.h
+++ b/llvm/include/llvm/IR/DiagnosticInfo.h
@@ -425,6 +425,7 @@ public:
     Argument(StringRef Key, unsigned N);
     Argument(StringRef Key, uint64_t N);
     Argument(StringRef Key, bool B) : Key(Key), Val(B ? "true" : "false") {}
+    Argument(StringRef Key, DebugLoc dl);
   };
 
   /// \p PassName is the name of the pass emitting this diagnostic. \p
diff --git a/llvm/include/llvm/Transforms/Instrumentation.h b/llvm/include/llvm/Transforms/Instrumentation.h
index 71aed33db72..41f209ebae9 100644
--- a/llvm/include/llvm/Transforms/Instrumentation.h
+++ b/llvm/include/llvm/Transforms/Instrumentation.h
@@ -111,7 +111,7 @@ bool isLegalToPromote(Instruction *Inst, Function *F, const char **Reason);
 Instruction *promoteIndirectCall(Instruction *Inst, Function *F, uint64_t Count,
                                  uint64_t TotalCount,
                                  bool AttachProfToDirectCall,
-                                 OptimizationRemarkEmitter *ORE = nullptr);
+                                 OptimizationRemarkEmitter *ORE);
 
 /// Options for the frontend instrumentation based profiling pass.
 struct InstrProfOptions {
diff --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp
index eeb6ce3dd50..99f6a33dc40 100644
--- a/llvm/lib/IR/DiagnosticInfo.cpp
+++ b/llvm/lib/IR/DiagnosticInfo.cpp
@@ -233,6 +233,16 @@ DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, unsigned N)
 DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, uint64_t N)
     : Key(Key), Val(utostr(N)) {}
 
+DiagnosticInfoOptimizationBase::Argument::Argument(StringRef Key, DebugLoc Loc)
+    : Key(Key), Loc(Loc) {
+  if (Loc) {
+    Val = (Loc->getFilename() + ":" + Twine(Loc.getLine()) + ":" +
+           Twine(Loc.getCol())).str();
+  } else {
+    Val = "<UNKNOWN LOCATION>";
+  }
+}
+
 void DiagnosticInfoOptimizationBase::print(DiagnosticPrinter &DP) const {
   DP << getLocationStr() << ": " << getMsg();
   if (Hotness)
diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index c8d07e63fa2..246680f04c4 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -29,6 +29,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/AssumptionCache.h"
 #include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
 #include "llvm/Analysis/PostDominators.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfo.h"
@@ -148,16 +149,16 @@ public:
   SampleProfileLoader(StringRef Name = SampleProfileFile)
       : DT(nullptr), PDT(nullptr), LI(nullptr), ACT(nullptr), Reader(),
         Samples(nullptr), Filename(Name), ProfileIsValid(false),
-        TotalCollectedSamples(0) {}
+        TotalCollectedSamples(0), ORE(nullptr) {}
 
   bool doInitialization(Module &M);
-  bool runOnModule(Module &M);
+  bool runOnModule(Module &M, ModuleAnalysisManager *AM);
   void setACT(AssumptionCacheTracker *A) { ACT = A; }
 
   void dump() { Reader->dump(); }
 
 protected:
-  bool runOnFunction(Function &F);
+  bool runOnFunction(Function &F, ModuleAnalysisManager *AM);
   unsigned getFunctionLoc(Function &F);
   bool emitAnnotations(Function &F);
   ErrorOr<uint64_t> getInstWeight(const Instruction &I);
@@ -249,6 +250,9 @@ protected:
   /// This is the sum of all the samples collected in all the functions executed
   /// at runtime.
   uint64_t TotalCollectedSamples;
+
+  /// \brief Optimization Remark Emitter used to emit diagnostic remarks.
+  OptimizationRemarkEmitter *ORE;
 };
 
 class SampleProfileLoaderLegacyPass : public ModulePass {
@@ -497,13 +501,17 @@ ErrorOr<uint64_t> SampleProfileLoader::getInstWeight(const Instruction &Inst) {
     bool FirstMark =
         CoverageTracker.markSamplesUsed(FS, LineOffset, Discriminator, R.get());
     if (FirstMark) {
-      const Function *F = Inst.getParent()->getParent();
-      LLVMContext &Ctx = F->getContext();
-      emitOptimizationRemark(
-          Ctx, DEBUG_TYPE, *F, DLoc,
-          Twine("Applied ") + Twine(*R) +
-              " samples from profile (offset: " + Twine(LineOffset) +
-              ((Discriminator) ? Twine(".") + Twine(Discriminator) : "") + ")");
+      if (Discriminator)
+        ORE->emit(OptimizationRemarkAnalysis(DEBUG_TYPE, "AppliedSamples", &Inst)
+                  << "Applied " << ore::NV("NumSamples", *R)
+                  << " samples from profile (offset: "
+                  << ore::NV("LineOffset", LineOffset) << "."
+                  << ore::NV("Discriminator", Discriminator) << ")");
+      else
+        ORE->emit(OptimizationRemarkAnalysis(DEBUG_TYPE, "AppliedSamples", &Inst)
+                  << "Applied " << ore::NV("NumSamples", *R)
+                  << " samples from profile (offset: "
+                  << ore::NV("LineOffset", LineOffset) << ")");
     }
     DEBUG(dbgs() << "    " << DLoc.getLine() << "."
                  << DIL->getBaseDiscriminator() << ":" << Inst
@@ -669,7 +677,6 @@ bool SampleProfileLoader::inlineHotFunctions(
     Function &F, DenseSet<GlobalValue::GUID> &ImportGUIDs) {
   DenseSet<Instruction *> PromotedInsns;
   bool Changed = false;
-  LLVMContext &Ctx = F.getContext();
   std::function<AssumptionCache &(Function &)> GetAssumptionCache = [&](
       Function &F) -> AssumptionCache & { return ACT->getAssumptionCache(F); };
   while (true) {
@@ -720,7 +727,7 @@ bool SampleProfileLoader::inlineHotFunctions(
             // We set the probability to 80% taken to indicate that the static
             // call is likely taken.
             DI = dyn_cast<Instruction>(
-                promoteIndirectCall(I, CalledFunction, 80, 100, false)
+                promoteIndirectCall(I, CalledFunction, 80, 100, false, ORE)
                     ->stripPointerCasts());
             PromotedInsns.insert(I);
           } else {
@@ -737,12 +744,14 @@ bool SampleProfileLoader::inlineHotFunctions(
         continue;
       }
       DebugLoc DLoc = I->getDebugLoc();
+      BasicBlock *BB = I->getParent();
       if (InlineFunction(CallSite(DI), IFI)) {
         LocalChanged = true;
-        emitOptimizationRemark(Ctx, DEBUG_TYPE, F, DLoc,
-                               Twine("inlined hot callee '") +
-                                   CalledFunction->getName() + "' into '" +
-                                   F.getName() + "'");
+        // The call to InlineFunction erases DI, so we can't pass it here.
+        ORE->emit(OptimizationRemark(DEBUG_TYPE, "HotInline", DLoc, BB)
+                  << "inlined hot callee '"
+                  << ore::NV("Callee", CalledFunction) << "' into '"
+                  << ore::NV("Caller", &F) << "'");
       }
     }
     if (LocalChanged) {
@@ -1213,7 +1222,7 @@ void SampleProfileLoader::propagateWeights(Function &F) {
                  << ".\n");
     SmallVector<uint32_t, 4> Weights;
     uint32_t MaxWeight = 0;
-    DebugLoc MaxDestLoc;
+    Instruction *MaxDestInst;
     for (unsigned I = 0; I < TI->getNumSuccessors(); ++I) {
       BasicBlock *Succ = TI->getSuccessor(I);
       Edge E = std::make_pair(BB, Succ);
@@ -1232,7 +1241,7 @@ void SampleProfileLoader::propagateWeights(Function &F) {
       if (Weight != 0) {
         if (Weight > MaxWeight) {
           MaxWeight = Weight;
-          MaxDestLoc = Succ->getFirstNonPHIOrDbgOrLifetime()->getDebugLoc();
+          MaxDestInst = Succ->getFirstNonPHIOrDbgOrLifetime();
         }
       }
     }
@@ -1247,13 +1256,9 @@ void SampleProfileLoader::propagateWeights(Function &F) {
       DEBUG(dbgs() << "SUCCESS. Found non-zero weights.\n");
       TI->setMetadata(llvm::LLVMContext::MD_prof,
                       MDB.createBranchWeights(Weights));
-      emitOptimizationRemark(
-          Ctx, DEBUG_TYPE, F, MaxDestLoc,
-          Twine("most popular destination for conditional branches at ") +
-              ((BranchLoc) ? Twine(BranchLoc->getFilename() + ":" +
-                                   Twine(BranchLoc.getLine()) + ":" +
-                                   Twine(BranchLoc.getCol()))
-                           : Twine("<UNKNOWN LOCATION>")));
+      ORE->emit(OptimizationRemark(DEBUG_TYPE, "PopularDest", MaxDestInst)
+                << "most popular destination for conditional branches at "
+                << ore::NV("CondBranchesLoc", BranchLoc));
     } else {
       DEBUG(dbgs() << "SKIPPED. All branch weights are zero.\n");
     }
@@ -1433,7 +1438,7 @@ ModulePass *llvm::createSampleProfileLoaderPass(StringRef Name) {
   return new SampleProfileLoaderLegacyPass(Name);
 }
 
-bool SampleProfileLoader::runOnModule(Module &M) {
+bool SampleProfileLoader::runOnModule(Module &M, ModuleAnalysisManager *AM) {
   if (!ProfileIsValid)
     return false;
 
@@ -1465,7 +1470,7 @@ bool SampleProfileLoader::runOnModule(Module &M) {
   for (auto &F : M)
     if (!F.isDeclaration()) {
       clearFunctionData();
-      retval |= runOnFunction(F);
+      retval |= runOnFunction(F, AM);
     }
   if (M.getProfileSummary() == nullptr)
     M.setProfileSummary(Reader->getSummary().getMD(M.getContext()));
@@ -1475,11 +1480,21 @@ bool SampleProfileLoader::runOnModule(Module &M) {
 bool SampleProfileLoaderLegacyPass::runOnModule(Module &M) {
   // FIXME: pass in AssumptionCache correctly for the new pass manager.
   SampleLoader.setACT(&getAnalysis<AssumptionCacheTracker>());
-  return SampleLoader.runOnModule(M);
+  return SampleLoader.runOnModule(M, nullptr);
 }
 
-bool SampleProfileLoader::runOnFunction(Function &F) {
+bool SampleProfileLoader::runOnFunction(Function &F, ModuleAnalysisManager *AM) {
   F.setEntryCount(0);
+  std::unique_ptr<OptimizationRemarkEmitter> OwnedORE;
+  if (AM) {
+    auto &FAM =
+        AM->getResult<FunctionAnalysisManagerModuleProxy>(*F.getParent())
+            .getManager();
+    ORE = &FAM.getResult<OptimizationRemarkEmitterAnalysis>(F);
+  } else {
+    OwnedORE = make_unique<OptimizationRemarkEmitter>(&F);
+    ORE = OwnedORE.get();
+  }
   Samples = Reader->getSamplesFor(F);
   if (Samples && !Samples->empty())
     return emitAnnotations(F);
@@ -1494,7 +1509,7 @@ PreservedAnalyses SampleProfileLoaderPass::run(Module &M,
 
   SampleLoader.doInitialization(M);
 
-  if (!SampleLoader.runOnModule(M))
+  if (!SampleLoader.runOnModule(M, &AM))
     return PreservedAnalyses::all();
 
   return PreservedAnalyses::none();
diff --git a/llvm/test/Transforms/SampleProfile/cov-zero-samples.ll b/llvm/test/Transforms/SampleProfile/cov-zero-samples.ll
index 5239d74fdc6..07b2d610612 100644
--- a/llvm/test/Transforms/SampleProfile/cov-zero-samples.ll
+++ b/llvm/test/Transforms/SampleProfile/cov-zero-samples.ll
@@ -1,5 +1,5 @@
-; RUN: opt < %s -instcombine -sample-profile -sample-profile-file=%S/Inputs/cov-zero-samples.prof -sample-profile-check-record-coverage=100 -pass-remarks=sample-profile -o /dev/null 2>&1 | FileCheck %s
-; RUN: opt < %s -passes="function(instcombine),sample-profile" -sample-profile-file=%S/Inputs/cov-zero-samples.prof -sample-profile-check-record-coverage=100 -pass-remarks=sample-profile -o /dev/null 2>&1 | FileCheck %s
+; RUN: opt < %s -instcombine -sample-profile -sample-profile-file=%S/Inputs/cov-zero-samples.prof -sample-profile-check-record-coverage=100 -pass-remarks=sample-profile -pass-remarks-analysis=sample-profile -o /dev/null 2>&1 | FileCheck %s
+; RUN: opt < %s -passes="function(instcombine),sample-profile" -sample-profile-file=%S/Inputs/cov-zero-samples.prof -sample-profile-check-record-coverage=100 -pass-remarks=sample-profile -pass-remarks-analysis=sample-profile -o /dev/null 2>&1 | FileCheck %s
 ;
 ; CHECK: remark: cov-zero-samples.cc:9:29: Applied 404065 samples from profile (offset: 2.1)
 ; CHECK: remark: cov-zero-samples.cc:10:9: Applied 443089 samples from profile (offset: 3)
diff --git a/llvm/test/Transforms/SampleProfile/inline-coverage.ll b/llvm/test/Transforms/SampleProfile/inline-coverage.ll
index 080876a4647..0a77e1ca335 100644
--- a/llvm/test/Transforms/SampleProfile/inline-coverage.ll
+++ b/llvm/test/Transforms/SampleProfile/inline-coverage.ll
@@ -1,5 +1,5 @@
-; RUN: opt < %s -instcombine -sample-profile -sample-profile-file=%S/Inputs/inline-coverage.prof -sample-profile-check-record-coverage=100 -sample-profile-check-sample-coverage=110 -pass-remarks=sample-profile -o /dev/null 2>&1 | FileCheck %s
-; RUN: opt < %s -passes="function(instcombine),sample-profile" -sample-profile-file=%S/Inputs/inline-coverage.prof -sample-profile-check-record-coverage=100 -sample-profile-check-sample-coverage=110 -pass-remarks=sample-profile -o /dev/null 2>&1 | FileCheck %s
+; RUN: opt < %s -instcombine -sample-profile -sample-profile-file=%S/Inputs/inline-coverage.prof -sample-profile-check-record-coverage=100 -sample-profile-check-sample-coverage=110 -pass-remarks=sample-profile -pass-remarks-analysis=sample-profile -o /dev/null 2>&1 | FileCheck %s
+; RUN: opt < %s -passes="function(instcombine),sample-profile" -sample-profile-file=%S/Inputs/inline-coverage.prof -sample-profile-check-record-coverage=100 -sample-profile-check-sample-coverage=110 -pass-remarks=sample-profile -pass-remarks-analysis=sample-profile -o /dev/null 2>&1 | FileCheck %s
 ;
 ; Original code:
 ;
diff --git a/llvm/test/Transforms/SampleProfile/remarks.ll b/llvm/test/Transforms/SampleProfile/remarks.ll
index dfb075ee00e..6285d69f9c7 100644
--- a/llvm/test/Transforms/SampleProfile/remarks.ll
+++ b/llvm/test/Transforms/SampleProfile/remarks.ll
@@ -1,6 +1,6 @@
-; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/remarks.prof -S -pass-remarks=sample-profile 2>&1 | FileCheck %s
+; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/remarks.prof -S -pass-remarks=sample-profile -pass-remarks-output=%t.opt.yaml 2>&1 | FileCheck %s
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/remarks.prof -S -pass-remarks=sample-profile 2>&1 | FileCheck %s
-;
+; RUN: FileCheck %s -check-prefix=YAML < %t.opt.yaml
 ; Original test case.
 ;
 ;     1    #include <stdlib.h>
@@ -27,6 +27,44 @@
 ; The predicate almost always chooses the 'else' branch.
 ; CHECK: remark: remarks.cc:9:15: most popular destination for conditional branches at remarks.cc:6:9
 
+; Checking to see if YAML file is generated and contains remarks
+;YAML:       --- !Passed
+;YAML-NEXT:  Pass:            sample-profile
+;YAML-NEXT:  Name:            HotInline
+;YAML-NEXT:  DebugLoc:        { File: remarks.cc, Line: 13, Column: 21 }
+;YAML-NEXT:  Function:        main
+;YAML-NEXT:  Args:
+;YAML-NEXT:    - String:          'inlined hot callee '''
+;YAML-NEXT:    - Callee:          _Z3foov
+;YAML-NEXT:      DebugLoc:        { File: remarks.cc, Line: 3, Column: 0 }
+;YAML-NEXT:    - String:          ''' into '''
+;YAML-NEXT:    - Caller:          main
+;YAML-NEXT:        DebugLoc:        { File: remarks.cc, Line: 13, Column: 0 }
+;YAML-NEXT:    - String:          ''''
+;YAML-NEXT:  ...
+;YAML:  --- !Analysis
+;YAML-NEXT:  Pass:            sample-profile
+;YAML-NEXT:  Name:            AppliedSamples
+;YAML-NEXT:  DebugLoc:        { File: remarks.cc, Line: 5, Column: 8 }
+;YAML-NEXT:  Function:        main
+;YAML-NEXT:  Args:
+;YAML-NEXT:    - String:          'Applied '
+;YAML-NEXT:    - NumSamples:      '18305'
+;YAML-NEXT:    - String:          ' samples from profile (offset: '
+;YAML-NEXT:    - LineOffset:      '2'
+;YAML-NEXT:    - String:          ')'
+;YAML-NEXT:  ...
+;YAML:  --- !Passed
+;YAML-NEXT:  Pass:            sample-profile
+;YAML-NEXT:  Name:            PopularDest
+;YAML-NEXT:  DebugLoc:        { File: remarks.cc, Line: 6, Column: 9 }
+;YAML-NEXT:  Function:        main
+;YAML-NEXT:  Args:
+;YAML-NEXT:    - String:          'most popular destination for conditional branches at '
+;YAML-NEXT:    - CondBranchesLoc: 'remarks.cc:5:3'
+;YAML-NEXT:      DebugLoc:        { File: remarks.cc, Line: 5, Column: 3 }
+;YAML-NEXT:  ...
+
 ; Function Attrs: nounwind uwtable
 define i64 @_Z3foov() #0 !dbg !4 {
 entry: