[PATCH] D14843: [ThinLTO/LTO] Don't link in unneeded metadata

[Teresa Johnson via llvm-commits]

tejohnson marked 2 inline comments as done.
tejohnson added a comment.

Thanks for the review. Rebased and addressed all your comments. Final version going in shortly.


================
Comment at: lib/Linker/LinkModules.cpp:482
@@ +481,3 @@
+  /// or via an inlined body in an imported function.
+  SmallPtrSet<const Metadata *, 16> *UnneededSubprograms;
+
----------------
joker.eph wrote:
> Replace with a real member instead of a pointer.
Done, along with your related suggestions that fall out as a result of this change.

================
Comment at: lib/Linker/LinkModules.cpp:1826
@@ +1825,3 @@
+// Squash null subprograms from compile unit subprogram lists.
+void ModuleLinker::stripNullSubprograms() {
+  NamedMDNode *CompileUnits = DstM->getNamedMetadata("llvm.dbg.cu");
----------------
joker.eph wrote:
> Is there a test for this? I'm not sure I saw a `CHECK-NOT: null` line or something equivalent?
I added one to test/Linker/thinlto_funcimport_debug.ll. Turns out there is a verifier assert that fires if you have null in the SP list. I had to comment that out temporarily to test my new check. But I think it makes sense to check for it explicitly in the test case too.

================
Comment at: lib/Linker/LinkModules.cpp:1828
@@ +1827,3 @@
+  NamedMDNode *CompileUnits = DstM->getNamedMetadata("llvm.dbg.cu");
+  if (CompileUnits) {
+    for (unsigned I = 0, E = CompileUnits->getNumOperands(); I != E; ++I) {
----------------
joker.eph wrote:
> early exit
Done


http://reviews.llvm.org/D14843