[PATCH] D18298: ThinLTO: do not promote GlobalVariable that have a specific section.

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 25 10:19:03 PDT 2016


joker.eph added inline comments.

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:99
@@ +98,3 @@
+  auto Summary = DefinedGVSummaries.find(GUID);
+  if (Summary == DefinedGVSummaries.end())
+    return true;
----------------
tejohnson wrote:
> DefinedGVSummaries only contains entries for the module we are importing into, not from. So I don't think this is the right check. In fact, won't this cause this routine to always return true for anything not already in this module and therefore exactly when we would be introducing a cross-module reference?
> 
> Looks like your new test is checking that we don't import a function that has a section, but needs to be extended to check that we don't import a function containing references to something that has a section.
You're right that the logic seems wrong.

But I'm not sure your are describing what the test is doing: it is checking that we don't import `reference_gv_with_section` which is a function that does not have a section attached, but reference an internal variable with a section.


http://reviews.llvm.org/D18298





More information about the llvm-commits mailing list