[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

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

178

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

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))
+static void filterModule(Module *M,
+                         function_ref<bool(const GlobalValue *)> ShouldKeep) {
+  auto I = M->global_values().begin();
+  auto E = M->global_values().end();
+  while (I != E) {
+    GlobalValue &GA = *I++;
+    if (ShouldKeep(&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))
+    if (convertToDeclaration(GA))
       continue;
-
-    GV.setInitializer(nullptr);
-    GV.setLinkage(GlobalValue::ExternalLinkage);
-    GV.setComdat(nullptr);
-    GV.clearMetadata();
+    GA.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;