[PATCH] D18908: ThinLTO: Move the ODR resolution to be based purely on the summary.

[Mehdi AMINI via llvm-commits]

joker.eph added inline comments.

================
Comment at: include/llvm/Transforms/IPO/FunctionImport.h:73
@@ -68,2 +72,3 @@
 /// is the set of globals that need to be promoted/renamed appropriately.
 void ComputeCrossModuleImport(
+                              const ModuleSummaryIndex &Index,
----------------
tejohnson wrote:
> Looks like this new line is unnecessary (the next line's parameter appears to start at the column after "(", but maybe my eyes are deceiving me.
I changed it late and didn't clang-format it

================
Comment at: lib/IR/ModuleSummaryIndex.cpp:81
@@ +80,3 @@
+      if (isa<GlobalVarSummary>(Summary))
+        /// Ignore global variable, focus on functions
+        continue;
----------------
tejohnson wrote:
> Why? This seems like a regression in functionality from current HEAD
This should be almost "code motion", please look at the changes in FunctionImport.cpp in this patch, the comment comes from there.

================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:191
@@ +190,3 @@
+/// Fixup linkage, see ResolveODR() above.
+void fixupODR(
+    Module &TheModule,
----------------
tejohnson wrote:
> Just a note for future cleanup - this file uses inconsistent function capitalization.
OK, will fix in a separate patch


http://reviews.llvm.org/D18908