[PATCH] D20268: [wip] Resolution-based LTO API.

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: include/llvm/LTO/LTO.h:113
@@ +112,3 @@
+  /// getMaxTasks()-1, which is supplied to the callback via the Task parameter.
+  /// There are two exceptions to the general rule that a task represents an
+  /// entire pipeline:
----------------
Is the general rule then for ThinLTO? Otherwise, I think regular LTO with no parallel code gen has 1 task, right?

================
Comment at: include/llvm/LTO/LTO.h:138
@@ +137,3 @@
+  typedef std::function<LTONativeObjectStream(size_t Task)> AddStreamFn;
+
+  /// This type defines a file callback. The file callback passed to run() is
----------------
If we have mixed ThinLTO and LTO mode, would you have one instance of this class for the ThinLTO portion and one for the regular LTO portion, or is it trying to manage both? If the former, does it make sense to add derived classes to handle each, so that the class doesn't contain methods and members that only are used in the other type?

Also, I have done the refactoring of the ThinLTOCodeGeneration handling for ODR resolution and internalization to operate via the Index. For now, I have them still in the same ThinLTOCodeGeneration.cpp file, but need to move them out to somewhere that can be shared by the various linkers. This new file seems like a good place for those that are applicable to the ThinLTO thin link step (i.e. consume resolution info and update the index). If we can have a derived class here for ThinLTO (thin link) I would put those here. The others (consume the index and do the actual ODR linkage changes and internalization) happen in the ThinLTO backends, which seems better suited to Transforms/IPO, especially since we will have the distributed backends coming in via clang.


http://reviews.llvm.org/D20268