[PATCH] D36311: [ThinLTO] Add GraphTraits for FunctionSummaries

[Davide Italiano via Phabricator via llvm-commits]

davide added inline comments.


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:110
+inline bool operator<(const ValueInfo &A, const ValueInfo &B) {
+  assert(A.Ref && B.Ref);
+  return A.getGUID() < B.getGUID();
----------------
Add a message.


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:436-444
+            GlobalValue::LinkageTypes::AvailableExternallyLinkage, true, false),
+        0, FunctionSummary::FFlags{}, std::vector<ValueInfo>(),
+        std::vector<FunctionSummary::EdgeTy>(),
+        std::vector<GlobalValue::GUID>(),
+        std::vector<FunctionSummary::VFuncId>(),
+        std::vector<FunctionSummary::VFuncId>(),
+        std::vector<FunctionSummary::ConstVCall>(),
----------------
This is not really readable. I wonder if you can make it shorter.


================
Comment at: include/llvm/IR/ModuleSummaryIndex.h:646
+      for (auto &C :
+           cast<FunctionSummary>(V.getSummaryList().front().get())->calls()) {
+        bool notDiscovered =
----------------
`dyn_cast<>` + `assert != nullptr` ?


================
Comment at: lib/IR/ModuleSummaryIndex.cpp:74
+
+void ModuleSummaryIndex::dumpSCCs(raw_ostream &O) {
+  for (scc_iterator<ModuleSummaryIndex *> I =
----------------
Shouldn't this be under `NDEBUG || LLVM_ENABLE_DUMP` (or whatever it's called?)


https://reviews.llvm.org/D36311