Skip to content

Commit

Permalink
Remove the PreserveSource linker mode.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@espindola
espindola committed Oct 28, 2014
1 parent 9ef2fd8 commit c35c39b
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 66 deletions.
24 changes: 5 additions & 19 deletions include/llvm/Linker/Linker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
49 changes: 20 additions & 29 deletions lib/Linker/LinkModules.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}

Expand All @@ -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);
}

//===----------------------------------------------------------------------===//
Expand All @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion tools/bugpoint/BugDriver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
10 changes: 4 additions & 6 deletions tools/bugpoint/Miscompilation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
26 changes: 15 additions & 11 deletions unittests/Linker/LinkModulesTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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

0 comments on commit c35c39b

Please sign in to comment.