[PATCH] D23488: ThinLTO: add early "dead-stripping" on the Index

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/LTO/LTO.cpp:200
@@ -199,2 +199,3 @@
     GlobalRes.Partition = GlobalResolution::External;
-  else
+    GlobalRes.VisibleToRegularObj = Res.VisibleToRegularObj;
+  } else {
----------------
This should be:
   GlobalRes.VisibleToRegularObj |= Res.VisibleToRegularObj;

with that fix the gold tests pass.

================
Comment at: lib/LTO/LTO.cpp:201
@@ -201,1 +200,3 @@
+    GlobalRes.VisibleToRegularObj = Res.VisibleToRegularObj;
+  } else {
     GlobalRes.Partition = Partition;
----------------
New braces around else block not needed

================
Comment at: lib/LTO/LTO.cpp:603
@@ -590,3 +602,3 @@
             ExportList->second.count(GUID)) ||
-           ExportedGUIDs.count(GUID);
+           (!DeadSymbols.count(GUID) && ExportedGUIDs.count(GUID));
   };
----------------
If a symbol is in ExportedGUIDs, wouldn't it necessarily also be in GUIDPreservedSymbols and therefore not dead (if it is VisibleToRegularObj the partition would also have been set to External). So do we even need the ExportedGUIDs check here? And if not, do we need to compute it?

================
Comment at: test/ThinLTO/X86/deadstrip.ll:18
@@ +17,3 @@
+; RUN:   -r %t2.bc,_another_dead_func,pl
+; RUN: llvm-dis < %t.out.0.3.import.bc
+
----------------
Should be %t.out.1.3.import.bc


https://reviews.llvm.org/D23488