[ThinLTO] - Simplify code in ThinLTOBitcodeWriter.

[grimar]

Summary

Recently introduced convertToDeclaration is very similar
to code used in filterModule function.
Patch reuses it to reduce duplication.

[tejohnson]

LGTM with the below changes. Thanks for the cleanup!

lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

174 ↗

(On Diff #133007)

Recommend keeping the old callback name - it still fits the behavior and reduces unnecessary churn.

178 ↗

(On Diff #133007)

s/GA/GV/ since to match the data type.

[grimar]

Reimplemented.

[grimar]

Sorry, I had to revert it after commit, it broke few bots.

It happened because it turned out that order of proccessing
globals is important here, with 'ShouldKeepDefinition' implementation
it works only if aliases are dropped first (like original code did)
(https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp#L320)
otherwise if we drop the aliasee's metadata first, condition
if (HasTypeMetadata(... will never trigger for alias and we never drop it.

I suggest to use work list then. It even makes code a bit more simple.
Updated the patch, verified that check-all pass.

[tejohnson]

LGTM

llvm/trunk/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/FunctionAttrs.h"
+#include "llvm/Transforms/IPO/FunctionImport.h"
 #include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 using namespace llvm;
 
 namespace {
 
 // Promote each local-linkage entity defined by ExportM and used by ImportM by
 // changing visibility and appending the given ModuleId.
for (auto I = M.global_begin(), E = M.global_end(); I != E;) {
     GlobalVariable &GV = *I++;
     if (GV.isDeclaration() && GV.use_empty()) {
       GV.eraseFromParent();
       continue;
     }
   }
 }
 
-void filterModule(
-    Module *M, function_ref<bool(const GlobalValue *)> ShouldKeepDefinition) {
-  for (Module::alias_iterator I = M->alias_begin(), E = M->alias_end();
-       I != E;) {
-    GlobalAlias *GA = &*I++;
-    if (ShouldKeepDefinition(GA))
-      continue;
-
-    GlobalObject *GO;
-    if (GA->getValueType()->isFunctionTy())
-      GO = Function::Create(cast<FunctionType>(GA->getValueType()),
-                            GlobalValue::ExternalLinkage, "", M);
-    else
-      GO = new GlobalVariable(
-          *M, GA->getValueType(), false, GlobalValue::ExternalLinkage,
-          nullptr, "", nullptr,
-          GA->getThreadLocalMode(), GA->getType()->getAddressSpace());
-    GO->takeName(GA);
-    GA->replaceAllUsesWith(GO);
-    GA->eraseFromParent();
-  }
-
-  for (Function &F : *M) {
-    if (ShouldKeepDefinition(&F))
-      continue;
-
-    F.deleteBody();
-    F.setComdat(nullptr);
-    F.clearMetadata();
-  }
-
-  for (GlobalVariable &GV : M->globals()) {
-    if (ShouldKeepDefinition(&GV))
-      continue;
-
-    GV.setInitializer(nullptr);
-    GV.setLinkage(GlobalValue::ExternalLinkage);
-    GV.setComdat(nullptr);
-    GV.clearMetadata();
-  }
+static void
+filterModule(Module *M,
+             function_ref<bool(const GlobalValue *)> ShouldKeepDefinition) {
+  std::vector<GlobalValue *> V;
+  for (GlobalValue &GV : M->global_values())
+    if (!ShouldKeepDefinition(&GV))
+      V.push_back(&GV);
+
+  for (GlobalValue *GV : V)
+    if (!convertToDeclaration(*GV))
+      GV->eraseFromParent();
 }
 
 void forEachVirtualFunction(Constant *C, function_ref<void(Function *)> Fn) {
   if (auto *F = dyn_cast<Function>(C))
     return Fn(F);
   if (isa<GlobalValue>(C))
     return;
   for (Value *Op : C->operands())