[PATCH] D54815: [ThinLTO] Add summary entries for index-based WPD

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: include/llvm/Bitcode/LLVMBitCodes.h:271
   FS_TYPE_ID = 21,
+  // Maps type identifier to summary information for that type identifier
+  // computed from type metadata: the valueid of each vtable definition
----------------
It is a little difficult to parse the comment here.  Is it just mapping from type id to global value id of the vtable?   Also what is 'vtable definition decorated with a type metadata?'


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:818
+/// metadata.
+using TypeIdOffsetGVPair = std::pair<uint64_t, ValueInfo>;
+/// List of vtable definitions decorated by the same type id metadata,
----------------
Perhaps add that this is a mapping from type (id) to vtable info?


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:821
+/// and their corresponding offsets in the type id metadata.
+using TypeIdGVInfo = std::vector<TypeIdOffsetGVPair>;
+
----------------
Why is it a one-to-many mapping?


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:841
+  /// analysis and consumed by thin link.
+  std::map<std::string, TypeIdGVInfo> TypeIdMetadataMap;
+
----------------
TypeIdMetadataMap -- the name sounds too generic.  Is it possible to rename it to carry more semantics?


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:1214
+  /// (if not present). This may be used when importing.
+  const TypeIdGVInfo *getTypeIdMetadataSummary(StringRef TypeId) const {
+    auto I = TypeIdMetadataMap.find(TypeId);
----------------
Returns an ArrayRef?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54815/new/

https://reviews.llvm.org/D54815