[ThinLTO] YAML traits for module summaries

[ncharlie]

Summary

[ThinLTO] Dump YAML for module summaries

Adds YAML traits for module summaries so dumps can include summary information for functions, global values, and aliases.

[ncharlie]

Oops - got rid of old llvm-dis code.

[pcc]

Can you split this into two patches (both with their own tests), one which adds the subcommand and the other which adds the new YAML output?

tools/llvm-dis/llvm-dis.cpp

155 ↗

(On Diff #102067)

This seems like an unrelated change.

[ncharlie]

include/llvm/IR/ModuleSummaryIndexYAML.h

277

Some of these traits are duplicated. I could create a second struct called SharedSummaryYaml or something to deduplicate these fields, if anyone thinks they're worth centralizing.

302

I couldn't figure out how to thread a pointer to the Index down to the code that needs it (i.e. structs that are nested deeper in and have their own MappingTraits), so I left it here for now. If anyone has recommendations for a better place to put this, please let me know.

[ncharlie]

In D34063#777137, @pcc wrote:

Can you split this into two patches (both with their own tests), one which adds the subcommand and the other which adds the new YAML output?

Sorry - the bit you commented on wasn't supposed to be there - that was old code I forgot to remove. Do you want the new YAML code and llvm-lto2 subcommand to be in separate patches?

[pcc]

In D34063#777149, @ncharlie wrote:

Do you want the new YAML code and llvm-lto2 subcommand to be in separate patches?

Yes, please.

[mehdi_amini]

include/llvm/IR/ModuleSummaryIndex.h

538

Never seen "Oid" used here before.

Also s/dissassembly/disassembly and s/stringified//

539

Do we want this on all the time? Is this a fundamental property of the Index?
It isn't clear to me if it isn't just a debugging tool that map to the IR when available right now?

654

Return a const ref, and don't name it find... but get...OrEmpty if you want this API contract.

[ncharlie]

Addressed some updates from the comments.

[ncharlie]

include/llvm/IR/ModuleSummaryIndex.h

539

How would you recommend making this an optional debugging option? Would a constructor argument work?

[mehdi_amini]

include/llvm/IR/ModuleSummaryIndex.h

539

Is this information in the index in the bitcode?
If not then it can be reconstructed at dump time.

[ncharlie]

include/llvm/IR/ModuleSummaryIndex.h

539

The reason I'm storing the map in the index is because I only have access to the ValueName when the bitcode is being parsed in ModuleSummaryIndexBitcodeReader (see BitcodeReader.cpp:4715 in this patch). Also, when the YAML is being generated, I'm only given a pointer to a ModuleSummaryIndex, which is why the map is in that class.

[tejohnson]

include/llvm/IR/ModuleSummaryIndex.h

539

The value name is available on the GlobalValue (Value::getName()), as is the GUID (getGUID()). So you can just walk the Module's list of global_objects() (which gives you all the functions and global variables) and the list of aliases(), and compute the mapping.

That being said, I am not familiar enough with the YAML support to know how to get an additional object (either the Module, or a mapping pre-computed by the client or a wrapper function), passed down to where you need it. pcc may have some insight.

[mehdi_amini]

include/llvm/IR/ModuleSummaryIndex.h

539

Also, when the YAML is being generated, I'm only given a pointer to a ModuleSummaryIndex, which is why the map is in that class.

Then just modify the YAML to get an optional Module * as well? If you get it then you have names available, otherwise not.
Since it wouldn't roundtrip the YAML, I'd display it as a comment next to the GUID.

[ncharlie]

include/llvm/IR/ModuleSummaryIndex.h

539

The value name is available on the GlobalValue (Value::getName()), as is the GUID (getGUID()). So you can just walk the Module's list of global_objects()

Then just modify the YAML to get an optional Module * as well?

OK, I'll look into to this. I didn't realize there was already a map that stored the names in the Module class, so now I really want to get rid of my duplicate.

[tejohnson]

include/llvm/IR/ModuleSummaryIndex.h

539

It isn't really a map, but rather the global values are available from the Module, and as I mentioned in my comment (just above Mehdi's), the names and guids are available on the global values.

[ncharlie]

include/llvm/IR/ModuleSummaryIndex.h

539

Then just modify the YAML to get an optional Module * as well?

After looking into this, I can see two options for doing this.

1. Create a struct that holds both the Module * and the ModuleSummaryIndex, and output the new struct. I went part way down this path today, and I definitely think it's possible to do this, but it might result in some slightly odd code in some places - especially when reading in YAML (since it may not always be entirely obvious why a ModuleSummaryIndex and Module * have to be tracked together).
2. Add a Module * or getModule function to the ModuleSummaryIndex - I think this option is the cleanest and simplest implementation, despite adding a bit more information to the ModuleSummaryIndex class.

If there's some way to get a Module from a ModuleID or ModuleHash, that could work as well, but I'm not sure if it's possible.

Let me know which one you guys think is the best.

[mehdi_amini]

include/llvm/IR/ModuleSummaryIndex.h

539

I'm not sure we're all on the same track.

Names are never part of the summaries IIRC. So when you only work to/from YAML you don't have any names. Which is why I mentioned that the names in the YAML can only be comments.

I don't expect any change to include/llvm/IR/ModuleSummaryIndex.h ; it should be fully handled in the routine that dumps to YAML: it should take an optional Module. The first it can do is build a map from GUID -> Name for every symbol in the module. The YAML dump takes the summary index and this map and process from here.

[ncharlie]

include/llvm/IR/ModuleSummaryIndex.h

539

Got it - I'll pull my changes from the ModuleSummaryIndex and only worry about the names at dump time.

[ncharlie]

Fixes according to comments.

[tejohnson]

Can you add some test cases?

include/llvm/IR/ModuleSummaryIndexYAML.h

277

Yes I think that would be good. How about GlobalValueSummaryYaml, to mirror the class structure of the summaries themselves.

377

Also need to iterate over the refs() here and for alias and global var summaries (refs() is in the GlobalValueSummary base class)

424

Should there be an assert here that this is never invoked then?

438

Needs clang format. I believe you will also need to iterate over the aliases doing the same thing (global objects only includes functions and globalvars).

[ncharlie]

In D34063#784939, @tejohnson wrote:

Can you add some test cases?

Sure. How should I be testing the output?

[tejohnson]

In D34063#785295, @ncharlie wrote:

In D34063#784939, @tejohnson wrote:

Can you add some test cases?

Sure. How should I be testing the output?

With the llvm-lto2 support you are adding in D34080 - it's already approved so can go in.

[ncharlie]

Added test, further cleanup.

[ncharlie]

formatting

[ncharlie]

include/llvm/IR/ModuleSummaryIndexYAML.h

424

I realized that mapping wasn't necessary so I removed it.

[mehdi_amini]

include/llvm/IR/ModuleSummaryIndexYAML.h

438

There is a helper for this: Module::global_values()

(also: prefer for-range loops, and variable name needs to be uppercase).

[ncharlie]

Updated GlobalValue loop.

[sfertile]

include/llvm/IR/ModuleSummaryIndexYAML.h

400

GVSum is unused, which will introduce a warning.

[tejohnson]

Looks really close, just a few minor suggestions/nits left.

include/llvm/IR/ModuleSummaryIndexYAML.h

400

You can use "isa" instead of "dyn_cast" here now

426

Minor nit: prefer no braces around single statement for and if bodies.

include/llvm/Support/YAMLTraits.h

657

Ditto on no braces preferred.

test/tools/llvm-lto2/X86/dump-summary-yaml.ll

15

For the lines that have names, can you check for the names as well? You could either put in the expected GUID value, or use a regex to accept any GUID #. The FileCheck regex format for matching anything is: {{.*}}

[ncharlie]

test/tools/llvm-lto2/X86/dump-summary-yaml.ll

15

Added the regex to match the GUIDs now.

I didn't check for names, because the current llvm-lto2 pass won't print them out, because it needs to pass in a special struct with that includes a Module pointer for the name. Since the llvm-lto2 dump summary patch doesn't support the struct yet (as it's in this patch), I didn't enable the names in the other patch yet. I'll need to create another patch once both these patches are merged in so llvm-lto2 prints the names in the summary.

[tejohnson]

test/tools/llvm-lto2/X86/dump-summary-yaml.ll

15

I think the other patch is approved, and can go in now. Then since you are adding the name mapping functionality in this patch, I would say go ahead and add the llvm-lto2 name dumping in this patch.

[ncharlie]

Added names to dump-summary in llvm-lto2.

[tejohnson]

Can you commit D34080 and update the patch so it only shows the differences relative to that?

[ncharlie]

Applied D34080

removed test/tools/llvm-lto2/X86/dump-summary.ll from D34080 since it was more of a temporary placeholder test before test/tools/llvm-lto2/X86/dump-summary-yaml.ll could be added.

include/llvm/IR/ModuleSummaryIndex.h

 #include <utility>
 #include <vector>
 
 namespace llvm {
 
 namespace yaml {
 
 template <typename T> struct MappingTraits;
+struct ModuleSummaryIndexWithModule;
 
 } // end namespace yaml
 
 /// \brief Class to accumulate and hold information about a callee.
 struct CalleeInfo {
   enum class HotnessType : uint8_t { Unknown = 0, Cold = 1, None = 2, Hot = 3 };
   HotnessType Hotness = HotnessType::Unknown;
 
private:
   /// Holds strings for combined index, mapping to the corresponding module ID.
   ModulePathStringTableTy ModulePathStringTable;
 
   /// Mapping from type identifiers to summary information for that type
   /// identifier.
   // FIXME: Add bitcode read/write support for this field.
   std::map<std::string, TypeIdSummary> TypeIdMap;
 
   /// Mapping from original ID to GUID. If original ID can map to multiple
   /// GUIDs, it will be mapped to 0.
   std::map<GlobalValue::GUID, GlobalValue::GUID> OidGuidMap;
 
   /// Indicates that summary-based GlobalValue GC has run, and values with
   /// GVFlags::Live==false are really dead. Otherwise, all values must be
   /// considered live.
   bool WithGlobalValueDeadStripping = false;
 
   std::set<std::string> CfiFunctionDefs;
   std::set<std::string> CfiFunctionDecls;
 
   // YAML I/O support.
   friend yaml::MappingTraits<ModuleSummaryIndex>;
+  friend yaml::MappingTraits<yaml::ModuleSummaryIndexWithModule>;
 
   GlobalValueSummaryMapTy::value_type *
   getOrInsertValuePtr(GlobalValue::GUID GUID) {
     return &*GlobalValueMap.emplace(GUID, GlobalValueSummaryInfo{}).first;
   }
 
 public:
   gvsummary_iterator begin() { return GlobalValueMap.begin(); }
auto Summary =
                       [&](const std::unique_ptr<GlobalValueSummary> &Summary) {
                         return Summary->modulePath() == ModuleId;
                       });
     if (Summary == CalleeInfo.getSummaryList().end())
       return nullptr;
     return Summary->get();
   }
 
   /// Returns the first GlobalValueSummary for \p GV, asserting that there
   /// is only one if \p PerModuleIndex.
   GlobalValueSummary *getGlobalValueSummary(const GlobalValue &GV,
                                             bool PerModuleIndex = true) const {
     assert(GV.hasName() && "Can't get GlobalValueSummary for GV with no name");
     return getGlobalValueSummary(GlobalValue::getGUID(GV.getName()),
                                  PerModuleIndex);
   }
 

include/llvm/IR/ModuleSummaryIndexYAML.h

 #define LLVM_IR_MODULESUMMARYINDEXYAML_H
 
 #include "llvm/IR/ModuleSummaryIndex.h"
 #include "llvm/Support/YAMLTraits.h"
 
 namespace llvm {
 namespace yaml {
 
+using NameMapTy = std::map<GlobalValue::GUID, StringRef>;
+
+template <typename T> struct NameMapWrapper {
+  T *Val;
+  NameMapTy *NameMap;
+
+  StringRef getNameOrEmpty(GlobalValue::GUID G) {
+    if (NameMap)
+      return (*NameMap)[G];
+    return StringRef("");
+  }
+};
+
 template <> struct ScalarEnumerationTraits<TypeTestResolution::Kind> {
   static void enumeration(IO &io, TypeTestResolution::Kind &value) {
     io.enumCase(value, "Unsat", TypeTestResolution::Unsat);
     io.enumCase(value, "ByteArray", TypeTestResolution::ByteArray);
     io.enumCase(value, "Inline", TypeTestResolution::Inline);
     io.enumCase(value, "Single", TypeTestResolution::Single);
     io.enumCase(value, "AllOnes", TypeTestResolution::AllOnes);
   }
 };
 
+template <> struct ScalarEnumerationTraits<CalleeInfo::HotnessType> {
+  static void enumeration(IO &io, CalleeInfo::HotnessType &value) {
+    io.enumCase(value, "Unknown", CalleeInfo::HotnessType::Unknown);
+    io.enumCase(value, "Cold", CalleeInfo::HotnessType::Cold);
+    io.enumCase(value, "None", CalleeInfo::HotnessType::None);
+    io.enumCase(value, "Hot", CalleeInfo::HotnessType::Hot);
+  }
+};
+
+template <> struct ScalarEnumerationTraits<GlobalValueSummary::SummaryKind> {
+  static void enumeration(IO &io, GlobalValueSummary::SummaryKind &value) {
+    io.enumCase(value, "Alias", GlobalValueSummary::SummaryKind::AliasKind);
+    io.enumCase(value, "Function",
+                GlobalValueSummary::SummaryKind::FunctionKind);
+    io.enumCase(value, "GlobalVar",
+                GlobalValueSummary::SummaryKind::GlobalVarKind);
+  }
+};
+
 template <> struct MappingTraits<TypeTestResolution> {
   static void mapping(IO &io, TypeTestResolution &res) {
     io.mapOptional("Kind", res.TheKind);
     io.mapOptional("SizeM1BitWidth", res.SizeM1BitWidth);
   }
 };
 
 template <>

 
 template <> struct MappingTraits<TypeIdSummary> {
   static void mapping(IO &io, TypeIdSummary& summary) {
     io.mapOptional("TTRes", summary.TTRes);
     io.mapOptional("WPDRes", summary.WPDRes);
   }
 };
 
-struct FunctionSummaryYaml {
-  unsigned Linkage;
+struct RefYaml {
+  StringRef Name;
+  GlobalValue::GUID GUID;
+};
+
+struct CalleeInfoYaml {
+  StringRef Name;
+  GlobalValue::GUID GUID;
+  CalleeInfo::HotnessType Hotness;
+};
+
+struct GlobalValueSummaryYaml {
+  StringRef Name;
+  GlobalValueSummary::SummaryKind Kind;
+  GlobalValue::LinkageTypes Linkage;
   bool NotEligibleToImport, Live;
+  std::vector<RefYaml> Refs;
+};
+
+struct FunctionSummaryYaml {
+  GlobalValueSummaryYaml GVSum;
+  unsigned InstCount;
+  std::vector<CalleeInfoYaml> Calls;
+
   std::vector<uint64_t> TypeTests;
   std::vector<FunctionSummary::VFuncId> TypeTestAssumeVCalls,
       TypeCheckedLoadVCalls;
   std::vector<FunctionSummary::ConstVCall> TypeTestAssumeConstVCalls,
       TypeCheckedLoadConstVCalls;
 };
 
+struct GlobalVarSummaryYaml {
+  GlobalValueSummaryYaml GVSum;
+};
+
+struct AliasSummaryYaml {
+  GlobalValueSummaryYaml GVSum;
+  GlobalValue::GUID Aliasee;
+  StringRef AliaseeName;
+};
+
 } // End yaml namespace
 } // End llvm namespace
 
 LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(uint64_t)
 
 namespace llvm {
 namespace yaml {
 

 } // End llvm namespace
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionSummary::VFuncId)
 LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionSummary::ConstVCall)
 
 namespace llvm {
 namespace yaml {
 
-template <> struct MappingTraits<FunctionSummaryYaml> {
-  static void mapping(IO &io, FunctionSummaryYaml& summary) {
-    io.mapOptional("Linkage", summary.Linkage);
-    io.mapOptional("NotEligibleToImport", summary.NotEligibleToImport);
-    io.mapOptional("Live", summary.Live);
+template <> struct MappingTraits<RefYaml> {
+  static void mapping(IO &io, RefYaml &summary) {
+    io.mapOptional("GUID", summary.GUID);
+    io.addComment(Comment(summary.Name, false));
+  }
+};
+
+template <> struct MappingTraits<CalleeInfoYaml> {
+  static void mapping(IO &io, CalleeInfoYaml &summary) {
+    io.mapOptional("GUID", summary.GUID);
+    io.addComment(Comment(summary.Name, false));
+    io.mapOptional("Hotness", summary.Hotness);
+  }
+};
+
+static void mapGlobalValueSummary(IO &io, GlobalValueSummaryYaml &summary) {
+  io.addComment(Comment(summary.Name));
+  io.mapOptional("Kind", summary.Kind);
+  io.mapOptional("Refs", summary.Refs);
+}
+
+template <> struct MappingTraits<FunctionSummaryYaml> {
+  static void mapping(IO &io, FunctionSummaryYaml& summary) {
+    mapGlobalValueSummary(io, summary.GVSum);
+    io.mapOptional("InstCount", summary.InstCount);
+    io.mapOptional("Calls", summary.Calls);
     io.mapOptional("TypeTests", summary.TypeTests);
     io.mapOptional("TypeTestAssumeVCalls", summary.TypeTestAssumeVCalls);
     io.mapOptional("TypeCheckedLoadVCalls", summary.TypeCheckedLoadVCalls);
     io.mapOptional("TypeTestAssumeConstVCalls",
                    summary.TypeTestAssumeConstVCalls);
     io.mapOptional("TypeCheckedLoadConstVCalls",
                    summary.TypeCheckedLoadConstVCalls);
   }
 };
 
+template <> struct MappingTraits<GlobalVarSummaryYaml> {
+  static void mapping(IO &io, GlobalVarSummaryYaml &summary) {
+    mapGlobalValueSummary(io, summary.GVSum);
+  }
+};
+
+template <> struct MappingTraits<AliasSummaryYaml> {
+  static void mapping(IO &io, AliasSummaryYaml &summary) {
+    mapGlobalValueSummary(io, summary.GVSum);
+    io.mapOptional("Aliasee", summary.Aliasee);
+    io.addComment(Comment{summary.AliaseeName, false});
+  }
+};
+
 } // End yaml namespace
 } // End llvm namespace
 
 LLVM_YAML_IS_STRING_MAP(TypeIdSummary)
 LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionSummaryYaml)
+LLVM_YAML_IS_SEQUENCE_VECTOR(CalleeInfoYaml)
+LLVM_YAML_IS_SEQUENCE_VECTOR(RefYaml)
+LLVM_YAML_IS_SEQUENCE_VECTOR(GlobalVarSummaryYaml)
+LLVM_YAML_IS_SEQUENCE_VECTOR(AliasSummaryYaml)
 LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(std::string)
 
 namespace llvm {
 namespace yaml {
 
+template <> struct ScalarEnumerationTraits<GlobalValue::LinkageTypes> {
+  static void enumeration(IO &io, GlobalValue::LinkageTypes &value) {
+    io.enumCase(value, "ExternalLinkage",
+                GlobalValue::LinkageTypes::ExternalLinkage);
+    io.enumCase(value, "AvailableExternallyLinkage",
+                GlobalValue::LinkageTypes::AvailableExternallyLinkage);
+    io.enumCase(value, "LinkOnceAnyLinkage",
+                GlobalValue::LinkageTypes::LinkOnceAnyLinkage);
+    io.enumCase(value, "LinkOnceODRLinkage",
+                GlobalValue::LinkageTypes::LinkOnceODRLinkage);
+    io.enumCase(value, "WeakAnyLinkage",
+                GlobalValue::LinkageTypes::WeakAnyLinkage);
+    io.enumCase(value, "WeakODRLinkage",
+                GlobalValue::LinkageTypes::WeakODRLinkage);
+    io.enumCase(value, "AppendingLinkage",
+                GlobalValue::LinkageTypes::AppendingLinkage);
+    io.enumCase(value, "InternalLinkage",
+                GlobalValue::LinkageTypes::InternalLinkage);
+    io.enumCase(value, "PrivateLinkage",
+                GlobalValue::LinkageTypes::PrivateLinkage);
+    io.enumCase(value, "ExternalWeakLinkage",
+                GlobalValue::LinkageTypes::ExternalWeakLinkage);
+    io.enumCase(value, "CommonLinkage",
+                GlobalValue::LinkageTypes::CommonLinkage);
+  }
+};
+
 // FIXME: Add YAML mappings for the rest of the module summary.
-template <> struct CustomMappingTraits<GlobalValueSummaryMapTy> {
-  static void inputOne(IO &io, StringRef Key, GlobalValueSummaryMapTy &V) {
+template <>
+struct CustomMappingTraits<NameMapWrapper<GlobalValueSummaryMapTy>> {
+  static void inputOne(IO &io, StringRef Key,
+                       NameMapWrapper<GlobalValueSummaryMapTy> &V) {
     std::vector<FunctionSummaryYaml> FSums;
+    std::vector<GlobalVarSummaryYaml> GVSums;
+    std::vector<AliasSummaryYaml> ASums;
     io.mapRequired(Key.str().c_str(), FSums);
-    uint64_t KeyInt;
-    if (Key.getAsInteger(0, KeyInt)) {
-      io.setError("key not an integer");
-      return;
-    }
-    auto &Elem = V[KeyInt];
+    io.mapRequired(Key.str().c_str(), GVSums);
+    io.mapRequired(Key.str().c_str(), ASums);
+    auto &Elem = (*V.Val)[std::stoi(Key)];
     for (auto &FSum : FSums) {
       Elem.SummaryList.push_back(llvm::make_unique<FunctionSummary>(
           GlobalValueSummary::GVFlags(
-              static_cast<GlobalValue::LinkageTypes>(FSum.Linkage),
-              FSum.NotEligibleToImport, FSum.Live),
+              static_cast<GlobalValue::LinkageTypes>(FSum.GVSum.Linkage),
+              FSum.GVSum.NotEligibleToImport, FSum.GVSum.Live),
           0, ArrayRef<ValueInfo>{}, ArrayRef<FunctionSummary::EdgeTy>{},
           std::move(FSum.TypeTests), std::move(FSum.TypeTestAssumeVCalls),
           std::move(FSum.TypeCheckedLoadVCalls),
           std::move(FSum.TypeTestAssumeConstVCalls),
           std::move(FSum.TypeCheckedLoadConstVCalls)));
     }
+
+    for (auto &GVSum : GVSums) {
+      Elem.SummaryList.push_back(llvm::make_unique<GlobalVarSummary>(
+          GlobalValueSummary::GVFlags(
+              static_cast<GlobalValue::LinkageTypes>(GVSum.GVSum.Linkage),
+              GVSum.GVSum.NotEligibleToImport, GVSum.GVSum.Live),
+          ArrayRef<ValueInfo>{}));
     }
-  static void output(IO &io, GlobalValueSummaryMapTy &V) {
-    for (auto &P : V) {
+
+    for (auto &ASum : ASums) {
+      Elem.SummaryList.push_back(llvm::make_unique<AliasSummary>(
+          GlobalValueSummary::GVFlags(
+              static_cast<GlobalValue::LinkageTypes>(ASum.GVSum.Linkage),
+              ASum.GVSum.NotEligibleToImport, ASum.GVSum.Live),
+          ArrayRef<ValueInfo>{}));
+    }
+  }
+
+  static void output(IO &io, NameMapWrapper<GlobalValueSummaryMapTy> &V) {
+    for (auto &P : *V.Val) {
       std::vector<FunctionSummaryYaml> FSums;
+      std::vector<GlobalVarSummaryYaml> GVSums;
+      std::vector<AliasSummaryYaml> ASums;
       for (auto &Sum : P.second.SummaryList) {
-        if (auto *FSum = dyn_cast<FunctionSummary>(Sum.get()))
+        std::vector<RefYaml> Refs;
+        for (auto i : Sum->refs()) {
+          Refs.push_back({V.getNameOrEmpty(i.getGUID()), i.getGUID()});
+        }
+        GlobalValueSummaryYaml GVSumY = {
+            V.getNameOrEmpty(P.first),
+            Sum->getSummaryKind(),
+            static_cast<GlobalValue::LinkageTypes>(Sum->flags().Linkage),
+            static_cast<bool>(Sum->flags().NotEligibleToImport),
+            static_cast<bool>(Sum->flags().Live),
+            Refs};
+
+        if (auto *FSum = dyn_cast<FunctionSummary>(Sum.get())) {
+          std::vector<CalleeInfoYaml> Calls;
+          for (auto edge : FSum->calls()) {
+            Calls.push_back(
+                CalleeInfoYaml{V.getNameOrEmpty(edge.first.getGUID()),
+                               edge.first.getGUID(), edge.second.Hotness});
+          }
           FSums.push_back(FunctionSummaryYaml{
-              FSum->flags().Linkage,
-              static_cast<bool>(FSum->flags().NotEligibleToImport),
-              static_cast<bool>(FSum->flags().Live), FSum->type_tests(),
+              GVSumY, FSum->instCount(), Calls, FSum->type_tests(),
               FSum->type_test_assume_vcalls(), FSum->type_checked_load_vcalls(),
               FSum->type_test_assume_const_vcalls(),
               FSum->type_checked_load_const_vcalls()});
         }
+        if (auto *GVSum = dyn_cast<GlobalVarSummary>(Sum.get()))
+          GVSums.push_back(GlobalVarSummaryYaml{GVSumY});
+        if (auto *ASum = dyn_cast<AliasSummary>(Sum.get()))
+          ASums.push_back(AliasSummaryYaml{
+              GVSumY, ASum->getAliasee().getOriginalName(),
+              V.getNameOrEmpty(ASum->getAliasee().getOriginalName())});
+      }
       if (!FSums.empty())
-        io.mapRequired(llvm::utostr(P.first).c_str(), FSums);
+        io.mapRequired(utostr(P.first).c_str(), FSums);
+      if (!GVSums.empty())
+        io.mapRequired(utostr(P.first).c_str(), GVSums);
+      if (!ASums.empty())
+        io.mapRequired(utostr(P.first).c_str(), ASums);
+    }
+  }
+};
+
+struct ModuleSummaryIndexWithModule {
+  ModuleSummaryIndex *Index;
+  Module *Mod;
+};
+
+template <> struct MappingTraits<ModuleSummaryIndexWithModule> {
+  static void mapping(IO &io, ModuleSummaryIndexWithModule &M) {
+    NameMapTy NameMap;
+    if (M.Mod) {
+      for (GlobalValue &V : M.Mod->global_values()) {
+        NameMap.insert(std::make_pair(V.getGUID(), V.getName()));
+      }
     }
+    NameMapWrapper<GlobalValueSummaryMapTy> N = {&M.Index->GlobalValueMap,
+                                                 &NameMap};
+    io.mapRequired("ModuleSummaryIndex", N);
   }
 };
 
 template <> struct MappingTraits<ModuleSummaryIndex> {
   static void mapping(IO &io, ModuleSummaryIndex& index) {
-    io.mapOptional("GlobalValueMap", index.GlobalValueMap);
+    NameMapWrapper<GlobalValueSummaryMapTy> M = {&index.GlobalValueMap,
+                                                 nullptr};
+    io.addComment(Comment{"Use output for debugging/testing purposes only"});
+    io.mapOptional("GlobalValueMap", M);
     io.mapOptional("TypeIdMap", index.TypeIdMap);
     io.mapOptional("WithGlobalValueDeadStripping",
                    index.WithGlobalValueDeadStripping);
 
     if (io.outputting()) {
       std::vector<std::string> CfiFunctionDefs(index.CfiFunctionDefs.begin(),
                                                index.CfiFunctionDefs.end());
       io.mapOptional("CfiFunctionDefs", CfiFunctionDefs);

include/llvm/Support/YAMLTraits.h

: public std::integral_constant<
                     has_MappingValidateTraits<T, Context>::value> {};
 
 template <typename T, typename Context>
 struct unvalidatedMappingTraits
     : public std::integral_constant<
           bool, has_MappingTraits<T, Context>::value &&
                     !has_MappingValidateTraits<T, Context>::value> {};
 
+struct Comment {
+  StringRef Str;
+  bool NewLine = true;
+
+  Comment(StringRef S) : Str(S){};
+  Comment(StringRef S, bool N) : Str(S), NewLine(N){};
+};
+
 // Base class for Input and Output.
 class IO {
 public:
   IO(void *Ctxt=nullptr);
   virtual ~IO();
 
   virtual bool outputting() = 0;
 
public:
   virtual bool matchEnumScalar(const char*, bool) = 0;
   virtual bool matchEnumFallback() = 0;
   virtual void endEnumScalar() = 0;
 
   virtual bool beginBitSetScalar(bool &) = 0;
   virtual bool bitSetMatch(const char*, bool) = 0;
   virtual void endBitSetScalar() = 0;
 
+  virtual void comment(StringRef &S, bool) = 0;
+
   virtual void scalarString(StringRef &, bool) = 0;
   virtual void blockScalarString(StringRef &) = 0;
 
   virtual void setError(const Twine &) = 0;
 
   template <typename T>
   void enumCase(T &Val, const char* Str, const T ConstVal) {
     if ( matchEnumScalar(Str, outputting() && Val == ConstVal) ) {
template <typename T> void mapRequired(const char *Key, T &Val) {
     EmptyContext Ctx;
     this->processKey(Key, Val, true, Ctx);
   }
   template <typename T, typename Context>
   void mapRequired(const char *Key, T &Val, Context &Ctx) {
     this->processKey(Key, Val, true, Ctx);
   }
 
+  void addComment(Comment C) {
+    if (outputting() && !C.Str.empty()) {
+      comment(C.Str, C.NewLine);
+    }
+  }
+
   template <typename T> void mapOptional(const char *Key, T &Val) {
     EmptyContext Ctx;
     mapOptionalWithContext(Key, Val, Ctx);
   }
 
   template <typename T>
   void mapOptional(const char *Key, T &Val, const T &Default) {
     EmptyContext Ctx;
private:
   void endFlowSequence() override;
   void beginEnumScalar() override;
   bool matchEnumScalar(const char*, bool) override;
   bool matchEnumFallback() override;
   void endEnumScalar() override;
   bool beginBitSetScalar(bool &) override;
   bool bitSetMatch(const char *, bool ) override;
   void endBitSetScalar() override;
+  void comment(StringRef &S, bool) override;
   void scalarString(StringRef &, bool) override;
   void blockScalarString(StringRef &) override;
   void setError(const Twine &message) override;
   bool canElideEmptySequence() override;
 
   class HNode {
     virtual void anchor();
 
public:
   void endFlowSequence() override;
   void beginEnumScalar() override;
   bool matchEnumScalar(const char*, bool) override;
   bool matchEnumFallback() override;
   void endEnumScalar() override;
   bool beginBitSetScalar(bool &) override;
   bool bitSetMatch(const char *, bool ) override;
   void endBitSetScalar() override;
+  void comment(StringRef &S, bool) override;
   void scalarString(StringRef &, bool) override;
   void blockScalarString(StringRef &) override;
   void setError(const Twine &message) override;
   bool canElideEmptySequence() override;
 
   // These are only used by operator<<. They could be private
   // if that templated operator could be made a friend.
   void beginDocuments();

lib/Support/YAMLTraits.cpp

for (unsigned i = 0; i < SQ->Entries.size(); ++i) {
       if (!BitValuesUsed[i]) {
         setError(SQ->Entries[i].get(), "unknown bit value");
         return;
       }
     }
   }
 }
 
+// ignore comments
+void Input::comment(StringRef &S, bool) {}
+
 void Input::scalarString(StringRef &S, bool) {
   if (ScalarHNode *SN = dyn_cast<ScalarHNode>(CurrentNode)) {
     S = SN->value();
   } else {
     setError(CurrentNode, "unexpected scalar");
   }
 }
 
bool Output::bitSetMatch(const char *Str, bool Matches) {
   }
   return false;
 }
 
 void Output::endBitSetScalar() {
   this->outputUpToEndOfLine(" ]");
 }
 
+void Output::comment(StringRef &S, bool NewLine) {
+  if (NewLine) {
+    this->outputNewLine();
+    unsigned Indent = StateStack.size();
+    for (unsigned i = 0; i < Indent; i++)
+      output(" ");
+  } else
+    this->output(" ");
+
+  if (S.empty()) {
+    this->outputUpToEndOfLine("#");
+  } else {
+    this->output("# ");
+    this->outputUpToEndOfLine(S);
+  }
+}
+
 void Output::scalarString(StringRef &S, bool MustQuote) {
   this->newLineCheck();
   if (S.empty()) {
     // Print '' for the empty string because leaving the field empty is not
     // allowed.
     this->outputUpToEndOfLine("''");
     return;
   }

test/tools/llvm-lto2/X86/dump-summary-yaml.ll

+; RUN: opt -module-summary %s -o %s.o
+; RUN: llvm-lto2 dump-summary %s.o | FileCheck %s
+; CHECK: ModuleSummaryIndex:
+; CHECK:     - Kind:            GlobalVar
+; CHECK:       Linkage:         ExternalLinkage
+; CHECK:       NotEligibleToImport: false
+; CHECK:       Live:            false
+; CHECK:     - Kind:            Function
+; CHECK:       Linkage:         ExternalLinkage
+; CHECK:       NotEligibleToImport: false
+; CHECK:       Live:            false
+; CHECK:       InstCount:       2
+; CHECK:       Calls:
+; CHECK:         - GUID:
+; CHECK:           Hotness:         Unknown
+; CHECK:     - Kind:            Alias
+; CHECK:       Linkage:         WeakAnyLinkage
+; CHECK:       NotEligibleToImport: false
+; CHECK:       Live:            false
+; CHECK:       Aliasee:
+; CHECK:     - Kind:            Function
+; CHECK:       Linkage:         ExternalLinkage
+; CHECK:       NotEligibleToImport: false
+; CHECK:       Live:            false
+; CHECK:       Refs:
+; CHECK:         - GUID:
+; CHECK:       InstCount:       2
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@X = constant i32 42, section "foo", align 4
+@a = weak alias i32, i32* @X
+
+define void  @f() {
+  %1 = load i32, i32* @a
+  ret void
+}
+
+define void @t() {
+  tail call void @b()
+  ret void
+}
+
+declare void @b()