From ce2b45bd7dc5dc17de6b50234f8d24ef1fed1d3b Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Sun, 5 Jul 2015 20:52:35 +0000 Subject: [PATCH] IR: Do not consider available_externally linkage to be linker-weak. From the linker's perspective, an available_externally global is equivalent to an external declaration (per isDeclarationForLinker()), so it is incorrect to consider it to be a weak definition. Also clean up some logic in the dead argument elimination pass and clarify its comments to better explain how its behavior depends on linkage, introduce GlobalValue::isStrongDefinitionForLinker() and start using it throughout the optimizers and backend. Differential Revision: http://reviews.llvm.org/D10941 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@241413 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/GlobalValue.h | 13 ++++--- lib/Analysis/ValueTracking.cpp | 2 +- lib/CodeGen/SelectionDAG/TargetLowering.cpp | 4 +-- lib/Target/AArch64/AArch64Subtarget.cpp | 8 ++--- lib/Target/ARM/ARMISelLowering.cpp | 6 ++-- lib/Target/ARM/ARMSubtarget.cpp | 36 +++++++------------ lib/Target/PowerPC/PPCAsmPrinter.cpp | 6 ++-- lib/Target/PowerPC/PPCFastISel.cpp | 2 +- lib/Target/PowerPC/PPCISelDAGToDAG.cpp | 2 +- lib/Target/PowerPC/PPCISelLowering.cpp | 6 ++-- lib/Target/X86/X86FastISel.cpp | 2 +- lib/Target/X86/X86ISelLowering.cpp | 2 +- lib/Target/X86/X86Subtarget.cpp | 11 +++--- .../IPO/DeadArgumentElimination.cpp | 26 +++++++------- 14 files changed, 55 insertions(+), 71 deletions(-) diff --git a/include/llvm/IR/GlobalValue.h b/include/llvm/IR/GlobalValue.h index f2379705d46..2961369a732 100644 --- a/include/llvm/IR/GlobalValue.h +++ b/include/llvm/IR/GlobalValue.h @@ -252,10 +252,9 @@ public: /// mistake: when working at the IR level use mayBeOverridden instead as it /// knows about ODR semantics. static bool isWeakForLinker(LinkageTypes Linkage) { - return Linkage == AvailableExternallyLinkage || Linkage == WeakAnyLinkage || - Linkage == WeakODRLinkage || Linkage == LinkOnceAnyLinkage || - Linkage == LinkOnceODRLinkage || Linkage == CommonLinkage || - Linkage == ExternalWeakLinkage; + return Linkage == WeakAnyLinkage || Linkage == WeakODRLinkage || + Linkage == LinkOnceAnyLinkage || Linkage == LinkOnceODRLinkage || + Linkage == CommonLinkage || Linkage == ExternalWeakLinkage; } bool hasExternalLinkage() const { return isExternalLinkage(Linkage); } @@ -349,6 +348,12 @@ public: return isDeclaration(); } + /// Returns true if this global's definition will be the one chosen by the + /// linker. + bool isStrongDefinitionForLinker() const { + return !(isDeclarationForLinker() || isWeakForLinker()); + } + /// This method unlinks 'this' from the containing module, but does not delete /// it. virtual void removeFromParent() = 0; diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp index c45005f343d..fa0d7798cae 100644 --- a/lib/Analysis/ValueTracking.cpp +++ b/lib/Analysis/ValueTracking.cpp @@ -1464,7 +1464,7 @@ void computeKnownBits(Value *V, APInt &KnownZero, APInt &KnownOne, // If the object is defined in the current Module, we'll be giving // it the preferred alignment. Otherwise, we have to assume that it // may only have the minimum ABI alignment. - if (!GVar->isDeclaration() && !GVar->isWeakForLinker()) + if (GVar->isStrongDefinitionForLinker()) Align = DL.getPreferredAlignment(GVar); else Align = DL.getABITypeAlignment(ObjectType); diff --git a/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/lib/CodeGen/SelectionDAG/TargetLowering.cpp index f8bdd6806fa..b6d14414f6a 100644 --- a/lib/CodeGen/SelectionDAG/TargetLowering.cpp +++ b/lib/CodeGen/SelectionDAG/TargetLowering.cpp @@ -265,9 +265,7 @@ TargetLowering::isOffsetFoldingLegal(const GlobalAddressSDNode *GA) const { // In dynamic-no-pic mode, assume that known defined values are safe. if (getTargetMachine().getRelocationModel() == Reloc::DynamicNoPIC && - GA && - !GA->getGlobal()->isDeclaration() && - !GA->getGlobal()->isWeakForLinker()) + GA && GA->getGlobal()->isStrongDefinitionForLinker()) return true; // Otherwise assume nothing is safe. diff --git a/lib/Target/AArch64/AArch64Subtarget.cpp b/lib/Target/AArch64/AArch64Subtarget.cpp index 554826b1e08..c88b559d1d5 100644 --- a/lib/Target/AArch64/AArch64Subtarget.cpp +++ b/lib/Target/AArch64/AArch64Subtarget.cpp @@ -57,7 +57,7 @@ AArch64Subtarget::AArch64Subtarget(const Triple &TT, const std::string &CPU, unsigned char AArch64Subtarget::ClassifyGlobalReference(const GlobalValue *GV, const TargetMachine &TM) const { - bool isDecl = GV->isDeclarationForLinker(); + bool isDef = GV->isStrongDefinitionForLinker(); // MachO large model always goes via a GOT, simply to get a single 8-byte // absolute relocation on all global addresses. @@ -66,8 +66,7 @@ AArch64Subtarget::ClassifyGlobalReference(const GlobalValue *GV, // The small code mode's direct accesses use ADRP, which cannot necessarily // produce the value 0 (if the code is above 4GB). - if (TM.getCodeModel() == CodeModel::Small && - GV->isWeakForLinker() && isDecl) { + if (TM.getCodeModel() == CodeModel::Small && GV->hasExternalWeakLinkage()) { // In PIC mode use the GOT, but in absolute mode use a constant pool load. if (TM.getRelocationModel() == Reloc::Static) return AArch64II::MO_CONSTPOOL; @@ -85,8 +84,7 @@ AArch64Subtarget::ClassifyGlobalReference(const GlobalValue *GV, // defined could end up in unexpected places. Use a GOT. if (TM.getRelocationModel() != Reloc::Static && GV->hasDefaultVisibility()) { if (isTargetMachO()) - return (isDecl || GV->isWeakForLinker()) ? AArch64II::MO_GOT - : AArch64II::MO_NO_FLAG; + return isDef ? AArch64II::MO_NO_FLAG : AArch64II::MO_GOT; else // No need to go through the GOT for local symbols on ELF. return GV->hasLocalLinkage() ? AArch64II::MO_NO_FLAG : AArch64II::MO_GOT; diff --git a/lib/Target/ARM/ARMISelLowering.cpp b/lib/Target/ARM/ARMISelLowering.cpp index ba109e61982..607ce767be2 100644 --- a/lib/Target/ARM/ARMISelLowering.cpp +++ b/lib/Target/ARM/ARMISelLowering.cpp @@ -1734,12 +1734,12 @@ ARMTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI, } else if (GlobalAddressSDNode *G = dyn_cast(Callee)) { const GlobalValue *GV = G->getGlobal(); isDirect = true; - bool isExt = GV->isDeclaration() || GV->isWeakForLinker(); - bool isStub = (isExt && Subtarget->isTargetMachO()) && + bool isDef = GV->isStrongDefinitionForLinker(); + bool isStub = (!isDef && Subtarget->isTargetMachO()) && getTargetMachine().getRelocationModel() != Reloc::Static; isARMFunc = !Subtarget->isThumb() || (isStub && !Subtarget->isMClass()); // ARM call to a local ARM function is predicable. - isLocalARMFunc = !Subtarget->isThumb() && (!isExt || !ARMInterworking); + isLocalARMFunc = !Subtarget->isThumb() && (isDef || !ARMInterworking); // tBX takes a register source operand. if (isStub && Subtarget->isThumb1Only() && !Subtarget->hasV5TOps()) { assert(Subtarget->isTargetMachO() && "WrapperPIC use on non-MachO?"); diff --git a/lib/Target/ARM/ARMSubtarget.cpp b/lib/Target/ARM/ARMSubtarget.cpp index 55808dfb9ef..af18eeabf0e 100644 --- a/lib/Target/ARM/ARMSubtarget.cpp +++ b/lib/Target/ARM/ARMSubtarget.cpp @@ -286,7 +286,7 @@ ARMSubtarget::GVIsIndirectSymbol(const GlobalValue *GV, if (RelocM == Reloc::Static) return false; - bool isDecl = GV->isDeclarationForLinker(); + bool isDef = GV->isStrongDefinitionForLinker(); if (!isTargetMachO()) { // Extra load is needed for all externally visible. @@ -294,34 +294,22 @@ ARMSubtarget::GVIsIndirectSymbol(const GlobalValue *GV, return false; return true; } else { - if (RelocM == Reloc::PIC_) { - // If this is a strong reference to a definition, it is definitely not - // through a stub. - if (!isDecl && !GV->isWeakForLinker()) - return false; - - // Unless we have a symbol with hidden visibility, we have to go through a - // normal $non_lazy_ptr stub because this symbol might be resolved late. - if (!GV->hasHiddenVisibility()) // Non-hidden $non_lazy_ptr reference. - return true; + // If this is a strong reference to a definition, it is definitely not + // through a stub. + if (isDef) + return false; + + // Unless we have a symbol with hidden visibility, we have to go through a + // normal $non_lazy_ptr stub because this symbol might be resolved late. + if (!GV->hasHiddenVisibility()) // Non-hidden $non_lazy_ptr reference. + return true; + if (RelocM == Reloc::PIC_) { // If symbol visibility is hidden, we have a stub for common symbol // references and external declarations. - if (isDecl || GV->hasCommonLinkage()) + if (GV->isDeclarationForLinker() || GV->hasCommonLinkage()) // Hidden $non_lazy_ptr reference. return true; - - return false; - } else { - // If this is a strong reference to a definition, it is definitely not - // through a stub. - if (!isDecl && !GV->isWeakForLinker()) - return false; - - // Unless we have a symbol with hidden visibility, we have to go through a - // normal $non_lazy_ptr stub because this symbol might be resolved late. - if (!GV->hasHiddenVisibility()) // Non-hidden $non_lazy_ptr reference. - return true; } } diff --git a/lib/Target/PowerPC/PPCAsmPrinter.cpp b/lib/Target/PowerPC/PPCAsmPrinter.cpp index 87a5236e711..adcf8ee8291 100644 --- a/lib/Target/PowerPC/PPCAsmPrinter.cpp +++ b/lib/Target/PowerPC/PPCAsmPrinter.cpp @@ -197,7 +197,7 @@ void PPCAsmPrinter::printOperand(const MachineInstr *MI, unsigned OpNo, // External or weakly linked global variables need non-lazily-resolved stubs if (TM.getRelocationModel() != Reloc::Static && - (GV->isDeclaration() || GV->isWeakForLinker())) { + !GV->isStrongDefinitionForLinker()) { if (!GV->hasHiddenVisibility()) { SymToPrint = getSymbolWithGlobalValueBase(GV, "$non_lazy_ptr"); MachineModuleInfoImpl::StubValueTy &StubSym = @@ -624,7 +624,7 @@ void PPCAsmPrinter::EmitInstruction(const MachineInstr *MI) { IsExternal = GV->isDeclaration(); IsCommon = GV->hasCommonLinkage(); IsNonLocalFunction = GV->getType()->getElementType()->isFunctionTy() && - (GV->isDeclaration() || GV->isWeakForLinker()); + !GV->isStrongDefinitionForLinker(); IsAvailExt = GV->hasAvailableExternallyLinkage(); } else if (MO.isCPI()) MOSymbol = GetCPISymbol(MO.getIndex()); @@ -706,7 +706,7 @@ void PPCAsmPrinter::EmitInstruction(const MachineInstr *MI) { MOSymbol = getSymbol(GV); IsExternal = GV->isDeclaration(); IsNonLocalFunction = GV->getType()->getElementType()->isFunctionTy() && - (GV->isDeclaration() || GV->isWeakForLinker()); + !GV->isStrongDefinitionForLinker(); } else if (MO.isCPI()) MOSymbol = GetCPISymbol(MO.getIndex()); diff --git a/lib/Target/PowerPC/PPCFastISel.cpp b/lib/Target/PowerPC/PPCFastISel.cpp index fafcd76f9d1..ac59542cb7a 100644 --- a/lib/Target/PowerPC/PPCFastISel.cpp +++ b/lib/Target/PowerPC/PPCFastISel.cpp @@ -1979,7 +1979,7 @@ unsigned PPCFastISel::PPCMaterializeGV(const GlobalValue *GV, MVT VT) { // on the "if" path here. if (CModel == CodeModel::Large || (GV->getType()->getElementType()->isFunctionTy() && - (GV->isDeclaration() || GV->isWeakForLinker())) || + !GV->isStrongDefinitionForLinker()) || GV->isDeclaration() || GV->hasCommonLinkage() || GV->hasAvailableExternallyLinkage()) BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(PPC::LDtocL), diff --git a/lib/Target/PowerPC/PPCISelDAGToDAG.cpp b/lib/Target/PowerPC/PPCISelDAGToDAG.cpp index c85c2610d2f..7004a805a96 100644 --- a/lib/Target/PowerPC/PPCISelDAGToDAG.cpp +++ b/lib/Target/PowerPC/PPCISelDAGToDAG.cpp @@ -2901,7 +2901,7 @@ SDNode *PPCDAGToDAGISel::Select(SDNode *N) { if (GlobalAddressSDNode *G = dyn_cast(GA)) { const GlobalValue *GValue = G->getGlobal(); if ((GValue->getType()->getElementType()->isFunctionTy() && - (GValue->isDeclaration() || GValue->isWeakForLinker())) || + !GValue->isStrongDefinitionForLinker()) || GValue->isDeclaration() || GValue->hasCommonLinkage() || GValue->hasAvailableExternallyLinkage()) return transferMemOperands(N, CurDAG->getMachineNode(PPC::LDtocL, dl, diff --git a/lib/Target/PowerPC/PPCISelLowering.cpp b/lib/Target/PowerPC/PPCISelLowering.cpp index 76ff04e1958..303d935b477 100644 --- a/lib/Target/PowerPC/PPCISelLowering.cpp +++ b/lib/Target/PowerPC/PPCISelLowering.cpp @@ -4084,8 +4084,7 @@ unsigned PrepareCall(SelectionDAG &DAG, SDValue &Callee, SDValue &InFlag, if ((DAG.getTarget().getRelocationModel() != Reloc::Static && (Subtarget.getTargetTriple().isMacOSX() && Subtarget.getTargetTriple().isMacOSXVersionLT(10, 5)) && - (G->getGlobal()->isDeclaration() || - G->getGlobal()->isWeakForLinker())) || + !G->getGlobal()->isStrongDefinitionForLinker()) || (Subtarget.isTargetELF() && !isPPC64 && !G->getGlobal()->hasLocalLinkage() && DAG.getTarget().getRelocationModel() == Reloc::PIC_)) { @@ -4254,8 +4253,7 @@ static bool isLocalCall(const SDValue &Callee) { if (GlobalAddressSDNode *G = dyn_cast(Callee)) - return !G->getGlobal()->isDeclaration() && - !G->getGlobal()->isWeakForLinker(); + return G->getGlobal()->isStrongDefinitionForLinker(); return false; } diff --git a/lib/Target/X86/X86FastISel.cpp b/lib/Target/X86/X86FastISel.cpp index 02645460b6a..0e2b1d31f04 100644 --- a/lib/Target/X86/X86FastISel.cpp +++ b/lib/Target/X86/X86FastISel.cpp @@ -3108,7 +3108,7 @@ bool X86FastISel::fastLowerCall(CallLoweringInfo &CLI) { GV->hasDefaultVisibility() && !GV->hasLocalLinkage()) { OpFlags = X86II::MO_PLT; } else if (Subtarget->isPICStyleStubAny() && - (GV->isDeclaration() || GV->isWeakForLinker()) && + !GV->isStrongDefinitionForLinker() && (!Subtarget->getTargetTriple().isMacOSX() || Subtarget->getTargetTriple().isMacOSXVersionLT(10, 5))) { // PC-relative references to external symbols should go through $stub, diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp index ee6785fd953..74d8e2187a6 100644 --- a/lib/Target/X86/X86ISelLowering.cpp +++ b/lib/Target/X86/X86ISelLowering.cpp @@ -3109,7 +3109,7 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI, GV->hasDefaultVisibility() && !GV->hasLocalLinkage()) { OpFlags = X86II::MO_PLT; } else if (Subtarget->isPICStyleStubAny() && - (GV->isDeclaration() || GV->isWeakForLinker()) && + !GV->isStrongDefinitionForLinker() && (!Subtarget->getTargetTriple().isMacOSX() || Subtarget->getTargetTriple().isMacOSXVersionLT(10, 5))) { // PC-relative references to external symbols should go through $stub, diff --git a/lib/Target/X86/X86Subtarget.cpp b/lib/Target/X86/X86Subtarget.cpp index 3b25d30dc22..8076443795d 100644 --- a/lib/Target/X86/X86Subtarget.cpp +++ b/lib/Target/X86/X86Subtarget.cpp @@ -68,7 +68,7 @@ ClassifyGlobalReference(const GlobalValue *GV, const TargetMachine &TM) const { if (GV->hasDLLImportStorageClass()) return X86II::MO_DLLIMPORT; - bool isDecl = GV->isDeclarationForLinker(); + bool isDef = GV->isStrongDefinitionForLinker(); // X86-64 in PIC mode. if (isPICStyleRIPRel()) { @@ -80,8 +80,7 @@ ClassifyGlobalReference(const GlobalValue *GV, const TargetMachine &TM) const { // If symbol visibility is hidden, the extra load is not needed if // target is x86-64 or the symbol is definitely defined in the current // translation unit. - if (GV->hasDefaultVisibility() && - (isDecl || GV->isWeakForLinker())) + if (GV->hasDefaultVisibility() && !isDef) return X86II::MO_GOTPCREL; } else if (!isTargetWin64()) { assert(isTargetELF() && "Unknown rip-relative target"); @@ -107,7 +106,7 @@ ClassifyGlobalReference(const GlobalValue *GV, const TargetMachine &TM) const { // If this is a strong reference to a definition, it is definitely not // through a stub. - if (!isDecl && !GV->isWeakForLinker()) + if (isDef) return X86II::MO_PIC_BASE_OFFSET; // Unless we have a symbol with hidden visibility, we have to go through a @@ -117,7 +116,7 @@ ClassifyGlobalReference(const GlobalValue *GV, const TargetMachine &TM) const { // If symbol visibility is hidden, we have a stub for common symbol // references and external declarations. - if (isDecl || GV->hasCommonLinkage()) { + if (GV->isDeclarationForLinker() || GV->hasCommonLinkage()) { // Hidden $non_lazy_ptr reference. return X86II::MO_DARWIN_HIDDEN_NONLAZY_PIC_BASE; } @@ -131,7 +130,7 @@ ClassifyGlobalReference(const GlobalValue *GV, const TargetMachine &TM) const { // If this is a strong reference to a definition, it is definitely not // through a stub. - if (!isDecl && !GV->isWeakForLinker()) + if (isDef) return X86II::MO_NO_FLAG; // Unless we have a symbol with hidden visibility, we have to go through a diff --git a/lib/Transforms/IPO/DeadArgumentElimination.cpp b/lib/Transforms/IPO/DeadArgumentElimination.cpp index 76898f27505..d0447640259 100644 --- a/lib/Transforms/IPO/DeadArgumentElimination.cpp +++ b/lib/Transforms/IPO/DeadArgumentElimination.cpp @@ -326,7 +326,18 @@ bool DAE::DeleteDeadVarargs(Function &Fn) { /// instead. bool DAE::RemoveDeadArgumentsFromCallers(Function &Fn) { - if (Fn.isDeclaration() || Fn.mayBeOverridden()) + // We cannot change the arguments if this TU does not define the function or + // if the linker may choose a function body from another TU, even if the + // nominal linkage indicates that other copies of the function have the same + // semantics. In the below example, the dead load from %p may not have been + // eliminated from the linker-chosen copy of f, so replacing %p with undef + // in callers may introduce undefined behavior. + // + // define linkonce_odr void @f(i32* %p) { + // %v = load i32 %p + // ret void + // } + if (!Fn.isStrongDefinitionForLinker()) return false; // Functions with local linkage should already have been handled, except the @@ -334,19 +345,6 @@ bool DAE::RemoveDeadArgumentsFromCallers(Function &Fn) if (Fn.hasLocalLinkage() && !Fn.getFunctionType()->isVarArg()) return false; - // If a function seen at compile time is not necessarily the one linked to - // the binary being built, it is illegal to change the actual arguments - // passed to it. These functions can be captured by isWeakForLinker(). - // *NOTE* that mayBeOverridden() is insufficient for this purpose as it - // doesn't include linkage types like AvailableExternallyLinkage and - // LinkOnceODRLinkage. Take link_odr* as an example, it indicates a set of - // *EQUIVALENT* globals that can be merged at link-time. However, the - // semantic of *EQUIVALENT*-functions includes parameters. Changing - // parameters breaks this assumption. - // - if (Fn.isWeakForLinker()) - return false; - if (Fn.use_empty()) return false; -- 2.34.1