[PATCH] D20268: [wip] Resolution-based LTO API.

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: tools/gold/gold-plugin.cpp:557
@@ -556,3 @@
-      if (GV) {
-        assert(!GV->hasExternalWeakLinkage() &&
-               !GV->hasAvailableExternallyLinkage() && "Not a declaration!");
----------------
Should this assertion checking be moved somewhere (e.g. inside Symbol)?

================
Comment at: tools/gold/gold-plugin.cpp:733
@@ -732,3 @@
-                             Resolution == LDPR_PREVAILING_DEF_IRONLY))
-      Resolution = LDPR_PREVAILING_DEF;
-
----------------
Your patch is working around the lack of this for now by upgrading all the linkages, right? Until gold uses the summary based linkage handling I refactored out of ThinLTOResolution, can't we do this in addThinLTO (conditionally upgradeLinkage for prevailing resolutions), as addRegularLto is doing now?

================
Comment at: tools/gold/gold-plugin.cpp:772
@@ +771,3 @@
+  if (options::Parallelism) {
+    if (options::thinlto)
+      Backend = createInProcessThinBackend(options::Parallelism);
----------------
&& !options::thinlto_index_only ?

================
Comment at: tools/gold/gold-plugin.cpp:792
@@ +791,3 @@
+  Conf.PostInternalizeModuleHook = [](size_t Task, Module &M) -> bool {
+    if (Task == 0)
+      addCommons(M);
----------------
Is there a Task 0 for ThinLTO? If not, do we need to do anything in that case?


http://reviews.llvm.org/D20268