From 9f8eff31db00ba4efdef4c81858d12a7e9c93b46 Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Tue, 28 Oct 2014 00:24:16 +0000 Subject: [PATCH] Remove the PreserveSource linker mode. 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. llvm-svn: 220741 --- llvm/include/llvm/Linker/Linker.h | 24 ++++----------- llvm/lib/Linker/LinkModules.cpp | 49 +++++++++++++------------------ llvm/tools/bugpoint/BugDriver.cpp | 2 +- llvm/tools/bugpoint/Miscompilation.cpp | 10 +++---- llvm/unittests/Linker/LinkModulesTest.cpp | 26 +++++++++------- 5 files changed, 45 insertions(+), 66 deletions(-) diff --git a/llvm/include/llvm/Linker/Linker.h b/llvm/include/llvm/Linker/Linker.h index 50922e3..d1c02c2 100644 --- a/llvm/include/llvm/Linker/Linker.h +++ b/llvm/include/llvm/Linker/Linker.h @@ -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 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; diff --git a/llvm/lib/Linker/LinkModules.cpp b/llvm/lib/Linker/LinkModules.cpp index 1e08262..403a1ff 100644 --- a/llvm/lib/Linker/LinkModules.cpp +++ b/llvm/lib/Linker/LinkModules.cpp @@ -410,8 +410,6 @@ namespace { std::vector AppendingVars; - unsigned Mode; // Mode to treat source module. - // Set of items not to link in from source. SmallPtrSet 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 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()); diff --git a/llvm/tools/bugpoint/BugDriver.cpp b/llvm/tools/bugpoint/BugDriver.cpp index b54ea23..b8be17e 100644 --- a/llvm/tools/bugpoint/BugDriver.cpp +++ b/llvm/tools/bugpoint/BugDriver.cpp @@ -126,7 +126,7 @@ bool BugDriver::addSources(const std::vector &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; } diff --git a/llvm/tools/bugpoint/Miscompilation.cpp b/llvm/tools/bugpoint/Miscompilation.cpp index ce16d4d..8cb4583 100644 --- a/llvm/tools/bugpoint/Miscompilation.cpp +++ b/llvm/tools/bugpoint/Miscompilation.cpp @@ -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. diff --git a/llvm/unittests/Linker/LinkModulesTest.cpp b/llvm/unittests/Linker/LinkModulesTest.cpp index 540849e..a21ee47 100644 --- a/llvm/unittests/Linker/LinkModulesTest.cpp +++ b/llvm/unittests/Linker/LinkModulesTest.cpp @@ -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 InternalM(getInternal(Ctx)); + std::unique_ptr EmptyM(new Module("EmptyModule1", Ctx)); + Linker::LinkModules(EmptyM.get(), InternalM.get()); +} - delete EmptyM; - delete InternalM; +TEST_F(LinkModuleTest, EmptyModule2) { + std::unique_ptr InternalM(getInternal(Ctx)); + std::unique_ptr EmptyM(new Module("EmptyModule1", Ctx)); + Linker::LinkModules(InternalM.get(), EmptyM.get()); } } // end anonymous namespace -- 2.7.4