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

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

In http://reviews.llvm.org/D18908#395912, @joker.eph wrote:

> (I'm not sure what you're asking about let me know if this does not answer)
>
> Previously we loaded the IR in memory and iterated through the Module:
>
> - looking at linkage on the GlovalValue, deciding if we need to change it.
> - changing it immediately
>
>   Now the first phase is performed purely on the summary, without loading the IR.


Got it. I looked at the summary but somehow not the title, was in a hurry. =)


================
Comment at: include/llvm/Transforms/IPO/FunctionImport.h:63
@@ -61,1 +62,3 @@
 ///
+/// \p DefinedFunctions contains for each Module a map from (GUID) to Summary
+/// for every global defined in the module.
----------------
(GUID -> Summary) ?

================
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,
----------------
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.

================
Comment at: include/llvm/Transforms/IPO/FunctionImport.h:76
@@ -71,1 +75,3 @@
+                              const StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
+                              &DefinedFunctions,
     StringMap<FunctionImporter::ImportMapTy> &ImportLists,
----------------
Maybe name this map consistently throughout? How about ModuleToDefinedGVSummaries?

================
Comment at: lib/IR/ModuleSummaryIndex.cpp:81
@@ +80,3 @@
+      if (isa<GlobalVarSummary>(Summary))
+        /// Ignore global variable, focus on functions
+        continue;
----------------
Why? This seems like a regression in functionality from current HEAD

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


http://reviews.llvm.org/D18908