[PATCH] D36440: [ThinLTO] Fix thinLTO crash

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: test/Transforms/FunctionImport/funcimport_var.ll:13
+; Function Attrs: nounwind
+declare i32 @link(i8*, i8*) local_unnamed_addr 
----------------
davidxl wrote:
> mehdi_amini wrote:
> > Something seems strange to me: the static variable GUID shouldn't be the same as the one here: the static variable name should be prepended with the module name when computing its GUID, shouldn't it?
> That is what I thought too, but it is not implemented this way -- see computeVariableSummary and computeFunctionSummary, where summaries are added to the index with the original names.
The GVs get added to the per-module index (when we compute the summaries in the compile step) with the original name. But when the combined index is built during the thin link (actually when we read in the summaries in the bitcode reader) we add with a GUID that prepends the module path for statics.

So for the attached example, I would expect that funcimport_var.ll will end up in the index with a call to link which uses a GUID that is the MD5 hash of "link", whereas funcimport_var2.ll will have a definition for link which has the GUID that is the MD5 hash of something like "funcimport_var2.ll:link". Again in the combined index not the individual summaries in the .bc files

David, can you use the -print-summary-global-ids option to print the GUIDs when we read in the summaries during the thinlink action? And/or -debug to trace the importing to figure out why we pick that GUID which should be different.


https://reviews.llvm.org/D36440