[PATCH] D11721: [ThinLTO] Data structures for holding ThinLTO function index/summary

[David Li via llvm-commits]

davidxl added a subscriber: davidxl.

================
Comment at: include/llvm/IR/ThinLTOInfo.h:47
@@ +46,3 @@
+  // Path of module containing function IR
+  StringRef ModulePath;
+  // Index of function within bitcode module block in its module.
----------------
This field 'ModulePath' belongs to ThinLTOFunctionSummaryIndex class (when used before combining).

================
Comment at: include/llvm/IR/ThinLTOInfo.h:51
@@ +50,3 @@
+  // Function summary information used to help make importing decisions.
+  ThinLTOFunctionSummary *FunctionSummary;
+  // Used during bitcode parsing and writing to hold the offset of the
----------------
Is there a 1-1 mapping from ThinLTOFunctionInfo to FunctionSummary? If yes, can ThinLTOFunctionInfo inherite from FunctionSummary?

================
Comment at: include/llvm/IR/ThinLTOInfo.h:163
@@ +162,3 @@
+  void addFunctionInfo(StringRef FuncName,
+                       ThinLTOFunctionInfo Info) {
+    if (FunctionMap.count(FuncName))
----------------
why passing by value?

================
Comment at: lib/IR/ThinLTOInfo.cpp:44
@@ +43,3 @@
+
+    if (ModPath.empty()) {
+      ModPath = Info.modulePath();
----------------
The ModPath should be read once from 'Other' outside the loop.

================
Comment at: lib/IR/ThinLTOInfo.cpp:56
@@ +55,3 @@
+      // the same name. The symbol table created for the compiled index
+      // file should contain the renamed symbols.
+      SmallString<256> NewName(FuncName);
----------------
Is it using the same naming scheme when local function is static promoted? If yes, it might be better to have a common helper function such as "globalizeThinLTOLocalName", or "getGlobalThinLTONameForLocal" ..


http://reviews.llvm.org/D11721