Output optimization remarks in YAML

[anemet]

Summary

This allows various presentation of this data using an external tool.
This was first recommended here[1].

As an example, consider this module:

1 int foo();
2 int bar();
3
4 int baz() {
5   return foo() + bar();
6 }

The inliner generates these missed-optimization remarks today (the
hotness information is pulled from PGO):

remark: /tmp/s.c:5:10: foo will not be inlined into baz (hotness: 30)
remark: /tmp/s.c:5:18: bar will not be inlined into baz (hotness: 30)

Now with -pass-remarks-output=<yaml-file>, we generate this YAML file:

--- !Missed
Pass:            inline
Name:            NotInlined
DebugLoc:        { File: /tmp/s.c, Line: 5, Column: 10 }
Function:        baz
Hotness:         30
Args:
  - Callee: foo
  - String:  will not be inlined into
  - Caller: baz
...
--- !Missed
Pass:            inline
Name:            NotInlined
DebugLoc:        { File: /tmp/s.c, Line: 5, Column: 18 }
Function:        baz
Hotness:         30
Args:
  - Callee: bar
  - String:  will not be inlined into
  - Caller: baz
...

This is a summary of the high-level decisions:

• There is a new streaming interface to emit optimization remarks. E.g. for the inliner remark above:
  
  ORE.emit(DiagnosticInfoOptimizationRemarkMissed( DEBUG_TYPE, "NotInlined", &I) << NV("Callee", Callee) << " will not be inlined into " << NV("Caller", CS.getCaller()) << setIsVerbose());

NV stands for named value and allows the YAML client to process a remark
using its name (NotInlined) and the named arguments (Callee and Caller)
without parsing the text of the message.

Subsequent patches will update ORE users to use the new streaming API.

• I am using YAML I/O for writing the YAML file. YAML I/O requires you to specify reading and writing at once but reading is highly non-trivial for some of the more complex LLVM types. Since it's not clear that we (ever) want to use LLVM to parse this YAML file, the code supports and asserts that we're writing only.

On the other hand, I did experiment that the class hierarchy starting at
DiagnosticInfoOptimizationBase can be mapped back from YAML generated
here (see D24479).

• The YAML stream is stored in the LLVM context.

• As before hotness is computed in the analysis pass instead of DiganosticInfo. This avoids the layering problem since BFI is in Analysis while DiagnosticInfo is in IR.

[1] https://reviews.llvm.org/D19678#419445

[anemet]

Ping. @hfinkel, would be great if you could comment on the direction in this RFC. I don't want to start updating the optimization before we settle on the API. Thanks!

[hfinkel]

In D24587#548958, @anemet wrote:

Ping. @hfinkel, would be great if you could comment on the direction in this RFC. I don't want to start updating the optimization before we settle on the API. Thanks!

Thanks for working on this! I think this direction makes sense. I think that we'd like the remark classes to be serializable as YAML, but we probably don't want the file writing code to be in the backend. The diagnostic classes get passed to the handler, which is often in the frontend, and we should probably let the handler decide what to do with them (e.g. where/if to write them to a file).

• string: will not be inlined into

We don't need to fix this all at once, but we need to think carefully about how remarks are identified. I think we want it to be very clear how each diagnostic is identified, and we don't want strings that compose the human-readable summary message to subtly also form an interface contract with external tools. So the string here is fine, but we'll want each diagnostic to have some name that is designed to be machine consumed, and probably key-value pairs which likewise have machine-consumable names.

[anemet]

Thanks for taking a look!

In D24587#550270, @hfinkel wrote:

Thanks for working on this! I think this direction makes sense. I think that we'd like the remark classes to be serializable as YAML, but we probably don't want the file writing code to be in the backend. The diagnostic classes get passed to the handler, which is often in the frontend, and we should probably let the handler decide what to do with them (e.g. where/if to write them to a file).

Then maybe I misunderstood what was meant by "doing this in the backend" in earlier conversations. I thought the idea was (which made sense to me) that all the backend user (e.g. clang, lto, opt) does it to instruct the backend to write the diagnostics into a file by invoking Context.setDiagnosticsOutputFile. I agree that this only gives binary control to the backend users but I am not sure we want more control at this time. Do you have something in mind?

It's also has the benefit that we don't need to duplicate this across the backend users.

• string: will not be inlined into

We don't need to fix this all at once, but we need to think carefully about how remarks are identified. I think we want it to be very clear how each diagnostic is identified, and we don't want strings that compose the human-readable summary message to subtly also form an interface contract with external tools. So the string here is fine, but we'll want each diagnostic to have some name that is designed to be machine consumed, and probably key-value pairs which likewise have machine-consumable names.

OK, so sounds like we want the interface to enforce a single string per message policy. So probably a format like this would work better:

ORE.emit(DiagnosticInfoOptimizationRemarkMissed(DEBUG_TYPE, &I)
         << "% will not be inlined into %"
         << Callee << CS.getCaller());

What do you think?

[hfinkel]

In D24587#550359, @anemet wrote:

Thanks for taking a look!

In D24587#550270, @hfinkel wrote:

Thanks for working on this! I think this direction makes sense. I think that we'd like the remark classes to be serializable as YAML, but we probably don't want the file writing code to be in the backend. The diagnostic classes get passed to the handler, which is often in the frontend, and we should probably let the handler decide what to do with them (e.g. where/if to write them to a file).

Then maybe I misunderstood what was meant by "doing this in the backend" in earlier conversations. I thought the idea was (which made sense to me) that all the backend user (e.g. clang, lto, opt) does it to instruct the backend to write the diagnostics into a file by invoking Context.setDiagnosticsOutputFile. I agree that this only gives binary control to the backend users but I am not sure we want more control at this time. Do you have something in mind?

It's also has the benefit that we don't need to duplicate this across the backend users.

I agree; this is good. The yaml::Output object can be a memory-mapped buffer. This implementation is fairly generic. The diagnostic handler does not get to filter what to serialize, true, but perhaps that's okay.

• string: will not be inlined into

We don't need to fix this all at once, but we need to think carefully about how remarks are identified. I think we want it to be very clear how each diagnostic is identified, and we don't want strings that compose the human-readable summary message to subtly also form an interface contract with external tools. So the string here is fine, but we'll want each diagnostic to have some name that is designed to be machine consumed, and probably key-value pairs which likewise have machine-consumable names.

OK, so sounds like we want the interface to enforce a single string per message policy. So probably a format like this would work better:

ORE.emit(DiagnosticInfoOptimizationRemarkMissed(DEBUG_TYPE, &I)
         << "% will not be inlined into %"
         << Callee << CS.getCaller());

What do you think?

I'd much rather we be forced to actually name these things. I don't want to have to change the tools that interpret the YAML files every time we update the messages to make them more helpful. How about this:

ORE.emit(DiagnosticInfoOptimizationRemarkMissed(DEBUG_TYPE, &I, "InlinerNotInlined")
         << NV("Callee", Callee) << " will not be inlined into "
         << NV("Caller", CS.getCaller()));

Where NV is supposed to stand for "named value", and the idea being that the name is used as the key for the YAML output.

lib/Analysis/OptimizationDiagnosticInfo.cpp

131 ↗

(On Diff #71442)

Why don't we call this regardless of whether we're saving the output? We might not want to change which messages the user sees while compiling just because we're also recording the output (I don't think that we do).

[anemet]

In D24587#550951, @hfinkel wrote:

ORE.emit(DiagnosticInfoOptimizationRemarkMissed(DEBUG_TYPE, &I, "InlinerNotInlined")
         << NV("Callee", Callee) << " will not be inlined into "
         << NV("Caller", CS.getCaller()));

Where NV is supposed to stand for "named value", and the idea being that the name is used as the key for the YAML output.

I think that this makes a lot of sense! Thanks for your input!

lib/Analysis/OptimizationDiagnosticInfo.cpp

131 ↗

(On Diff #71442)

Makes sense. The backend user can decide not to emit it at the higher level.

This is probably that we should use now for IsVerbose (D23415).

[anemet]

I will update the patch with all this and also add support for streaming the IsVerbose flag.

[anemet]

Addresses Hal's comments. I also got this ready for a formal review, please
take a look.

A few notes:

• The format is slightly changed with the named values and the remark name. I will update the review description to reflect this. The testcase is of course already updated.

• For the remark name I used "NotInlined" rather than "InlinerNotInlined" since the pass name is already provided

• Added support for IsVerbose in the streaming API. There is a new FIXME to handle IsVerbose in the backend users rather than in the backend. I will do this in a follow-up.

[hfinkel]

Great, thanks!

I'm glad you liked by suggestion for 'NV', because it is easy to type, but I do wonder if we'll have any issues because 'NV' is currently used as a variable name in a number of places. Having variable/type shadowing can certainly be confusing. What do you think?

[anemet]

In D24587#552967, @hfinkel wrote:

I'm glad you liked by suggestion for 'NV', because it is easy to type, but I do wonder if we'll have any issues because 'NV' is currently used as a variable name in a number of places. Having variable/type shadowing can certainly be confusing. What do you think?

I was worried about the same thing but I haven't started propagating this pattern yet so I don't know if we hit something.

My backup solution is creating a small namespace for NV and perhaps IsVerbose, something like "ore". Then you'd have to qualify NV:

ORE.emit(DiagnosticInfoOptimizationRemarkMissed( DEBUG_TYPE, "NotInlined", &I) 
                 << ore::NV("Callee", Callee) << " will not be inlined into "
                 << ore::NV("Caller", CS.getCaller())
                 << ore::setIsVerbose());

What do you think? Another option I was thinking is using std::make_pair directly.

[hfinkel]

In D24587#553036, @anemet wrote:

In D24587#552967, @hfinkel wrote:

I'm glad you liked by suggestion for 'NV', because it is easy to type, but I do wonder if we'll have any issues because 'NV' is currently used as a variable name in a number of places. Having variable/type shadowing can certainly be confusing. What do you think?

I was worried about the same thing but I haven't started propagating this pattern yet so I don't know if we hit something.

My backup solution is creating a small namespace for NV and perhaps IsVerbose, something like "ore". Then you'd have to qualify NV:

ORE.emit(DiagnosticInfoOptimizationRemarkMissed( DEBUG_TYPE, "NotInlined", &I) 
                 << ore::NV("Callee", Callee) << " will not be inlined into "
                 << ore::NV("Caller", CS.getCaller())
                 << ore::setIsVerbose());

What do you think? Another option I was thinking is using std::make_pair directly.

Using ore:: seems safer. That's a good option. I don't like std::make_pair, it is pretty long, and implies something about the implementation we might later wish to change.

[anemet]

In D24587#553073, @hfinkel wrote:

Using ore:: seems safer. That's a good option. I don't like std::make_pair, it is pretty long, and implies something about the implementation we might later wish to change.

Great. Do you want to see the namespace as part of this patch or is OK as a follow-on?

[hfinkel]

In D24587#553118, @anemet wrote:

In D24587#553073, @hfinkel wrote:

Using ore:: seems safer. That's a good option. I don't like std::make_pair, it is pretty long, and implies something about the implementation we might later wish to change.

Great. Do you want to see the namespace as part of this patch or is OK as a follow-on?

In D24587#553118, @anemet wrote:

In D24587#553073, @hfinkel wrote:

Using ore:: seems safer. That's a good option. I don't like std::make_pair, it is pretty long, and implies something about the implementation we might later wish to change.

Great. Do you want to see the namespace as part of this patch or is OK as a follow-on?

This patch (i.e. it should be committed that way if that's what we intend to use), but otherwise, this LGTM.

[anemet]

In D24587#553132, @hfinkel wrote:

This patch (i.e. it should be committed that way if that's what we intend to use), but otherwise, this LGTM.

Sure thing, thanks for the review!

[anemet]

For the record, the committed version adds the new namespace 'ore' as we talked about.

I was also able to simplify the underlying NV machinery. Essentially, I always create a string key to string value pair for each argument (if the arg is string, the key is just 'String').

I think there is room to optimize this with some move semantics but I leave that to a follow-on patch.

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

 // 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/Analysis/BlockFrequencyInfo.h"
+#include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Pass.h"
 
 namespace llvm {
 class DebugLoc;
 class Function;
 class LLVMContext;
 class Loop;
OptimizationRemarkEmitter(OptimizationRemarkEmitter &&Arg)
       : F(Arg.F), BFI(Arg.BFI) {}
 
   OptimizationRemarkEmitter &operator=(OptimizationRemarkEmitter &&RHS) {
     F = RHS.F;
     BFI = RHS.BFI;
     return *this;
   }
 
+  /// The new interface to emit remarks.
+  void emit(DiagnosticInfoOptimizationBase &OptDiag);
+
   /// Emit an optimization-applied message.
   ///
   /// \p PassName is the name of the pass emitting the message. If -Rpass= is
   /// given and \p PassName 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.

 private:
   Function *F;
 
   BlockFrequencyInfo *BFI;
 
   /// If we generate BFI on demand, we need to free it when ORE is freed.
   std::unique_ptr<BlockFrequencyInfo> OwnedBFI;
 
+  /// Compute hotness from IR value (currently assumed to be a block) if PGO is
+  /// available.
   Optional<uint64_t> computeHotness(const Value *V);
 
+  /// Similar but use value from \p OptDiag and update hotness there.
+  void computeHotness(DiagnosticInfoOptimizationBase &OptDiag);
+
   /// \brief Only allow verbose messages if we know we're filtering by hotness
   /// (BFI is only set in this case).
   bool shouldEmitVerbose() { return BFI != nullptr; }
 
   OptimizationRemarkEmitter(const OptimizationRemarkEmitter &) = delete;
   void operator=(const OptimizationRemarkEmitter &) = delete;
 };
 
+/// \brief Add a small namespace to avoid name clashes with the classes used in
+/// the streaming interface.  We want these to be short for better
+/// write/readability.
+namespace ore {
+using NV = DiagnosticInfoOptimizationBase::Argument;
+using setIsVerbose = DiagnosticInfoOptimizationBase::setIsVerbose;
+}
+
 /// OptimizationRemarkEmitter legacy analysis pass
 ///
 /// Note that this pass shouldn't generally be marked as preserved by other
 /// passes.  It's holding onto BFI, so if the pass does not preserve BFI, BFI
 /// could be freed.
 class OptimizationRemarkEmitterWrapperPass : public FunctionPass {
   std::unique_ptr<OptimizationRemarkEmitter> ORE;
 

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

 // Diagnostics reporting is still done as part of the LLVMContext.
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_IR_DIAGNOSTICINFO_H
 #define LLVM_IR_DIAGNOSTICINFO_H
 
 #include "llvm-c/Types.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/Support/CBindingWrapping.h"
+#include "llvm/Support/YAMLTraits.h"
 #include <functional>
 #include <string>
 
 namespace llvm {
 
 // Forward declarations.
 class DiagnosticPrinter;
 class Function;
private:
 
   /// Debug location where this diagnostic is triggered.
   DebugLoc DLoc;
 };
 
 /// Common features for diagnostics dealing with optimization remarks.
 class DiagnosticInfoOptimizationBase : public DiagnosticInfoWithDebugLocBase {
 public:
+  /// \brief Used to set IsVerbose via the stream interface.
+  struct setIsVerbose {};
+
+  /// \brief Used in the streaming interface as the general argument type.  It
+  /// internally converts everything into a key-value pair.
+  struct Argument {
+    StringRef Key;
+    std::string Val;
+
+    explicit Argument(StringRef Str = "") : Key("String"), Val(Str) {}
+    explicit Argument(StringRef Key, Value *V) : Key(Key), Val(V->getName()) {}
+    explicit Argument(StringRef Key, int N)
+        : Key(Key), Val(std::to_string(N)) {}
+  };
+
+  /// \p PassName is the name of the pass emitting this diagnostic. \p
+  /// RemarkName is a textual identifier for the remark.  \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 CodeRegion is IR
+  /// value (currently basic block) that the optimization operates on.  This is
+  /// currently used to provide run-time hotness information with PGO.
+  DiagnosticInfoOptimizationBase(enum DiagnosticKind Kind,
+                                 enum DiagnosticSeverity Severity,
+                                 const char *PassName, StringRef RemarkName,
+                                 const Function &Fn, const DebugLoc &DLoc,
+                                 Value *CodeRegion = nullptr)
+      : DiagnosticInfoWithDebugLocBase(Kind, Severity, Fn, DLoc),
+        PassName(PassName), RemarkName(RemarkName), CodeRegion(CodeRegion),
+        IsVerbose(false) {}
+
+  /// Legacy interface.
   /// \p PassName is the name of the pass emitting this diagnostic.
   /// \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.
   DiagnosticInfoOptimizationBase(enum DiagnosticKind Kind,
                                  enum DiagnosticSeverity Severity,
                                  const char *PassName, const Function &Fn,
                                  const DebugLoc &DLoc, const Twine &Msg,
                                  Optional<uint64_t> Hotness = None)
       : DiagnosticInfoWithDebugLocBase(Kind, Severity, Fn, DLoc),
-        PassName(PassName), Msg(Msg), Hotness(Hotness) {}
+        PassName(PassName), Hotness(Hotness), IsVerbose(false) {
+    Args.push_back(Argument(Msg.str()));
+  }
+
+  DiagnosticInfoOptimizationBase &operator<<(StringRef S);
+  DiagnosticInfoOptimizationBase &operator<<(Argument A);
+  DiagnosticInfoOptimizationBase &operator<<(setIsVerbose V);
 
   /// \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
   /// in BackendConsumer::OptimizationRemarkHandler).
   virtual bool isEnabled() const = 0;
 
   const char *getPassName() const { return PassName; }
-  const Twine &getMsg() const { return Msg; }
+  std::string getMsg() const;
   Optional<uint64_t> getHotness() const { return Hotness; }
+  void setHotness(Optional<uint64_t> H) { Hotness = H; }
+
+  Value *getCodeRegion() const { return CodeRegion; }
+
+  bool isVerbose() const { return IsVerbose; }
 
   static bool classof(const DiagnosticInfo *DI) {
     return DI->getKind() >= DK_FirstRemark &&
            DI->getKind() <= DK_LastRemark;
   }
 
 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;
+  /// Textual identifier for the remark.  Can be used by external tools reading
+  /// the YAML output file for optimization remarks to identify the remark.
+  StringRef RemarkName;
 
   /// 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;
+
+  /// The IR value (currently basic block) that the optimization operates on.
+  /// This is currently used to provide run-time hotness information with PGO.
+  Value *CodeRegion;
+
+  /// Arguments collected via the streaming interface.
+  SmallVector<Argument, 4> Args;
+
+  /// The remark is expected to be noisy.
+  bool IsVerbose;
+
+  friend struct yaml::MappingTraits<DiagnosticInfoOptimizationBase *>;
 };
 
 /// 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:
   /// must be valid for the whole life time of the diagnostic.
   DiagnosticInfoOptimizationRemarkMissed(const char *PassName,
                                          const Function &Fn,
                                          const DebugLoc &DLoc, const Twine &Msg,
                                          Optional<uint64_t> Hotness = None)
       : DiagnosticInfoOptimizationBase(DK_OptimizationRemarkMissed, DS_Remark,
                                        PassName, Fn, DLoc, Msg, Hotness) {}
 
+  /// \p PassName is the name of the pass emitting this diagnostic. If this name
+  /// matches the regular expression given in -Rpass-missed=, then the
+  /// diagnostic will be emitted.  \p RemarkName is a textual identifier for the
+  /// remark.  \p Inst is the instruction that the optimization operates on.
+  DiagnosticInfoOptimizationRemarkMissed(const char *PassName,
+                                         StringRef RemarkName,
+                                         Instruction *Inst);
+
   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

 class DICompositeType;
 class SMDiagnostic;
 class DiagnosticInfo;
 enum DiagnosticSeverity : char;
 template <typename T> class SmallVectorImpl;
 class Function;
 class DebugLoc;
 class OptBisect;
+namespace yaml {
+class Output;
+}
 
 /// This is an important class for using LLVM in a threaded context.  It
 /// (opaquely) owns and manages the core "global" data of LLVM's core
 /// infrastructure, including the type and constant uniquing tables.
 /// LLVMContext itself provides no locking guarantees, so you should be careful
 /// to have one context per thread.
 class LLVMContext {
 public:
public:
 
   /// \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 Return the YAML file used by the backend to save optimization
+  /// diagnostics.  If null, diagnostics are not saved in a file but only
+  /// emitted via the diagnostic handler.
+  yaml::Output *getDiagnosticsOutputFile();
+  /// Set the diagnostics output file used for optimization diagnostics.
+  ///
+  /// By default or if invoked with null, diagnostics are not saved in a file
+  /// but only emitted via the diagnostic handler.  Even if an output file is
+  /// set, the handler is invoked for each diagnostic message.
+  void setDiagnosticsOutputFile(yaml::Output *F);
+
   /// \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/lib/Analysis/OptimizationDiagnosticInfo.cpp

 // 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/BranchProbabilityInfo.h"
 #include "llvm/Analysis/LazyBlockFrequencyInfo.h"
 #include "llvm/Analysis/LoopInfo.h"
+#include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/LLVMContext.h"
 
 using namespace llvm;
 
 OptimizationRemarkEmitter::OptimizationRemarkEmitter(Function *F)
     : F(F), BFI(nullptr) {

 
 Optional<uint64_t> OptimizationRemarkEmitter::computeHotness(const Value *V) {
   if (!BFI)
     return None;
 
   return BFI->getBlockProfileCount(cast<BasicBlock>(V));
 }
 
+template <> struct yaml::MappingTraits<DiagnosticInfoOptimizationBase *> {
+  static void mapping(IO &io, DiagnosticInfoOptimizationBase *&OptDiag) {
+    assert(io.outputting() && "input not yet implemented");
+
+    if (io.mapTag("!Missed", OptDiag->getKind() == DK_OptimizationRemarkMissed))
+      ;
+    else
+      llvm_unreachable("todo");
+
+    // These are read-only for now.
+    DebugLoc DL = OptDiag->getDebugLoc();
+    StringRef FN = OptDiag->getFunction().getName();
+
+    StringRef PassName(OptDiag->PassName);
+    io.mapRequired("Pass", PassName);
+    io.mapRequired("Name", OptDiag->RemarkName);
+    if (!io.outputting() || DL)
+      io.mapOptional("DebugLoc", DL);
+    io.mapRequired("Function", FN);
+    io.mapOptional("Hotness", OptDiag->Hotness);
+    io.mapOptional("Args", OptDiag->Args);
+  }
+};
+
+template <> struct yaml::MappingTraits<DebugLoc> {
+  static void mapping(IO &io, DebugLoc &DL) {
+    assert(io.outputting() && "input not yet implemented");
+
+    auto *Scope = cast<DIScope>(DL.getScope());
+    StringRef File = Scope->getFilename();
+    unsigned Line = DL.getLine();
+    unsigned Col = DL.getCol();
+
+    io.mapRequired("File", File);
+    io.mapRequired("Line", Line);
+    io.mapRequired("Column", Col);
+  }
+
+  static const bool flow = true;
+};
+
+template <>
+struct yaml::ScalarTraits<DiagnosticInfoOptimizationBase::Argument> {
+  static void output(const DiagnosticInfoOptimizationBase::Argument &Arg,
+                     void *, llvm::raw_ostream &out) {
+    out << Arg.Key << ": " << Arg.Val;
+  }
+
+  static StringRef input(StringRef scalar, void *,
+                         DiagnosticInfoOptimizationBase::Argument &Arg) {
+    llvm_unreachable("input not yet implemented");
+  }
+
+  static bool mustQuote(StringRef) { return false; }
+};
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(DiagnosticInfoOptimizationBase::Argument)
+
+void OptimizationRemarkEmitter::computeHotness(
+    DiagnosticInfoOptimizationBase &OptDiag) {
+  Value *V = OptDiag.getCodeRegion();
+  if (V)
+    OptDiag.setHotness(computeHotness(V));
+}
+
+void OptimizationRemarkEmitter::emit(DiagnosticInfoOptimizationBase &OptDiag) {
+  computeHotness(OptDiag);
+
+  yaml::Output *Out = F->getContext().getDiagnosticsOutputFile();
+  if (Out && OptDiag.isEnabled()) {
+    auto *P = &const_cast<DiagnosticInfoOptimizationBase &>(OptDiag);
+    *Out << P;
+  }
+  // FIXME: now that IsVerbose is part of DI, filtering for this will be moved
+  // from here to clang.
+  if (!OptDiag.isVerbose() || shouldEmitVerbose())
+    F->getContext().diagnose(OptDiag);
+}
+
 void OptimizationRemarkEmitter::emitOptimizationRemark(const char *PassName,
                                                        const DebugLoc &DLoc,
                                                        const Value *V,
                                                        const Twine &Msg) {
   LLVMContext &Ctx = F->getContext();
   Ctx.diagnose(DiagnosticInfoOptimizationRemark(PassName, *F, DLoc, Msg,
                                                 computeHotness(V)));
 }

llvm/trunk/lib/IR/DiagnosticInfo.cpp

if (Hotness)
     DP << " (hotness: " << *Hotness << ")";
 }
 
 bool DiagnosticInfoOptimizationRemark::isEnabled() const {
   return PassRemarksOptLoc.Pattern &&
          PassRemarksOptLoc.Pattern->match(getPassName());
 }
 
+DiagnosticInfoOptimizationRemarkMissed::DiagnosticInfoOptimizationRemarkMissed(
+    const char *PassName, StringRef RemarkName, Instruction *Inst)
+    : DiagnosticInfoOptimizationBase(DK_OptimizationRemarkMissed, DS_Remark,
+                                     PassName, RemarkName,
+                                     *Inst->getParent()->getParent(),
+                                     Inst->getDebugLoc(), Inst->getParent()) {}
+
 bool DiagnosticInfoOptimizationRemarkMissed::isEnabled() const {
   return PassRemarksMissedOptLoc.Pattern &&
          PassRemarksMissedOptLoc.Pattern->match(getPassName());
 }
 
 bool DiagnosticInfoOptimizationRemarkAnalysis::isEnabled() const {
   return shouldAlwaysPrint() ||
          (PassRemarksAnalysisOptLoc.Pattern &&
void llvm::emitLoopInterleaveWarning(LLVMContext &Ctx, const Function &Fn,
                                      const DebugLoc &DLoc, const Twine &Msg) {
   Ctx.diagnose(DiagnosticInfoOptimizationFailure(
       Fn, DLoc, Twine("loop not interleaved: " + Msg)));
 }
 
 void DiagnosticInfoISelFallback::print(DiagnosticPrinter &DP) const {
   DP << "Instruction selection used fallback path for " << getFunction();
 }
+
+DiagnosticInfoOptimizationBase &DiagnosticInfoOptimizationBase::
+operator<<(StringRef S) {
+  Args.emplace_back(S);
+  return *this;
+}
+
+DiagnosticInfoOptimizationBase &DiagnosticInfoOptimizationBase::
+operator<<(Argument A) {
+  Args.push_back(std::move(A));
+  return *this;
+}
+
+DiagnosticInfoOptimizationBase &DiagnosticInfoOptimizationBase::
+operator<<(setIsVerbose V) {
+  IsVerbose = true;
+  return *this;
+}
+
+std::string DiagnosticInfoOptimizationBase::getMsg() const {
+  std::string Str;
+  raw_string_ostream OS(Str);
+  for (const DiagnosticInfoOptimizationBase::Argument &Arg : Args)
+    OS << Arg.Val;
+  return OS.str();
+}

llvm/trunk/lib/IR/LLVMContext.cpp

 
 void LLVMContext::setDiagnosticHotnessRequested(bool Requested) {
   pImpl->DiagnosticHotnessRequested = Requested;
 }
 bool LLVMContext::getDiagnosticHotnessRequested() const {
   return pImpl->DiagnosticHotnessRequested;
 }
 
+yaml::Output *LLVMContext::getDiagnosticsOutputFile() {
+  return pImpl->DiagnosticsOutputFile.get();
+}
+
+void LLVMContext::setDiagnosticsOutputFile(yaml::Output *F) {
+  pImpl->DiagnosticsOutputFile.reset(F);
+}
+
 LLVMContext::DiagnosticHandlerTy LLVMContext::getDiagnosticHandler() const {
   return pImpl->DiagnosticHandler;
 }
 
 void *LLVMContext::getDiagnosticContext() const {
   return pImpl->DiagnosticContext;
 }
 

llvm/trunk/lib/IR/LLVMContextImpl.h

 #include "llvm/ADT/StringMap.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Support/Dwarf.h"
+#include "llvm/Support/YAMLTraits.h"
 #include <vector>
 
 namespace llvm {
 
 class ConstantInt;
 class ConstantFP;
 class DiagnosticInfoOptimizationRemark;
 class DiagnosticInfoOptimizationRemarkMissed;
public:
   
   LLVMContext::InlineAsmDiagHandlerTy InlineAsmDiagHandler;
   void *InlineAsmDiagContext;
 
   LLVMContext::DiagnosticHandlerTy DiagnosticHandler;
   void *DiagnosticContext;
   bool RespectDiagnosticFilters;
   bool DiagnosticHotnessRequested;
+  std::unique_ptr<yaml::Output> DiagnosticsOutputFile;
 
   LLVMContext::YieldCallbackTy YieldCallback;
   void *YieldOpaqueHandle;
 
   typedef DenseMap<APInt, ConstantInt *, DenseMapAPIntKeyInfo> IntMapTy;
   IntMapTy IntConstants;
 
   typedef DenseMap<APFloat, ConstantFP *, DenseMapAPFloatKeyInfo> FPMapTy;

llvm/trunk/lib/Transforms/IPO/Inliner.cpp

for (BasicBlock &BB : *F)
         if (!CS || isa<IntrinsicInst>(I))
           continue;
 
         // If this is a direct call to an external function, we can never inline
         // it.  If it is an indirect call, inlining may resolve it to be a
         // direct call, so we keep it.
         if (Function *Callee = CS.getCalledFunction())
           if (Callee->isDeclaration()) {
-            ORE.emitOptimizationRemarkMissedAndAnalysis(
-                DEBUG_TYPE, &I,
-                Twine(Callee->getName()) + " will not be inlined into " +
-                    CS.getCaller()->getName(),
-                Twine("definition of ") + Callee->getName() +
+            ORE.emitOptimizationRemarkAnalysis(
+                DEBUG_TYPE, &I, Twine("definition of ") + Callee->getName() +
                                     " is not available",
                 /*Verbose=*/true);
+            using namespace ore;
+            ORE.emit(DiagnosticInfoOptimizationRemarkMissed(DEBUG_TYPE,
+                                                            "NotInlined", &I)
+                     << NV("Callee", Callee) << " will not be inlined into "
+                     << NV("Caller", CS.getCaller()) << setIsVerbose());
             continue;
           }
 
         CallSites.push_back(std::make_pair(CS, -1));
       }
   }
 
   DEBUG(dbgs() << ": " << CallSites.size() << " call sites.\n");

llvm/trunk/test/Transforms/Inline/optimization-remarks-yaml.ll

+; RUN: opt < %s -inline -pass-remarks-missed=inline -pass-remarks-with-hotness \
+; RUN:     -pass-remarks-output=%t 2>&1 | FileCheck %s
+; RUN: cat %t | FileCheck -check-prefix=YAML %s
+
+; Check the YAML file generated for inliner remarks for this program:
+;
+;   1  int foo();
+;   2  int bar();
+;   3
+;   4  int baz() {
+;   5    return foo() + bar();
+;   6  }
+
+; CHECK:      remark: /tmp/s.c:5:10: foo will not be inlined into baz (hotness: 30)
+; CHECK-NEXT: remark: /tmp/s.c:5:18: bar will not be inlined into baz (hotness: 30)
+
+; YAML:      --- !Missed
+; YAML-NEXT: Pass:            inline
+; YAML-NEXT: Name:            NotInlined
+; YAML-NEXT: DebugLoc:        { File: /tmp/s.c, Line: 5, Column: 10 }
+; YAML-NEXT: Function:        baz
+; YAML-NEXT: Hotness:         30
+; YAML-NEXT: Args:
+; YAML-NEXT:   - Callee: foo
+; YAML-NEXT:   - String:  will not be inlined into
+; YAML-NEXT:   - Caller: baz
+; YAML-NEXT: ...
+; YAML-NEXT: --- !Missed
+; YAML-NEXT: Pass:            inline
+; YAML-NEXT: Name:            NotInlined
+; YAML-NEXT: DebugLoc:        { File: /tmp/s.c, Line: 5, Column: 18 }
+; YAML-NEXT: Function:        baz
+; YAML-NEXT: Hotness:         30
+; YAML-NEXT: Args:
+; YAML-NEXT:   - Callee: bar
+; YAML-NEXT:   - String:  will not be inlined into
+; YAML-NEXT:   - Caller: baz
+; YAML-NEXT: ...
+
+; ModuleID = '/tmp/s.c'
+source_filename = "/tmp/s.c"
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+; Function Attrs: nounwind ssp uwtable
+define i32 @baz() !dbg !7 !prof !14 {
+entry:
+  %call = call i32 (...) @foo(), !dbg !9
+  %call1 = call i32 (...) @bar(), !dbg !10
+  %add = add nsw i32 %call, %call1, !dbg !12
+  ret i32 %add, !dbg !13
+}
+
+declare i32 @foo(...)
+
+declare i32 @bar(...)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 4.0.0 (trunk 281293) (llvm/trunk 281290)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
+!1 = !DIFile(filename: "/tmp/s.c", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"PIC Level", i32 2}
+!6 = !{!"clang version 4.0.0 (trunk 281293) (llvm/trunk 281290)"}
+!7 = distinct !DISubprogram(name: "baz", scope: !1, file: !1, line: 4, type: !8, isLocal: false, isDefinition: true, scopeLine: 4, isOptimized: true, unit: !0, variables: !2)
+!8 = !DISubroutineType(types: !2)
+!9 = !DILocation(line: 5, column: 10, scope: !7)
+!10 = !DILocation(line: 5, column: 18, scope: !11)
+!11 = !DILexicalBlockFile(scope: !7, file: !1, discriminator: 1)
+!12 = !DILocation(line: 5, column: 16, scope: !7)
+!13 = !DILocation(line: 5, column: 3, scope: !7)
+!14 = !{!"function_entry_count", i64 30}

llvm/trunk/tools/opt/opt.cpp

 #include "llvm/Support/PluginLoader.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/SystemUtils.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/YAMLTraits.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/Coroutines.h"
 #include "llvm/Transforms/IPO/AlwaysInliner.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
 #include "llvm/Transforms/Utils/Cloning.h"
 #include <algorithm>
 #include <memory>
 using namespace llvm;
static cl::opt<bool> Coroutines(
   cl::desc("Enable coroutine passes."),
   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 cl::opt<std::string>
+    RemarksFilename("pass-remarks-output",
+                    cl::desc("YAML output filename for pass remarks"),
+                    cl::value_desc("filename"));
+
 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
 
   Context.setDiscardValueNames(DiscardValueNames);
   if (!DisableDITypeMap)
     Context.enableDebugTypeODRUniquing();
 
   if (PassRemarksWithHotness)
     Context.setDiagnosticHotnessRequested(true);
 
+  std::unique_ptr<tool_output_file> YamlFile;
+  if (RemarksFilename != "") {
+    std::error_code EC;
+    YamlFile = llvm::make_unique<tool_output_file>(RemarksFilename, EC,
+                                                   sys::fs::F_None);
+    if (EC) {
+      errs() << EC.message() << '\n';
+      return 1;
+    }
+    Context.setDiagnosticsOutputFile(new yaml::Output(YamlFile->os()));
+  }
+
   // Load the input module...
   std::unique_ptr<Module> M = parseIRFile(InputFilename, Err, Context);
 
   if (!M) {
     Err.print(argv[0], errs());
     return 1;
   }
 
if (Buffer.size() != CompileTwiceBuffer.size() ||
         (memcmp(Buffer.data(), CompileTwiceBuffer.data(), Buffer.size()) !=
          0)) {
       errs() << "Running the pass manager twice changed the output.\n"
                 "Writing the result of the second run to the specified output.\n"
                 "To generate the one-run comparison binary, just run without\n"
                 "the compile-twice option\n";
       Out->os() << BOS->str();
       Out->keep();
+      if (YamlFile)
+        YamlFile->keep();
       return 1;
     }
     Out->os() << BOS->str();
   }
 
   // Declare success.
   if (!NoOutput || PrintBreakpoints)
     Out->keep();
 
+  if (YamlFile)
+    YamlFile->keep();
+
   return 0;
 }