Remove the PreserveSource linker mode.
authorRafael Espindola <rafael.espindola@gmail.com>
Tue, 28 Oct 2014 00:24:16 +0000 (00:24 +0000)
committerRafael Espindola <rafael.espindola@gmail.com>
Tue, 28 Oct 2014 00:24:16 +0000 (00:24 +0000)
I noticed that it was untested, and forcing it on caused some tests to fail:

    LLVM :: Linker/metadata-a.ll
    LLVM :: Linker/prefixdata.ll
    LLVM :: Linker/type-unique-odr-a.ll
    LLVM :: Linker/type-unique-simple-a.ll
    LLVM :: Linker/type-unique-simple2-a.ll
    LLVM :: Linker/type-unique-simple2.ll
    LLVM :: Linker/type-unique-type-array-a.ll
    LLVM :: Linker/unnamed-addr1-a.ll
    LLVM :: Linker/visibility1.ll

If it is to be resurrected, it has to be fixed and we should probably have a
-preserve-source command line option in llvm-mc and run tests with and without
it.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@220741 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Linker/Linker.h
lib/Linker/LinkModules.cpp
tools/bugpoint/BugDriver.cpp
tools/bugpoint/Miscompilation.cpp
unittests/Linker/LinkModulesTest.cpp

index 50922e3..d1c02c2 100644 (file)
@@ -25,11 +25,6 @@ class StructType;
 /// something with it after the linking.
 class Linker {
   public:
-    enum LinkerMode {
-      DestroySource = 0, // Allow source module to be destroyed.
-      PreserveSource = 1 // Preserve the source module.
-    };
-
     typedef std::function<void(const DiagnosticInfo &)>
         DiagnosticHandlerFunction;
 
@@ -40,23 +35,14 @@ class Linker {
     Module *getModule() const { return Composite; }
     void deleteModule();
 
-    /// \brief Link \p Src into the composite. The source is destroyed if
-    /// \p Mode is DestroySource and preserved if it is PreserveSource.
-    /// If \p ErrorMsg is not null, information about any error is written
-    /// to it.
+    /// \brief Link \p Src into the composite. The source is destroyed.
     /// Returns true on error.
-    bool linkInModule(Module *Src, unsigned Mode);
-    bool linkInModule(Module *Src) {
-      return linkInModule(Src, Linker::DestroySource);
-    }
-
-    static bool
-    LinkModules(Module *Dest, Module *Src, unsigned Mode,
-                DiagnosticHandlerFunction DiagnosticHandler);
+    bool linkInModule(Module *Src);
 
-    static bool
-    LinkModules(Module *Dest, Module *Src, unsigned Mode);
+    static bool LinkModules(Module *Dest, Module *Src,
+                            DiagnosticHandlerFunction DiagnosticHandler);
 
+    static bool LinkModules(Module *Dest, Module *Src);
 
   private:
     Module *Composite;
index 1e08262..403a1ff 100644 (file)
@@ -410,8 +410,6 @@ namespace {
 
     std::vector<AppendingVarInfo> AppendingVars;
 
-    unsigned Mode; // Mode to treat source module.
-
     // Set of items not to link in from source.
     SmallPtrSet<const Value*, 16> DoNotLinkFromSource;
 
@@ -421,10 +419,10 @@ namespace {
     Linker::DiagnosticHandlerFunction DiagnosticHandler;
 
   public:
-    ModuleLinker(Module *dstM, TypeSet &Set, Module *srcM, unsigned mode,
+    ModuleLinker(Module *dstM, TypeSet &Set, Module *srcM,
                  Linker::DiagnosticHandlerFunction DiagnosticHandler)
         : DstM(dstM), SrcM(srcM), TypeMap(Set),
-          ValMaterializer(TypeMap, DstM, LazilyLinkFunctions), Mode(mode),
+          ValMaterializer(TypeMap, DstM, LazilyLinkFunctions),
           DiagnosticHandler(DiagnosticHandler) {}
 
     bool run();
@@ -1337,25 +1335,17 @@ void ModuleLinker::linkFunctionBody(Function *Dst, Function *Src) {
     ValueMap[I] = DI;
   }
 
-  if (Mode == Linker::DestroySource) {
-    // Splice the body of the source function into the dest function.
-    Dst->getBasicBlockList().splice(Dst->end(), Src->getBasicBlockList());
-
-    // At this point, all of the instructions and values of the function are now
-    // copied over.  The only problem is that they are still referencing values in
-    // the Source function as operands.  Loop through all of the operands of the
-    // functions and patch them up to point to the local versions.
-    for (Function::iterator BB = Dst->begin(), BE = Dst->end(); BB != BE; ++BB)
-      for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I)
-        RemapInstruction(I, ValueMap, RF_IgnoreMissingEntries,
-                         &TypeMap, &ValMaterializer);
+  // Splice the body of the source function into the dest function.
+  Dst->getBasicBlockList().splice(Dst->end(), Src->getBasicBlockList());
 
-  } else {
-    // Clone the body of the function into the dest function.
-    SmallVector<ReturnInst*, 8> Returns; // Ignore returns.
-    CloneFunctionInto(Dst, Src, ValueMap, false, Returns, "", nullptr,
-                      &TypeMap, &ValMaterializer);
-  }
+  // At this point, all of the instructions and values of the function are now
+  // copied over.  The only problem is that they are still referencing values in
+  // the Source function as operands.  Loop through all of the operands of the
+  // functions and patch them up to point to the local versions.
+  for (Function::iterator BB = Dst->begin(), BE = Dst->end(); BB != BE; ++BB)
+    for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ++I)
+      RemapInstruction(I, ValueMap, RF_IgnoreMissingEntries, &TypeMap,
+                       &ValMaterializer);
 
   // There is no need to map the arguments anymore.
   for (Function::arg_iterator I = Src->arg_begin(), E = Src->arg_end();
@@ -1745,8 +1735,9 @@ void Linker::deleteModule() {
   Composite = nullptr;
 }
 
-bool Linker::linkInModule(Module *Src, unsigned Mode) {
-  ModuleLinker TheLinker(Composite, IdentifiedStructTypes, Src, Mode, DiagnosticHandler);
+bool Linker::linkInModule(Module *Src) {
+  ModuleLinker TheLinker(Composite, IdentifiedStructTypes, Src,
+                         DiagnosticHandler);
   return TheLinker.run();
 }
 
@@ -1759,15 +1750,15 @@ bool Linker::linkInModule(Module *Src, unsigned Mode) {
 /// true is returned and ErrorMsg (if not null) is set to indicate the problem.
 /// Upon failure, the Dest module could be in a modified state, and shouldn't be
 /// relied on to be consistent.
-bool Linker::LinkModules(Module *Dest, Module *Src, unsigned Mode,
+bool Linker::LinkModules(Module *Dest, Module *Src,
                          DiagnosticHandlerFunction DiagnosticHandler) {
   Linker L(Dest, DiagnosticHandler);
-  return L.linkInModule(Src, Mode);
+  return L.linkInModule(Src);
 }
 
-bool Linker::LinkModules(Module *Dest, Module *Src, unsigned Mode) {
+bool Linker::LinkModules(Module *Dest, Module *Src) {
   Linker L(Dest);
-  return L.linkInModule(Src, Mode);
+  return L.linkInModule(Src);
 }
 
 //===----------------------------------------------------------------------===//
@@ -1782,7 +1773,7 @@ LLVMBool LLVMLinkModules(LLVMModuleRef Dest, LLVMModuleRef Src,
   DiagnosticPrinterRawOStream DP(Stream);
 
   LLVMBool Result = Linker::LinkModules(
-      D, unwrap(Src), Mode, [&](const DiagnosticInfo &DI) { DI.print(DP); });
+      D, unwrap(Src), [&](const DiagnosticInfo &DI) { DI.print(DP); });
 
   if (OutMessages && Result)
     *OutMessages = strdup(Message.c_str());
index b54ea23..b8be17e 100644 (file)
@@ -126,7 +126,7 @@ bool BugDriver::addSources(const std::vector<std::string> &Filenames) {
     if (!M.get()) return true;
 
     outs() << "Linking in input file: '" << Filenames[i] << "'\n";
-    if (Linker::LinkModules(Program, M.get(), Linker::DestroySource))
+    if (Linker::LinkModules(Program, M.get()))
       return true;
   }
 
index ce16d4d..8cb4583 100644 (file)
@@ -222,7 +222,7 @@ static Module *TestMergedProgram(const BugDriver &BD, Module *M1, Module *M2,
     M1 = CloneModule(M1);
     M2 = CloneModule(M2);
   }
-  if (Linker::LinkModules(M1, M2, Linker::DestroySource))
+  if (Linker::LinkModules(M1, M2))
     exit(1);
   delete M2;   // We are done with this module.
 
@@ -392,8 +392,7 @@ static bool ExtractLoops(BugDriver &BD,
                                                   F->getFunctionType()));
       }
 
-      if (Linker::LinkModules(ToNotOptimize, ToOptimizeLoopExtracted,
-                              Linker::DestroySource))
+      if (Linker::LinkModules(ToNotOptimize, ToOptimizeLoopExtracted))
         exit(1);
 
       MiscompiledFunctions.clear();
@@ -422,8 +421,7 @@ static bool ExtractLoops(BugDriver &BD,
     // extraction both didn't break the program, and didn't mask the problem.
     // Replace the current program with the loop extracted version, and try to
     // extract another loop.
-    if (Linker::LinkModules(ToNotOptimize, ToOptimizeLoopExtracted,
-                            Linker::DestroySource))
+    if (Linker::LinkModules(ToNotOptimize, ToOptimizeLoopExtracted))
       exit(1);
 
     delete ToOptimizeLoopExtracted;
@@ -601,7 +599,7 @@ static bool ExtractBlocks(BugDriver &BD,
       MisCompFunctions.push_back(std::make_pair(I->getName(),
                                                 I->getFunctionType()));
 
-  if (Linker::LinkModules(ProgClone, Extracted.get(), Linker::DestroySource))
+  if (Linker::LinkModules(ProgClone, Extracted.get()))
     exit(1);
 
   // Set the new program and delete the old one.
index 540849e..a21ee47 100644 (file)
@@ -88,7 +88,7 @@ TEST_F(LinkModuleTest, BlockAddress) {
   Builder.CreateRet(ConstantPointerNull::get(Type::getInt8PtrTy(Ctx)));
 
   Module *LinkedModule = new Module("MyModuleLinked", Ctx);
-  Linker::LinkModules(LinkedModule, M.get(), Linker::PreserveSource);
+  Linker::LinkModules(LinkedModule, M.get());
 
   // Delete the original module.
   M.reset();
@@ -122,12 +122,13 @@ TEST_F(LinkModuleTest, BlockAddress) {
   delete LinkedModule;
 }
 
-TEST_F(LinkModuleTest, EmptyModule) {
+static Module *getInternal(LLVMContext &Ctx) {
   Module *InternalM = new Module("InternalModule", Ctx);
   FunctionType *FTy = FunctionType::get(
       Type::getVoidTy(Ctx), Type::getInt8PtrTy(Ctx), false /*=isVarArgs*/);
 
-  F = Function::Create(FTy, Function::InternalLinkage, "bar", InternalM);
+  Function *F =
+      Function::Create(FTy, Function::InternalLinkage, "bar", InternalM);
   F->setCallingConv(CallingConv::C);
 
   BasicBlock *BB = BasicBlock::Create(Ctx, "", F);
@@ -141,16 +142,19 @@ TEST_F(LinkModuleTest, EmptyModule) {
                          GlobalValue::InternalLinkage, nullptr, "g");
 
   GV->setInitializer(ConstantStruct::get(STy, F));
+  return InternalM;
+}
 
-  Module *EmptyM = new Module("EmptyModule1", Ctx);
-  Linker::LinkModules(EmptyM, InternalM, Linker::PreserveSource);
-
-  delete EmptyM;
-  EmptyM = new Module("EmptyModule2", Ctx);
-  Linker::LinkModules(InternalM, EmptyM, Linker::PreserveSource);
+TEST_F(LinkModuleTest, EmptyModule) {
+  std::unique_ptr<Module> InternalM(getInternal(Ctx));
+  std::unique_ptr<Module> EmptyM(new Module("EmptyModule1", Ctx));
+  Linker::LinkModules(EmptyM.get(), InternalM.get());
+}
 
-  delete EmptyM;
-  delete InternalM;
+TEST_F(LinkModuleTest, EmptyModule2) {
+  std::unique_ptr<Module> InternalM(getInternal(Ctx));
+  std::unique_ptr<Module> EmptyM(new Module("EmptyModule1", Ctx));
+  Linker::LinkModules(InternalM.get(), EmptyM.get());
 }
 
 } // end anonymous namespace