[PATCH] D29734: IR: Function summary extensions for whole-program devirtualization pass.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h:253
+  /// An "identifier" for a virtual function. This contains the type identifier
+  /// represented as a GUID and the offset from the address point to the virtual
+  /// function pointer.
----------------
`address point`? 

Beyond the typo, I have no idea reading this what is the address pointer referring to? Is it the address of the vtable?


================
Comment at: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h:266
+    std::vector<uint64_t> Args;
+  };
+
----------------
The plan being to decorate edges in general with constant arguments, there is potential for code-sharing here.


================
Comment at: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h:288
+  /// all constant integer arguments.
+  std::vector<ConstVCall> TypeTestAssumeConstVCalls, TypeCheckedLoadConstVCalls;
 
----------------
How much do we grow the size of index entries? It seems that we're adding 5 vectors while in the general case this feature is unused.
Can we get an auxiliary structure of some sort?


================
Comment at: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h:355
+  }
+  static inline FunctionSummary::VFuncId getTombstoneKey() {
+    return {0, uint64_t(-2)};
----------------
inline is the default here, isn't it?


================
Comment at: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp:4854
+  std::vector<FunctionSummary::ConstVCall> PendingTypeTestAssumeConstVCalls,
+      PendingTypeCheckedLoadConstVCalls;
 
----------------
Document.


Repository:
  rL LLVM

https://reviews.llvm.org/D29734