[OptRemark,LDist] RFC: Add hotness attribute

[anemet]

Summary

This is the first set of changes implementing the RFC from
http://thread.gmane.org/gmane.comp.compilers.llvm.devel/98334

This is a cross-sectional patch; rather than implementing the hotness
attribute for all optimization remarks and all passes in a patch set, it
implements it for the 'missed-optimization' remark for Loop
Distribution. My goal is to shake out the design issues before scaling
it up to other types and passes.

Hotness is computed as an integer as the multiplication of the block
frequency with the function entry count. It's only printed in opt
currently since clang prints the diagnostic fields directly. E.g.:

remark: /tmp/t.c:3:3: loop not distributed: use -Rpass-analysis=loop-distribute for more info (hotness: 300)

A new API added is similar to emitOptimizationRemarkMissed. The
difference is that it additionally takes a code region that the
diagnostic corresponds to. From this, hotness is computed using BFI.
The new API is exposed via an analysis pass so that it can be made
dependent on LazyBFI. (Thanks to Hal for the analysis pass idea.)

This feature can all be enabled by setDiagnosticHotnessRequested in the
LLVM context. If this is off, LazyBFI is not calculated (D22141) so
there should be no overhead.

A new command-line option is added to turn this on in opt.

My plan is to switch all user of emitOptimizationRemark* to use this
module instead.

[anemet]

Add a missing function comment

[anemet]

Add a more convenient, alternative API for emitOptimizationRemarkMissed that
takes a loop and derives debug location and the IR Value internally.

[anemet]

Fix rebase glitch.

[anemet]

BFI::getBlockProfileCount already multiplies the BB count with the function's
count. Update hotness calculation and test.

[anemet]

Use getAnalysisIfAvailable to access BFI. If the pass does not currently use
BFI the original code wouldn't work. This does not trigger with LV but
porting Loop Distribution tripped on this.

[anemet]

@hfinkel it would be great if you could look at this. I have bunch of patches lined up after this with most of them post-commit-reviewable as long as you sign off on the design sketched out in this patch. Thanks!

[mzolotukhin]

A couple of typos spotted:

include/llvm/IR/DiagnosticInfo.h

425 ↗

(On Diff #62579)

s/informatin/information/
s/this zero/this is zero/

[anemet]

Thanks, Michael!

[hfinkel]

Thanks for working on this.

Is there a reason that these should be free functions instead of just making an Analysis pass with these as member functions? Making it an actual analysis will make it easy to require BFI. I think the "get if available" will end up being confusing (sometimes the filtering will work, sometimes not, depending on what gets invalidated when - especially with the new pass manager where this will be dynamic). Side note: We might want to make BFI lazy (or compute-on-first-query instead of actually computing things in runOnFunction).

include/llvm/IR/DiagnosticInfo.h

426 ↗

(On Diff #62579)

I think that we should differentiate between 0 and "no information". Maybe make this an Optional<uint64_t>?

lib/Analysis/OptimizationDiagnosticInfo.cpp

27 ↗

(On Diff #62579)

V might not always be a BB. This is something you were planning to generalize later?

[anemet]

In D21771#474604, @hfinkel wrote:

Is there a reason that these should be free functions instead of just making an Analysis pass with these as member functions? Making it an actual analysis will make it easy to require BFI. I think the "get if available" will end up being confusing (sometimes the filtering will work, sometimes not, depending on what gets invalidated when - especially with the new pass manager where this will be dynamic). Side note: We might want to make BFI lazy (or compute-on-first-query instead of actually computing things in runOnFunction).

Thanks for feedback!

The main reason these are free function so that it's easy to transition from the current API which uses free function (i.e. no major reason ;-). But yes, I wasn't completely happy with the BFI dependence either. It was working for LV that already depends on BFI but it didn't work for LoopDist for example. I added a new command flag and made the BFI dependence conditional on that.

How would an analysis pass help this. It feels orthogonal because we can't unconditionally require the new analysis either because it would populate BFI. I actually think that the new PM will help here because we could then hopefully check F.getEntryCount to see if PGO is available and only query BFI then, no?

Thinking a bit more, having this packaged as a new analysis pass may help because right now I have this in LoopDist for example:

+ if (PassRemarksWithHotness) <------ this is the command line option
+ AU.addRequired<BlockFrequencyInfoWrapperPass>();

but when the emitOptimizationRemark* function is called, I don't know if BFI is actually available (i.e. whether the pass was updated with the change above) so I need to use getAnalysisIfAvailable. Having the Analysis pass would take care of this because it's all under the control of the analysis pass.

Regarding the BFI population, I was thinking of using the command line flag (-pass-remarks-with-hotness). Hopefully we can somehow detect if PGO is available and then enable the flag automatically at some higher level. As you say, lazy BFI would be helpful but it would be nice not to directly depend on that feature for this project.

What do you think?

Adam

include/llvm/IR/DiagnosticInfo.h

426 ↗

(On Diff #62579)

I was undecided about this but you moved the needle now :). Will add.

lib/Analysis/OptimizationDiagnosticInfo.cpp

27 ↗

(On Diff #62579)

Yes, hence the cast so that we'd assert for anything else right. I hope to refine this part of the design as more things get hooked up.

[hfinkel]

In D21771#474636, @anemet wrote:

In D21771#474604, @hfinkel wrote:

Is there a reason that these should be free functions instead of just making an Analysis pass with these as member functions? Making it an actual analysis will make it easy to require BFI. I think the "get if available" will end up being confusing (sometimes the filtering will work, sometimes not, depending on what gets invalidated when - especially with the new pass manager where this will be dynamic). Side note: We might want to make BFI lazy (or compute-on-first-query instead of actually computing things in runOnFunction).

Thanks for feedback!

The main reason these are free function so that it's easy to transition from the current API which uses free function (i.e. no major reason ;-). But yes, I wasn't completely happy with the BFI dependence either. It was working for LV that already depends on BFI but it didn't work for LoopDist for example. I added a new command flag and made the BFI dependence conditional on that.

How would an analysis pass help this. It feels orthogonal because we can't unconditionally require the new analysis either because it would populate BFI. I actually think that the new PM will help here because we could then hopefully check F.getEntryCount to see if PGO is available and only query BFI then, no?

Thinking a bit more, having this packaged as a new analysis pass may help because right now I have this in LoopDist for example:

+ if (PassRemarksWithHotness) <------ this is the command line option
+ AU.addRequired<BlockFrequencyInfoWrapperPass>();

but when the emitOptimizationRemark* function is called, I don't know if BFI is actually available (i.e. whether the pass was updated with the change above) so I need to use getAnalysisIfAvailable. Having the Analysis pass would take care of this because it's all under the control of the analysis pass.

Exactly. Looks like we both agree on packaging this as an analysis pass...

Regarding the BFI population, I was thinking of using the command line flag (-pass-remarks-with-hotness). Hopefully we can somehow detect if PGO is available and then enable the flag automatically at some higher level. As you say, lazy BFI would be helpful but it would be nice not to directly depend on that feature for this project.

What do you think?

I think we should add some global state (perhaps something that is set when setDiagnosticHandler is called, or via some nearby API) that indicates whether diagnostic "hotness" is requested. We should package these wrappers as an analysis pass, and that pass should have a hard requirement on BFI.

Regarding making BFI lazy, I think we probably need to do this, at least in the trivial sense: Add a Boolean to BFI indicating whether or not 'calculate' has been called, and add 'if (!Calculated) calculate()' to BFI::getBlockFreq, BFI::getBlockProfileCount and BFI::setBlockFreq, remove the call to calculate in runOnFunction. There is a more-complicated sense of making BFI lazy -- only computing frequencies for parts of the function as required -- but that's a larger project.

Then if we make no BFI calls, then requiring it is free. I think then the infrastructure will just work for us.

Adam

[anemet]

In D21771#474673, @hfinkel wrote:

Exactly. Looks like we both agree on packaging this as an analysis pass...

Yes, sorry if it wasn't completely clear. I was more and more convinced as I was replying...

I think we should add some global state (perhaps something that is set when setDiagnosticHandler is called, or via some nearby API) that indicates whether diagnostic "hotness" is requested. We should package these wrappers as an analysis pass, and that pass should have a hard requirement on BFI.

Regarding making BFI lazy, I think we probably need to do this, at least in the trivial sense: Add a Boolean to BFI indicating whether or not 'calculate' has been called, and add 'if (!Calculated) calculate()' to BFI::getBlockFreq, BFI::getBlockProfileCount and BFI::setBlockFreq, remove the call to calculate in runOnFunction. There is a more-complicated sense of making BFI lazy -- only computing frequencies for parts of the function as required -- but that's a larger project.

Then if we make no BFI calls, then requiring it is free. I think then the infrastructure will just work for us.

Yes this should work.

I guess what you're saying is that we can't make the pass conditionally dependent on BFI because we have no access to the LLVM context in getAnalysisUsage. So while this may work for a command-line option, it does not work for a global in the LLVM context sense?

[hfinkel]

In D21771#474692, @anemet wrote:

In D21771#474673, @hfinkel wrote:

Exactly. Looks like we both agree on packaging this as an analysis pass...

Yes, sorry if it wasn't completely clear. I was more and more convinced as I was replying...

I think we should add some global state (perhaps something that is set when setDiagnosticHandler is called, or via some nearby API) that indicates whether diagnostic "hotness" is requested. We should package these wrappers as an analysis pass, and that pass should have a hard requirement on BFI.

Regarding making BFI lazy, I think we probably need to do this, at least in the trivial sense: Add a Boolean to BFI indicating whether or not 'calculate' has been called, and add 'if (!Calculated) calculate()' to BFI::getBlockFreq, BFI::getBlockProfileCount and BFI::setBlockFreq, remove the call to calculate in runOnFunction. There is a more-complicated sense of making BFI lazy -- only computing frequencies for parts of the function as required -- but that's a larger project.

Then if we make no BFI calls, then requiring it is free. I think then the infrastructure will just work for us.

Yes this should work.

I guess what you're saying is that we can't make the pass conditionally dependent on BFI because we have no access to the LLVM context in getAnalysisUsage. So while this may work for a command-line option, it does not work for a global in the LLVM context sense?

Yes, this is my thought.

[anemet]

Thanks very much for the feedback, Hal! I'll add a new review for the BFI changes and then update this one as well.

Adam

[anemet]

Address Hal's comments: repackage as an analysis pass, use lazy BFI

[anemet]

Updated to use the LazyBFI analysis from D22141.

Also switched to make it work with Loop Distribution rather than the
Vectorizer. This way I can also test the laziness if the global flag is off.
(LV itself depends on BFI.)

[anemet]

Hi @hfinkel,

I've updated this yesterday to reflect the commit of the LazyBFI pass. So this is ready to be looked at whenever you get a chance.

Thanks,
Adam

[hfinkel]

LGTM. Thanks!

include/llvm/IR/LLVMContext.h

178 ↗

(On Diff #63741)

diagnostic -> diagnostics

181 ↗

(On Diff #63741)

diagnostic -> diagnostics

[anemet]

Thanks very much for the review, Hal!

llvm/trunk/include/llvm/Analysis/OptimizationDiagnosticInfo.h

+//===- OptimizationDiagnosticInfo.h - Optimization Diagnostic ---*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Optimization diagnostic interfaces.  It's packaged as an analysis pass so
+// that by using this service passes become dependent on BFI as well.  BFI is
+// used to compute the "hotness" of the diagnostic message.
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_IR_OPTIMIZATIONDIAGNOSTICINFO_H
+#define LLVM_IR_OPTIMIZATIONDIAGNOSTICINFO_H
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/Pass.h"
+
+namespace llvm {
+class BlockFrequencyInfo;
+class DebugLoc;
+class Function;
+class LLVMContext;
+class Loop;
+class Pass;
+class Twine;
+class Value;
+
+class OptimizationRemarkEmitter : public FunctionPass {
+public:
+  OptimizationRemarkEmitter();
+
+  /// Emit an optimization-missed message.
+  ///
+  /// \p PassName is the name of the pass emitting the message. If
+  /// -Rpass-missed= is given and the name matches the regular expression in
+  /// -Rpass, then the remark will be emitted. \p Fn is the function triggering
+  /// the remark, \p DLoc is the debug location where the diagnostic is
+  /// generated. \p V is the IR Value that identifies the code region. \p Msg is
+  /// the message string to use.
+  void emitOptimizationRemarkMissed(const char *PassName, const DebugLoc &DLoc,
+                                    Value *V, const Twine &Msg);
+
+  /// \brief Same as above but derives the IR Value for the code region and the
+  /// debug location from the Loop parameter \p L.
+  void emitOptimizationRemarkMissed(const char *PassName, Loop *L,
+                                    const Twine &Msg);
+
+  bool runOnFunction(Function &F) override;
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override;
+
+  static char ID;
+
+private:
+  Function *F;
+
+  BlockFrequencyInfo *BFI;
+
+  Optional<uint64_t> computeHotness(Value *V);
+};
+}
+
+#endif // LLVM_IR_OPTIMIZATIONDIAGNOSTICINFO_H

llvm/trunk/include/llvm/IR/DiagnosticInfo.h

 // This file declares the different classes involved in low level diagnostics.
 //
 // Diagnostics reporting is still done as part of the LLVMContext.
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_IR_DIAGNOSTICINFO_H
 #define LLVM_IR_DIAGNOSTICINFO_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/Support/CBindingWrapping.h"
 #include "llvm-c/Types.h"
 #include <functional>
 #include <string>
 
public:
   /// the location information to use in the diagnostic. If line table
   /// information is available, the diagnostic will include the source code
   /// location. \p Msg is the message to show. Note that this class does not
   /// copy this message, so this reference must be valid for the whole life time
   /// of the diagnostic.
   DiagnosticInfoOptimizationBase(enum DiagnosticKind Kind,
                                  enum DiagnosticSeverity Severity,
                                  const char *PassName, const Function &Fn,
-                                 const DebugLoc &DLoc, const Twine &Msg)
+                                 const DebugLoc &DLoc, const Twine &Msg,
+                                 Optional<uint64_t> Hotness = None)
       : DiagnosticInfoWithDebugLocBase(Kind, Severity, Fn, DLoc),
-        PassName(PassName), Msg(Msg) {}
+        PassName(PassName), Msg(Msg), Hotness(Hotness) {}
 
   /// \see DiagnosticInfo::print.
   void print(DiagnosticPrinter &DP) const override;
 
   /// Return true if this optimization remark is enabled by one of
   /// of the LLVM command line flags (-pass-remarks, -pass-remarks-missed,
   /// or -pass-remarks-analysis). Note that this only handles the LLVM
   /// flags. We cannot access Clang flags from here (they are handled

 private:
   /// Name of the pass that triggers this report. If this matches the
   /// regular expression given in -Rpass=regexp, then the remark will
   /// be emitted.
   const char *PassName;
 
   /// Message to report.
   const Twine &Msg;
+
+  /// If profile information is available, this is the number of times the
+  /// corresponding code was executed in a profile instrumentation run.
+  Optional<uint64_t> Hotness;
 };
 
 /// Diagnostic information for applied optimization remarks.
 class DiagnosticInfoOptimizationRemark : public DiagnosticInfoOptimizationBase {
 public:
   /// \p PassName is the name of the pass emitting this diagnostic. If
   /// this name matches the regular expression given in -Rpass=, then the
   /// diagnostic will be emitted. \p Fn is the function where the diagnostic
public:
   /// diagnostic will be emitted. \p Fn is the function where the diagnostic
   /// is being emitted. \p DLoc is the location information to use in the
   /// diagnostic. If line table information is available, the diagnostic
   /// will include the source code location. \p Msg is the message to show.
   /// Note that this class does not copy this message, so this reference
   /// must be valid for the whole life time of the diagnostic.
   DiagnosticInfoOptimizationRemarkMissed(const char *PassName,
                                          const Function &Fn,
-                                         const DebugLoc &DLoc, const Twine &Msg)
+                                         const DebugLoc &DLoc, const Twine &Msg,
+                                         Optional<uint64_t> Hotness = None)
       : DiagnosticInfoOptimizationBase(DK_OptimizationRemarkMissed, DS_Remark,
-                                       PassName, Fn, DLoc, Msg) {}
+                                       PassName, Fn, DLoc, Msg, Hotness) {}
 
   static bool classof(const DiagnosticInfo *DI) {
     return DI->getKind() == DK_OptimizationRemarkMissed;
   }
 
   /// \see DiagnosticInfoOptimizationBase::isEnabled.
   bool isEnabled() const override;
 };

llvm/trunk/include/llvm/IR/LLVMContext.h

public:
   /// getDiagnosticHandler - Return the diagnostic handler set by
   /// setDiagnosticHandler.
   DiagnosticHandlerTy getDiagnosticHandler() const;
 
   /// getDiagnosticContext - Return the diagnostic context set by
   /// setDiagnosticContext.
   void *getDiagnosticContext() const;
 
+  /// \brief Return if a code hotness metric should be included in optimization
+  /// diagnostics.
+  bool getDiagnosticHotnessRequested() const;
+  /// \brief Set if a code hotness metric should be included in optimization
+  /// diagnostics.
+  void setDiagnosticHotnessRequested(bool Requested);
+
   /// \brief Get the prefix that should be printed in front of a diagnostic of
   ///        the given \p Severity
   static const char *getDiagnosticMessagePrefix(DiagnosticSeverity Severity);
 
   /// \brief Report a message to the currently installed diagnostic handler.
   ///
   /// This function returns, in particular in the case of error reporting
   /// (DI.Severity == \a DS_Error), so the caller should leave the compilation

llvm/trunk/include/llvm/InitializePasses.h

 void initializeNameAnonFunctionPass(PassRegistry &);
 void initializeNaryReassociatePass(PassRegistry&);
 void initializeNoAAPass(PassRegistry&);
 void initializeObjCARCAAWrapperPassPass(PassRegistry&);
 void initializeObjCARCAPElimPass(PassRegistry&);
 void initializeObjCARCContractPass(PassRegistry&);
 void initializeObjCARCExpandPass(PassRegistry&);
 void initializeObjCARCOptPass(PassRegistry&);
+void initializeOptimizationRemarkEmitterPass(PassRegistry&);
 void initializeOptimizePHIsPass(PassRegistry&);
 void initializePAEvalPass(PassRegistry &);
 void initializePEIPass(PassRegistry&);
 void initializePGOIndirectCallPromotionLegacyPassPass(PassRegistry&);
 void initializePGOInstrumentationGenLegacyPassPass(PassRegistry&);
 void initializePGOInstrumentationUseLegacyPassPass(PassRegistry&);
 void initializePHIEliminationPass(PassRegistry&);
 void initializePhysicalRegisterUsageInfoPass(PassRegistry &);

llvm/trunk/lib/Analysis/Analysis.cpp

void llvm::initializeAnalysis(PassRegistry &Registry) {
   initializeLintPass(Registry);
   initializeLoopInfoWrapperPassPass(Registry);
   initializeMemDepPrinterPass(Registry);
   initializeMemDerefPrinterPass(Registry);
   initializeMemoryDependenceWrapperPassPass(Registry);
   initializeModuleDebugInfoPrinterPass(Registry);
   initializeModuleSummaryIndexWrapperPassPass(Registry);
   initializeObjCARCAAWrapperPassPass(Registry);
+  initializeOptimizationRemarkEmitterPass(Registry);
   initializePostDominatorTreeWrapperPassPass(Registry);
   initializeRegionInfoPassPass(Registry);
   initializeRegionViewerPass(Registry);
   initializeRegionPrinterPass(Registry);
   initializeRegionOnlyViewerPass(Registry);
   initializeRegionOnlyPrinterPass(Registry);
   initializeSCEVAAWrapperPassPass(Registry);
   initializeScalarEvolutionWrapperPassPass(Registry);

llvm/trunk/lib/Analysis/CMakeLists.txt

add_llvm_library(LLVMAnalysis
   MemoryBuiltins.cpp
   MemoryDependenceAnalysis.cpp
   MemoryLocation.cpp
   ModuleDebugInfoPrinter.cpp
   ModuleSummaryAnalysis.cpp
   ObjCARCAliasAnalysis.cpp
   ObjCARCAnalysisUtils.cpp
   ObjCARCInstKind.cpp
+  OptimizationDiagnosticInfo.cpp
   OrderedBasicBlock.cpp
   PHITransAddr.cpp
   PostDominators.cpp
   ProfileSummaryInfo.cpp
   PtrUseVisitor.cpp
   RegionInfo.cpp
   RegionPass.cpp
   RegionPrinter.cpp

llvm/trunk/lib/Analysis/OptimizationDiagnosticInfo.cpp

+//===- OptimizationDiagnosticInfo.cpp - Optimization Diagnostic -*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Optimization diagnostic interfaces.  It's packaged as an analysis pass so
+// that by using this service passes become dependent on BFI as well.  BFI is
+// used to compute the "hotness" of the diagnostic message.
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
+#include "llvm/Analysis/LazyBlockFrequencyInfo.h"
+#include "llvm/Analysis/LoopInfo.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/LLVMContext.h"
+
+using namespace llvm;
+
+OptimizationRemarkEmitter::OptimizationRemarkEmitter() : FunctionPass(ID) {
+  initializeOptimizationRemarkEmitterPass(*PassRegistry::getPassRegistry());
+}
+
+Optional<uint64_t> OptimizationRemarkEmitter::computeHotness(Value *V) {
+  if (!BFI)
+    return None;
+
+  return BFI->getBlockProfileCount(cast<BasicBlock>(V));
+}
+
+void OptimizationRemarkEmitter::emitOptimizationRemarkMissed(
+    const char *PassName, const DebugLoc &DLoc, Value *V, const Twine &Msg) {
+  LLVMContext &Ctx = F->getContext();
+  Ctx.diagnose(DiagnosticInfoOptimizationRemarkMissed(PassName, *F, DLoc, Msg,
+                                                      computeHotness(V)));
+}
+
+void OptimizationRemarkEmitter::emitOptimizationRemarkMissed(
+    const char *PassName, Loop *L, const Twine &Msg) {
+  emitOptimizationRemarkMissed(PassName, L->getStartLoc(), L->getHeader(), Msg);
+}
+
+bool OptimizationRemarkEmitter::runOnFunction(Function &Fn) {
+  F = &Fn;
+
+  if (Fn.getContext().getDiagnosticHotnessRequested())
+    BFI = &getAnalysis<LazyBlockFrequencyInfoPass>().getBFI();
+  else
+    BFI = nullptr;
+
+  return false;
+}
+
+void OptimizationRemarkEmitter::getAnalysisUsage(AnalysisUsage &AU) const {
+  LazyBlockFrequencyInfoPass::getLazyBFIAnalysisUsage(AU);
+  AU.setPreservesAll();
+}
+
+char OptimizationRemarkEmitter::ID = 0;
+static const char ore_name[] = "Optimization Remark Emitter";
+#define ORE_NAME "opt-remark-emitter"
+
+INITIALIZE_PASS_BEGIN(OptimizationRemarkEmitter, ORE_NAME, ore_name, false,
+                      true)
+INITIALIZE_PASS_DEPENDENCY(LazyBFIPass)
+INITIALIZE_PASS_END(OptimizationRemarkEmitter, ORE_NAME, ore_name, false, true)

llvm/trunk/lib/IR/DiagnosticInfo.cpp

const std::string DiagnosticInfoWithDebugLocBase::getLocationStr() const {
   unsigned Column = 0;
   if (isLocationAvailable())
     getLocation(&Filename, &Line, &Column);
   return (Filename + ":" + Twine(Line) + ":" + Twine(Column)).str();
 }
 
 void DiagnosticInfoOptimizationBase::print(DiagnosticPrinter &DP) const {
   DP << getLocationStr() << ": " << getMsg();
+  if (Hotness)
+    DP << " (hotness: " << *Hotness << ")";
 }
 
 bool DiagnosticInfoOptimizationRemark::isEnabled() const {
   return PassRemarksOptLoc.Pattern &&
          PassRemarksOptLoc.Pattern->match(getPassName());
 }
 
 bool DiagnosticInfoOptimizationRemarkMissed::isEnabled() const {

llvm/trunk/lib/IR/LLVMContext.cpp

 void LLVMContext::setDiagnosticHandler(DiagnosticHandlerTy DiagnosticHandler,
                                        void *DiagnosticContext,
                                        bool RespectFilters) {
   pImpl->DiagnosticHandler = DiagnosticHandler;
   pImpl->DiagnosticContext = DiagnosticContext;
   pImpl->RespectDiagnosticFilters = RespectFilters;
 }
 
+void LLVMContext::setDiagnosticHotnessRequested(bool Requested) {
+  pImpl->DiagnosticHotnessRequested = Requested;
+}
+bool LLVMContext::getDiagnosticHotnessRequested() const {
+  return pImpl->DiagnosticHotnessRequested;
+}
+
 LLVMContext::DiagnosticHandlerTy LLVMContext::getDiagnosticHandler() const {
   return pImpl->DiagnosticHandler;
 }
 
 void *LLVMContext::getDiagnosticContext() const {
   return pImpl->DiagnosticContext;
 }
 

llvm/trunk/lib/IR/LLVMContextImpl.h

public:
   SmallPtrSet<Module*, 4> OwnedModules;
   
   LLVMContext::InlineAsmDiagHandlerTy InlineAsmDiagHandler;
   void *InlineAsmDiagContext;
 
   LLVMContext::DiagnosticHandlerTy DiagnosticHandler;
   void *DiagnosticContext;
   bool RespectDiagnosticFilters;
+  bool DiagnosticHotnessRequested;
 
   LLVMContext::YieldCallbackTy YieldCallback;
   void *YieldOpaqueHandle;
 
   typedef DenseMap<APInt, ConstantInt *, DenseMapAPIntKeyInfo> IntMapTy;
   IntMapTy IntConstants;
 
   typedef DenseMap<APFloat, ConstantFP *, DenseMapAPFloatKeyInfo> FPMapTy;

llvm/trunk/lib/IR/LLVMContextImpl.cpp

: TheTrueVal(nullptr), TheFalseVal(nullptr),
     Int32Ty(C, 32),
     Int64Ty(C, 64),
     Int128Ty(C, 128) {
   InlineAsmDiagHandler = nullptr;
   InlineAsmDiagContext = nullptr;
   DiagnosticHandler = nullptr;
   DiagnosticContext = nullptr;
   RespectDiagnosticFilters = false;
+  DiagnosticHotnessRequested = false;
   YieldCallback = nullptr;
   YieldOpaqueHandle = nullptr;
   NamedStructTypesUniqueID = 0;
 }
 
 LLVMContextImpl::~LLVMContextImpl() {
   // NOTE: We need to delete the contents of OwnedModules, but Module's dtor
   // will call LLVMContextImpl::removeModule, thus invalidating iterators into

llvm/trunk/lib/Transforms/Scalar/LoopDistribute.cpp

 // run-time disambiguate potentially overlapping arrays.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/EquivalenceClasses.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/LoopAccessAnalysis.h"
 #include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/Transforms/Utils/LoopUtils.h"

 private:
   AccessesType Accesses;
 };
 
 /// \brief The actual class performing the per-loop work.
 class LoopDistributeForLoop {
 public:
   LoopDistributeForLoop(Loop *L, Function *F, LoopInfo *LI, DominatorTree *DT,
-                        ScalarEvolution *SE)
-      : L(L), F(F), LI(LI), LAI(nullptr), DT(DT), SE(SE) {
+                        ScalarEvolution *SE, OptimizationRemarkEmitter *ORE)
+      : L(L), F(F), LI(LI), LAI(nullptr), DT(DT), SE(SE), ORE(ORE) {
     setForced();
   }
 
   /// \brief Try to distribute an inner-most loop.
   bool processLoop(LoopAccessLegacyAnalysis *LAA) {
     assert(L->empty() && "Only process inner loops.");
 
     DEBUG(dbgs() << "\nLDist: In \"" << L->getHeader()->getParent()->getName()
public:
   /// \brief Provide diagnostics then \return with false.
   bool fail(llvm::StringRef Message) {
     LLVMContext &Ctx = F->getContext();
     bool Forced = isForced().getValueOr(false);
 
     DEBUG(dbgs() << "Skipping; " << Message << "\n");
 
     // With Rpass-missed report that distribution failed.
-    emitOptimizationRemarkMissed(
-        Ctx, LDIST_NAME, *F, L->getStartLoc(),
+    ORE->emitOptimizationRemarkMissed(
+        LDIST_NAME, L,
         "loop not distributed: use -Rpass-analysis=loop-distribute for more "
         "info");
 
     // With Rpass-analysis report why.  This is on by default if distribution
     // was requested explicitly.
     emitOptimizationRemarkAnalysis(
         Ctx, Forced ? DiagnosticInfoOptimizationRemarkAnalysis::AlwaysPrint
                     : LDIST_NAME,
private:
   Loop *L;
   Function *F;
 
   // Analyses used.
   LoopInfo *LI;
   const LoopAccessInfo *LAI;
   DominatorTree *DT;
   ScalarEvolution *SE;
+  OptimizationRemarkEmitter *ORE;
 
   /// \brief Indicates whether distribution is forced to be enabled/disabled for
   /// the loop.
   ///
   /// If the optional has a value, it indicates whether distribution was forced
   /// to be enabled (true) or disabled (false).  If the optional has no value
   /// distribution was not forced either way.
   Optional<bool> IsForced;
public:
   bool runOnFunction(Function &F) override {
     if (skipFunction(F))
       return false;
 
     auto *LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
     auto *LAA = &getAnalysis<LoopAccessLegacyAnalysis>();
     auto *DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
     auto *SE = &getAnalysis<ScalarEvolutionWrapperPass>().getSE();
+    auto *ORE = &getAnalysis<OptimizationRemarkEmitter>();
 
     // Build up a worklist of inner-loops to vectorize. This is necessary as the
     // act of distributing a loop creates new loops and can invalidate iterators
     // across the loops.
     SmallVector<Loop *, 8> Worklist;
 
     for (Loop *TopLevelLoop : *LI)
       for (Loop *L : depth_first(TopLevelLoop))
         // We only handle inner-most loops.
         if (L->empty())
           Worklist.push_back(L);
 
     // Now walk the identified inner loops.
     bool Changed = false;
     for (Loop *L : Worklist) {
-      LoopDistributeForLoop LDL(L, &F, LI, DT, SE);
+      LoopDistributeForLoop LDL(L, &F, LI, DT, SE, ORE);
 
       // If distribution was forced for the specific loop to be
       // enabled/disabled, follow that.  Otherwise use the global flag.
       if (LDL.isForced().getValueOr(ProcessAllLoops))
         Changed |= LDL.processLoop(LAA);
     }
 
     // Process each loop nest in the function.
     return Changed;
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<ScalarEvolutionWrapperPass>();
     AU.addRequired<LoopInfoWrapperPass>();
     AU.addPreserved<LoopInfoWrapperPass>();
     AU.addRequired<LoopAccessLegacyAnalysis>();
     AU.addRequired<DominatorTreeWrapperPass>();
     AU.addPreserved<DominatorTreeWrapperPass>();
+    AU.addRequired<OptimizationRemarkEmitter>();
   }
 
   static char ID;
 
 private:
   /// \brief Whether distribution should be on in this function.  The per-loop
   /// pragma can override this.
   bool ProcessAllLoops;
 };
 } // anonymous namespace
 
 char LoopDistribute::ID;
 static const char ldist_name[] = "Loop Distribition";
 
 INITIALIZE_PASS_BEGIN(LoopDistribute, LDIST_NAME, ldist_name, false, false)
 INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(LoopAccessLegacyAnalysis)
 INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(ScalarEvolutionWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(OptimizationRemarkEmitter)
 INITIALIZE_PASS_END(LoopDistribute, LDIST_NAME, ldist_name, false, false)
 
 namespace llvm {
 FunctionPass *createLoopDistributePass(bool ProcessAllLoopsByDefault) {
   return new LoopDistribute(ProcessAllLoopsByDefault);
 }
 }

llvm/trunk/test/Transforms/LoopDistribute/diagnostics-with-hotness-lazy-BFI.ll

+; Check that BFI is not computed when -pass-remarks-with-hotness is off
+
+; RUN: opt -loop-distribute -S -pass-remarks-missed=loop-distribute \
+; RUN:     -debug-only=block-freq -pass-remarks-with-hotness < %s 2>&1 | FileCheck %s --check-prefix=HOTNESS
+; RUN: opt -loop-distribute -S -pass-remarks-missed=loop-distribute \
+; RUN:     -debug-only=block-freq                            < %s 2>&1 | FileCheck %s --check-prefix=NO_HOTNESS
+
+; REQUIRES: asserts
+
+; HOTNESS: block-frequency: forced
+; NO_HOTNESS-NOT: block-frequency: forced
+
+; This is the input program:
+;
+;     1 void forced (char *A, char *B, char *C, int N) {
+;     2 #pragma clang loop distribute(enable)
+;     3   for(int i = 0; i < N; i++) {
+;     4     A[i] = B[i] * C[i];
+;     5   }
+;     6 }
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+
+define void @forced(i8* %A, i8* %B, i8* %C, i32 %N) !dbg !7 !prof !22 {
+entry:
+  %cmp12 = icmp sgt i32 %N, 0, !dbg !9
+  br i1 %cmp12, label %ph, label %for.cond.cleanup, !dbg !10, !prof !23
+
+ph:
+  br label %for.body
+
+for.body:
+  %indvars.iv = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %ph ]
+  %arrayidx = getelementptr inbounds i8, i8* %B, i64 %indvars.iv, !dbg !12
+  %0 = load i8, i8* %arrayidx, align 1, !dbg !12, !tbaa !13
+  %arrayidx2 = getelementptr inbounds i8, i8* %C, i64 %indvars.iv, !dbg !16
+  %1 = load i8, i8* %arrayidx2, align 1, !dbg !16, !tbaa !13
+  %mul = mul i8 %1, %0, !dbg !17
+  %arrayidx6 = getelementptr inbounds i8, i8* %A, i64 %indvars.iv, !dbg !18
+  store i8 %mul, i8* %arrayidx6, align 1, !dbg !19, !tbaa !13
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1, !dbg !10
+  %lftr.wideiv = trunc i64 %indvars.iv.next to i32, !dbg !10
+  %exitcond = icmp eq i32 %lftr.wideiv, %N, !dbg !10
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body, !dbg !10, !llvm.loop !20, !prof !24
+
+for.cond.cleanup:
+  ret void, !dbg !11
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.9.0 (trunk 267633) (llvm/trunk 267675)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
+!1 = !DIFile(filename: "/tmp/t.c", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 2}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!7 = distinct !DISubprogram(name: "forced", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !2)
+!8 = !DISubroutineType(types: !2)
+!9 = !DILocation(line: 3, column: 20, scope: !7)
+!10 = !DILocation(line: 3, column: 3, scope: !7)
+!11 = !DILocation(line: 6, column: 1, scope: !7)
+!12 = !DILocation(line: 4, column: 12, scope: !7)
+!13 = !{!14, !14, i64 0}
+!14 = !{!"omnipotent char", !15, i64 0}
+!15 = !{!"Simple C/C++ TBAA"}
+!16 = !DILocation(line: 4, column: 19, scope: !7)
+!17 = !DILocation(line: 4, column: 17, scope: !7)
+!18 = !DILocation(line: 4, column: 5, scope: !7)
+!19 = !DILocation(line: 4, column: 10, scope: !7)
+!20 = distinct !{!20, !21}
+!21 = !{!"llvm.loop.distribute.enable", i1 true}
+!22 = !{!"function_entry_count", i64 3}
+!23 = !{!"branch_weights", i32 99, i32 1}
+!24 = !{!"branch_weights", i32 1, i32 99}

llvm/trunk/test/Transforms/LoopDistribute/diagnostics-with-hotness.ll

+; RUN: opt -loop-distribute -S -pass-remarks-missed=loop-distribute \
+; RUN:     -pass-remarks-with-hotness < %s 2>&1 | FileCheck %s --check-prefix=HOTNESS
+; RUN: opt -loop-distribute -S -pass-remarks-missed=loop-distribute \
+; RUN:                                < %s 2>&1 | FileCheck %s --check-prefix=NO_HOTNESS
+
+; REQUIRES: asserts
+
+; This is the input program:
+;
+;     1 void forced (char *A, char *B, char *C, int N) {
+;     2 #pragma clang loop distribute(enable)
+;     3   for(int i = 0; i < N; i++) {
+;     4     A[i] = B[i] * C[i];
+;     5   }
+;     6 }
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+; HOTNESS: remark: /tmp/t.c:3:3: loop not distributed: use -Rpass-analysis=loop-distribute for more info (hotness: 300)
+; NO_HOTNESS: remark: /tmp/t.c:3:3: loop not distributed: use -Rpass-analysis=loop-distribute for more info{{$}}
+
+define void @forced(i8* %A, i8* %B, i8* %C, i32 %N) !dbg !7 !prof !22 {
+entry:
+  %cmp12 = icmp sgt i32 %N, 0, !dbg !9
+  br i1 %cmp12, label %ph, label %for.cond.cleanup, !dbg !10, !prof !23
+
+ph:
+  br label %for.body
+
+for.body:
+  %indvars.iv = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %ph ]
+  %arrayidx = getelementptr inbounds i8, i8* %B, i64 %indvars.iv, !dbg !12
+  %0 = load i8, i8* %arrayidx, align 1, !dbg !12, !tbaa !13
+  %arrayidx2 = getelementptr inbounds i8, i8* %C, i64 %indvars.iv, !dbg !16
+  %1 = load i8, i8* %arrayidx2, align 1, !dbg !16, !tbaa !13
+  %mul = mul i8 %1, %0, !dbg !17
+  %arrayidx6 = getelementptr inbounds i8, i8* %A, i64 %indvars.iv, !dbg !18
+  store i8 %mul, i8* %arrayidx6, align 1, !dbg !19, !tbaa !13
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1, !dbg !10
+  %lftr.wideiv = trunc i64 %indvars.iv.next to i32, !dbg !10
+  %exitcond = icmp eq i32 %lftr.wideiv, %N, !dbg !10
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body, !dbg !10, !llvm.loop !20, !prof !24
+
+for.cond.cleanup:
+  ret void, !dbg !11
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.9.0 (trunk 267633) (llvm/trunk 267675)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
+!1 = !DIFile(filename: "/tmp/t.c", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 2}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!7 = distinct !DISubprogram(name: "forced", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !2)
+!8 = !DISubroutineType(types: !2)
+!9 = !DILocation(line: 3, column: 20, scope: !7)
+!10 = !DILocation(line: 3, column: 3, scope: !7)
+!11 = !DILocation(line: 6, column: 1, scope: !7)
+!12 = !DILocation(line: 4, column: 12, scope: !7)
+!13 = !{!14, !14, i64 0}
+!14 = !{!"omnipotent char", !15, i64 0}
+!15 = !{!"Simple C/C++ TBAA"}
+!16 = !DILocation(line: 4, column: 19, scope: !7)
+!17 = !DILocation(line: 4, column: 17, scope: !7)
+!18 = !DILocation(line: 4, column: 5, scope: !7)
+!19 = !DILocation(line: 4, column: 10, scope: !7)
+!20 = distinct !{!20, !21}
+!21 = !{!"llvm.loop.distribute.enable", i1 true}
+!22 = !{!"function_entry_count", i64 3}
+!23 = !{!"branch_weights", i32 99, i32 1}
+!24 = !{!"branch_weights", i32 1, i32 99}

llvm/trunk/tools/opt/opt.cpp

RunTwice("run-twice",
              cl::desc("Run all passes twice, re-using the same pass manager."),
              cl::init(false), cl::Hidden);
 
 static cl::opt<bool> DiscardValueNames(
     "discard-value-names",
     cl::desc("Discard names from Value (other than GlobalValue)."),
     cl::init(false), cl::Hidden);
 
+static cl::opt<bool> PassRemarksWithHotness(
+    "pass-remarks-with-hotness",
+    cl::desc("With PGO, include profile count in optimization remarks"),
+    cl::Hidden);
+
 static inline void addPass(legacy::PassManagerBase &PM, Pass *P) {
   // Add the pass to the pass manager...
   PM.add(P);
 
   // If we are verifying all of the intermediate steps, add the verifier...
   if (VerifyEach)
     PM.add(createVerifierPass());
 }
#endif
   }
 
   SMDiagnostic Err;
 
   Context.setDiscardValueNames(DiscardValueNames);
   if (!DisableDITypeMap)
     Context.enableDebugTypeODRUniquing();
 
+  if (PassRemarksWithHotness)
+    Context.setDiagnosticHotnessRequested(true);
+
   // Load the input module...
   std::unique_ptr<Module> M = parseIRFile(InputFilename, Err, Context);
 
   if (!M) {
     Err.print(argv[0], errs());
     return 1;
   }