From: Rafael Espindola Date: Sun, 2 Nov 2014 13:28:57 +0000 (+0000) Subject: Revert r221096 bringing back r221014 with a fix. X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=0a6665ead5089102595e83d337da491b639e5b4d Revert r221096 bringing back r221014 with a fix. The issue was that linkAppendingVarProto does the full linking job, including deleting the old dst variable. The fix is just to call it and return early if we have a GV with appending linkage. original message: Refactor duplicated code in liking GlobalValues. There is quiet a bit of logic that is common to any GlobalValue but was duplicated for Functions, GlobalVariables and GlobalAliases. While at it, merge visibility even when comdats are used, fixing pr21415. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@221098 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp index 4320e886bbb..bac187bb3a3 100644 --- a/lib/Linker/LinkModules.cpp +++ b/lib/Linker/LinkModules.cpp @@ -453,13 +453,6 @@ namespace { bool getComdatResult(const Comdat *SrcC, Comdat::SelectionKind &SK, bool &LinkFromSrc); - /// This analyzes the two global values and determines what the result will - /// look like in the destination module. - bool getLinkageResult(GlobalValue *Dest, const GlobalValue *Src, - GlobalValue::LinkageTypes <, - GlobalValue::VisibilityTypes &Vis, - bool &LinkFromSrc); - /// Given a global in the source module, return the global in the /// destination module that is being linked to, if any. GlobalValue *getLinkedToGlobal(const GlobalValue *SrcGV) { @@ -488,9 +481,15 @@ namespace { bool linkAppendingVarProto(GlobalVariable *DstGV, const GlobalVariable *SrcGV); - bool linkGlobalProto(const GlobalVariable *SrcGV); - bool linkFunctionProto(Function *SrcF); - bool linkAliasProto(GlobalAlias *SrcA); + + bool linkGlobalValueProto(GlobalValue *GV); + GlobalValue *linkGlobalVariableProto(const GlobalVariable *SGVar, + GlobalValue *DGV, bool LinkFromSrc); + GlobalValue *linkFunctionProto(const Function *SF, GlobalValue *DGV, + bool LinkFromSrc); + GlobalValue *linkGlobalAliasProto(const GlobalAlias *SGA, GlobalValue *DGV, + bool LinkFromSrc); + bool linkModuleFlagsMetadata(); void linkAppendingVarInit(const AppendingVarInfo &AVI); @@ -688,6 +687,12 @@ bool ModuleLinker::getComdatResult(const Comdat *SrcC, bool ModuleLinker::shouldLinkFromSource(bool &LinkFromSrc, const GlobalValue &Dest, const GlobalValue &Src) { + // We always have to add Src if it has appending linkage. + if (Src.hasAppendingLinkage()) { + LinkFromSrc = true; + return false; + } + bool SrcIsDeclaration = Src.isDeclarationForLinker(); bool DestIsDeclaration = Dest.isDeclarationForLinker(); @@ -757,36 +762,6 @@ bool ModuleLinker::shouldLinkFromSource(bool &LinkFromSrc, "': symbol multiply defined!"); } -/// This analyzes the two global values and determines what the result will look -/// like in the destination module. In particular, it computes the resultant -/// linkage type and visibility, computes whether the global in the source -/// should be copied over to the destination (replacing the existing one), and -/// computes whether this linkage is an error or not. -bool ModuleLinker::getLinkageResult(GlobalValue *Dest, const GlobalValue *Src, - GlobalValue::LinkageTypes <, - GlobalValue::VisibilityTypes &Vis, - bool &LinkFromSrc) { - assert(Dest && "Must have two globals being queried"); - assert(!Src->hasLocalLinkage() && - "If Src has internal linkage, Dest shouldn't be set!"); - - if (shouldLinkFromSource(LinkFromSrc, *Dest, *Src)) - return true; - - if (LinkFromSrc) - LT = Src->getLinkage(); - else - LT = Dest->getLinkage(); - - // Compute the visibility. We follow the rules in the System V Application - // Binary Interface. - assert(!GlobalValue::isLocalLinkage(LT) && - "Symbols with local linkage should not be merged"); - Vis = isLessConstraining(Src->getVisibility(), Dest->getVisibility()) ? - Dest->getVisibility() : Src->getVisibility(); - return false; -} - /// Loop over all of the linked values to compute type mappings. For example, /// if we link "extern Foo *x" and "Foo *x = NULL", then we have two struct /// types 'Foo' but one got renamed when the module was loaded into the same @@ -1010,256 +985,163 @@ bool ModuleLinker::linkAppendingVarProto(GlobalVariable *DstGV, return false; } -/// Loop through the global variables in the src module and merge them into the -/// dest module. -bool ModuleLinker::linkGlobalProto(const GlobalVariable *SGV) { +bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) { GlobalValue *DGV = getLinkedToGlobal(SGV); - llvm::Optional NewVisibility; + + // Handle the ultra special appending linkage case first. + if (DGV && DGV->hasAppendingLinkage()) + return linkAppendingVarProto(cast(DGV), + cast(SGV)); + + bool LinkFromSrc = true; + Comdat *C = nullptr; + GlobalValue::VisibilityTypes Visibility = SGV->getVisibility(); bool HasUnnamedAddr = SGV->hasUnnamedAddr(); - unsigned Alignment = SGV->getAlignment(); - bool LinkFromSrc = false; - Comdat *DC = nullptr; if (const Comdat *SC = SGV->getComdat()) { Comdat::SelectionKind SK; std::tie(SK, LinkFromSrc) = ComdatsChosen[SC]; - DC = DstM->getOrInsertComdat(SC->getName()); - DC->setSelectionKind(SK); + C = DstM->getOrInsertComdat(SC->getName()); + C->setSelectionKind(SK); + } else if (DGV) { + if (shouldLinkFromSource(LinkFromSrc, *DGV, *SGV)) + return true; } - if (DGV) { - if (!DC) { - // Concatenation of appending linkage variables is magic and handled later. - if (DGV->hasAppendingLinkage() || SGV->hasAppendingLinkage()) - return linkAppendingVarProto(cast(DGV), SGV); - - // Determine whether linkage of these two globals follows the source - // module's definition or the destination module's definition. - GlobalValue::LinkageTypes NewLinkage = GlobalValue::InternalLinkage; - GlobalValue::VisibilityTypes NV; - if (getLinkageResult(DGV, SGV, NewLinkage, NV, LinkFromSrc)) - return true; - NewVisibility = NV; - HasUnnamedAddr = HasUnnamedAddr && DGV->hasUnnamedAddr(); - if (DGV->hasCommonLinkage() && SGV->hasCommonLinkage()) - Alignment = std::max(Alignment, DGV->getAlignment()); - else if (!LinkFromSrc) - Alignment = DGV->getAlignment(); - - // If we're not linking from the source, then keep the definition that we - // have. - if (!LinkFromSrc) { - // Special case for const propagation. - if (GlobalVariable *DGVar = dyn_cast(DGV)) { - DGVar->setAlignment(Alignment); - - if (DGVar->isDeclaration() && !SGV->isConstant()) - DGVar->setConstant(false); - } - - // Set calculated linkage, visibility and unnamed_addr. - DGV->setLinkage(NewLinkage); - DGV->setVisibility(*NewVisibility); - DGV->setUnnamedAddr(HasUnnamedAddr); - } - } + if (!LinkFromSrc) { + // Track the source global so that we don't attempt to copy it over when + // processing global initializers. + DoNotLinkFromSource.insert(SGV); - if (!LinkFromSrc) { + if (DGV) // Make sure to remember this mapping. - ValueMap[SGV] = ConstantExpr::getBitCast(DGV,TypeMap.get(SGV->getType())); - - // Track the source global so that we don't attempt to copy it over when - // processing global initializers. - DoNotLinkFromSource.insert(SGV); + ValueMap[SGV] = + ConstantExpr::getBitCast(DGV, TypeMap.get(SGV->getType())); + } - return false; - } + if (DGV) { + Visibility = isLessConstraining(Visibility, DGV->getVisibility()) + ? DGV->getVisibility() + : Visibility; + HasUnnamedAddr = HasUnnamedAddr && DGV->hasUnnamedAddr(); } - // If the Comdat this variable was inside of wasn't selected, skip it. - if (DC && !DGV && !LinkFromSrc) { - DoNotLinkFromSource.insert(SGV); + if (!LinkFromSrc && !DGV) return false; + + GlobalValue *NewGV; + if (auto *SGVar = dyn_cast(SGV)) { + NewGV = linkGlobalVariableProto(SGVar, DGV, LinkFromSrc); + if (!NewGV) + return true; + } else if (auto *SF = dyn_cast(SGV)) { + NewGV = linkFunctionProto(SF, DGV, LinkFromSrc); + } else { + NewGV = linkGlobalAliasProto(cast(SGV), DGV, LinkFromSrc); } - // No linking to be performed or linking from the source: simply create an - // identical version of the symbol over in the dest module... the - // initializer will be filled in later by LinkGlobalInits. - GlobalVariable *NewDGV = - new GlobalVariable(*DstM, TypeMap.get(SGV->getType()->getElementType()), - SGV->isConstant(), SGV->getLinkage(), /*init*/nullptr, - SGV->getName(), /*insertbefore*/nullptr, - SGV->getThreadLocalMode(), - SGV->getType()->getAddressSpace()); - // Propagate alignment, visibility and section info. - copyGVAttributes(NewDGV, SGV); - NewDGV->setAlignment(Alignment); - if (NewVisibility) - NewDGV->setVisibility(*NewVisibility); - NewDGV->setUnnamedAddr(HasUnnamedAddr); + if (NewGV) { + if (NewGV != DGV) + copyGVAttributes(NewGV, SGV); - if (DC) - NewDGV->setComdat(DC); + NewGV->setUnnamedAddr(HasUnnamedAddr); + NewGV->setVisibility(Visibility); - if (DGV) { - DGV->replaceAllUsesWith(ConstantExpr::getBitCast(NewDGV, DGV->getType())); - DGV->eraseFromParent(); + if (auto *NewGO = dyn_cast(NewGV)) { + if (C) + NewGO->setComdat(C); + } + + // Make sure to remember this mapping. + if (NewGV != DGV) { + if (DGV) { + DGV->replaceAllUsesWith( + ConstantExpr::getBitCast(NewGV, DGV->getType())); + DGV->eraseFromParent(); + } + ValueMap[SGV] = NewGV; + } } - // Make sure to remember this mapping. - ValueMap[SGV] = NewDGV; return false; } -/// Link the function in the source module into the destination module if -/// needed, setting up mapping information. -bool ModuleLinker::linkFunctionProto(Function *SF) { - GlobalValue *DGV = getLinkedToGlobal(SF); - llvm::Optional NewVisibility; - bool HasUnnamedAddr = SF->hasUnnamedAddr(); - - bool LinkFromSrc = false; - Comdat *DC = nullptr; - if (const Comdat *SC = SF->getComdat()) { - Comdat::SelectionKind SK; - std::tie(SK, LinkFromSrc) = ComdatsChosen[SC]; - DC = DstM->getOrInsertComdat(SC->getName()); - DC->setSelectionKind(SK); - } +/// Loop through the global variables in the src module and merge them into the +/// dest module. +GlobalValue *ModuleLinker::linkGlobalVariableProto(const GlobalVariable *SGVar, + GlobalValue *DGV, + bool LinkFromSrc) { + unsigned Alignment = 0; + bool ClearConstant = false; if (DGV) { - if (!DC) { - GlobalValue::LinkageTypes NewLinkage = GlobalValue::InternalLinkage; - GlobalValue::VisibilityTypes NV; - if (getLinkageResult(DGV, SF, NewLinkage, NV, LinkFromSrc)) - return true; - NewVisibility = NV; - HasUnnamedAddr = HasUnnamedAddr && DGV->hasUnnamedAddr(); - - if (!LinkFromSrc) { - // Set calculated linkage - DGV->setLinkage(NewLinkage); - DGV->setVisibility(*NewVisibility); - DGV->setUnnamedAddr(HasUnnamedAddr); - } - } + if (DGV->hasCommonLinkage() && SGVar->hasCommonLinkage()) + Alignment = std::max(SGVar->getAlignment(), DGV->getAlignment()); - if (!LinkFromSrc) { - // Make sure to remember this mapping. - ValueMap[SF] = ConstantExpr::getBitCast(DGV, TypeMap.get(SF->getType())); - - // Track the function from the source module so we don't attempt to remap - // it. - DoNotLinkFromSource.insert(SF); + auto *DGVar = dyn_cast(DGV); + if (!SGVar->isConstant() || (DGVar && !DGVar->isConstant())) + ClearConstant = true; + } - return false; + if (!LinkFromSrc) { + if (auto *NewGVar = dyn_cast(DGV)) { + if (Alignment) + NewGVar->setAlignment(Alignment); + if (NewGVar->isDeclaration() && ClearConstant) + NewGVar->setConstant(false); } + return DGV; } + // No linking to be performed or linking from the source: simply create an + // identical version of the symbol over in the dest module... the + // initializer will be filled in later by LinkGlobalInits. + GlobalVariable *NewDGV = new GlobalVariable( + *DstM, TypeMap.get(SGVar->getType()->getElementType()), + SGVar->isConstant(), SGVar->getLinkage(), /*init*/ nullptr, + SGVar->getName(), /*insertbefore*/ nullptr, SGVar->getThreadLocalMode(), + SGVar->getType()->getAddressSpace()); + + if (Alignment) + NewDGV->setAlignment(Alignment); + + return NewDGV; +} + +/// Link the function in the source module into the destination module if +/// needed, setting up mapping information. +GlobalValue *ModuleLinker::linkFunctionProto(const Function *SF, + GlobalValue *DGV, + bool LinkFromSrc) { + if (!LinkFromSrc) + return DGV; + // If the function is to be lazily linked, don't create it just yet. // The ValueMaterializerTy will deal with creating it if it's used. if (!DGV && (SF->hasLocalLinkage() || SF->hasLinkOnceLinkage() || SF->hasAvailableExternallyLinkage())) { DoNotLinkFromSource.insert(SF); - return false; - } - - // If the Comdat this function was inside of wasn't selected, skip it. - if (DC && !DGV && !LinkFromSrc) { - DoNotLinkFromSource.insert(SF); - return false; + return nullptr; } // If there is no linkage to be performed or we are linking from the source, // bring SF over. - Function *NewDF = Function::Create(TypeMap.get(SF->getFunctionType()), - SF->getLinkage(), SF->getName(), DstM); - copyGVAttributes(NewDF, SF); - if (NewVisibility) - NewDF->setVisibility(*NewVisibility); - NewDF->setUnnamedAddr(HasUnnamedAddr); - - if (DC) - NewDF->setComdat(DC); - - if (DGV) { - // Any uses of DF need to change to NewDF, with cast. - DGV->replaceAllUsesWith(ConstantExpr::getBitCast(NewDF, DGV->getType())); - DGV->eraseFromParent(); - } - - ValueMap[SF] = NewDF; - return false; + return Function::Create(TypeMap.get(SF->getFunctionType()), SF->getLinkage(), + SF->getName(), DstM); } /// Set up prototypes for any aliases that come over from the source module. -bool ModuleLinker::linkAliasProto(GlobalAlias *SGA) { - GlobalValue *DGV = getLinkedToGlobal(SGA); - llvm::Optional NewVisibility; - bool HasUnnamedAddr = SGA->hasUnnamedAddr(); - - bool LinkFromSrc = false; - Comdat *DC = nullptr; - if (const Comdat *SC = SGA->getComdat()) { - Comdat::SelectionKind SK; - std::tie(SK, LinkFromSrc) = ComdatsChosen[SC]; - DC = DstM->getOrInsertComdat(SC->getName()); - DC->setSelectionKind(SK); - } - - if (DGV) { - if (!DC) { - GlobalValue::LinkageTypes NewLinkage = GlobalValue::InternalLinkage; - GlobalValue::VisibilityTypes NV; - if (getLinkageResult(DGV, SGA, NewLinkage, NV, LinkFromSrc)) - return true; - NewVisibility = NV; - HasUnnamedAddr = HasUnnamedAddr && DGV->hasUnnamedAddr(); - - if (!LinkFromSrc) { - // Set calculated linkage. - DGV->setLinkage(NewLinkage); - DGV->setVisibility(*NewVisibility); - DGV->setUnnamedAddr(HasUnnamedAddr); - } - } - - if (!LinkFromSrc) { - // Make sure to remember this mapping. - ValueMap[SGA] = ConstantExpr::getBitCast(DGV,TypeMap.get(SGA->getType())); - - // Track the alias from the source module so we don't attempt to remap it. - DoNotLinkFromSource.insert(SGA); - - return false; - } - } - - // If the Comdat this alias was inside of wasn't selected, skip it. - if (DC && !DGV && !LinkFromSrc) { - DoNotLinkFromSource.insert(SGA); - return false; - } +GlobalValue *ModuleLinker::linkGlobalAliasProto(const GlobalAlias *SGA, + GlobalValue *DGV, + bool LinkFromSrc) { + if (!LinkFromSrc) + return DGV; // If there is no linkage to be performed or we're linking from the source, // bring over SGA. auto *PTy = cast(TypeMap.get(SGA->getType())); - auto *NewDA = - GlobalAlias::create(PTy->getElementType(), PTy->getAddressSpace(), - SGA->getLinkage(), SGA->getName(), DstM); - copyGVAttributes(NewDA, SGA); - if (NewVisibility) - NewDA->setVisibility(*NewVisibility); - NewDA->setUnnamedAddr(HasUnnamedAddr); - - if (DGV) { - // Any uses of DGV need to change to NewDA, with cast. - DGV->replaceAllUsesWith(ConstantExpr::getBitCast(NewDA, DGV->getType())); - DGV->eraseFromParent(); - } - - ValueMap[SGA] = NewDA; - return false; + return GlobalAlias::create(PTy->getElementType(), PTy->getAddressSpace(), + SGA->getLinkage(), SGA->getName(), DstM); } static void getArrayElements(const Constant *C, @@ -1599,7 +1481,7 @@ bool ModuleLinker::run() { // initializers (which could refer to functions not yet mapped over). for (Module::global_iterator I = SrcM->global_begin(), E = SrcM->global_end(); I != E; ++I) - if (linkGlobalProto(I)) + if (linkGlobalValueProto(I)) return true; // Link the functions together between the two modules, without doing function @@ -1608,13 +1490,13 @@ bool ModuleLinker::run() { // all of the global values that may be referenced are available in our // ValueMap. for (Module::iterator I = SrcM->begin(), E = SrcM->end(); I != E; ++I) - if (linkFunctionProto(I)) + if (linkGlobalValueProto(I)) return true; // If there were any aliases, link them now. for (Module::alias_iterator I = SrcM->alias_begin(), E = SrcM->alias_end(); I != E; ++I) - if (linkAliasProto(I)) + if (linkGlobalValueProto(I)) return true; for (unsigned i = 0, e = AppendingVars.size(); i != e; ++i) diff --git a/test/Linker/Inputs/visibility.ll b/test/Linker/Inputs/visibility.ll index f5b2065b85f..2ab58fd3bd0 100644 --- a/test/Linker/Inputs/visibility.ll +++ b/test/Linker/Inputs/visibility.ll @@ -1,7 +1,10 @@ +$c1 = comdat any + ; Variables @v1 = weak hidden global i32 0 @v2 = weak protected global i32 0 @v3 = weak hidden global i32 0 +@v4 = hidden global i32 1, comdat $c1 ; Aliases @a1 = weak hidden alias i32* @v1 diff --git a/test/Linker/visibility.ll b/test/Linker/visibility.ll index fc950fa1d89..6436197bf20 100644 --- a/test/Linker/visibility.ll +++ b/test/Linker/visibility.ll @@ -4,16 +4,21 @@ ; The values in this file are strong, the ones in Inputs/visibility.ll are weak, ; but we should still get the visibility from them. + +$c1 = comdat any + ; Variables -; CHECK: @v1 = hidden global i32 0 +; CHECK-DAG: @v1 = hidden global i32 0 @v1 = global i32 0 -; CHECK: @v2 = protected global i32 0 +; CHECK-DAG: @v2 = protected global i32 0 @v2 = global i32 0 -; CHECK: @v3 = hidden global i32 0 +; CHECK-DAG: @v3 = hidden global i32 0 @v3 = protected global i32 0 +; CHECK-DAG: @v4 = hidden global i32 1, comdat $c1 +@v4 = global i32 1, comdat $c1 ; Aliases ; CHECK: @a1 = hidden alias i32* @v1