[PATCH] D42107: [ThinLTO] - Stop internalizing and drop non-prevailing symbols.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

LGTM with minor comment nit and missing change noted below.
Thanks - this looks cleaner using the enum!



================
Comment at: include/llvm/Transforms/IPO/FunctionImport.h:117
 /// in the graph from any of the given symbols listed in
-/// \p GUIDPreservedSymbols.
+/// \p GUIDPreservedSymbols. Non-prevailing symbols that are not anywhere in IR
+/// are normally "dead", \p isPrevailing predicate returns status of symbol.
----------------
Suggest changing "that are not anywhere in IR are normally dead" to something like "are symbols without a prevailing copy anywhere in IR and are normally dead". I.e. a non-prevailing symbol here can have a def in IR, it is just not prevailing.


================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:501
+    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
+    function_ref<PrevailingType(GlobalValue::GUID)> isPrevailing) {
   assert(!Index.withGlobalValueDeadStripping());
----------------
I think you lost the corresponding change to the header declaration in this version of the patch.


https://reviews.llvm.org/D42107