[PATCH] D26467: [ThinLTO] Only promote exported locals as marked in index

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.


================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:553
+  auto GUIDPreservedSymbols =
+      computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple);
+
----------------
mehdi_amini wrote:
> Not clear why you init TMBuilder, while you only seem to use the Triple?
Hmm, I cloned this from ThinLTOCodeGenerator::internalize, perhaps it isn't needed in either place. I removed the init from here for now.


================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:561
+            ExportList->second.count(GUID)) ||
+           GUIDPreservedSymbols.count(GUID);
+  };
----------------
mehdi_amini wrote:
> Is it clang-formatted?
Yes. I modified the spacing on this line, re-ran clang-format, and got back the same thing.


================
Comment at: lib/Transforms/Utils/FunctionImportUtils.cpp:80
+    // it must be promoted, so unconditionally promote all values in the
+    // importing module.
+    return true;
----------------
mehdi_amini wrote:
> why can't we just promote what is needed?
See the comment just above this - when we are importing we may not have a summary in the distributed backend case (we only get the summaries for things we want to import as defs, not all references in the module). At the worst, though, we will simply promote some values that are never referenced after import, so it won't affect the resulting module. 


https://reviews.llvm.org/D26467