[PATCH] D39625: Attribute nonlazybind should not affect calls to functions with hidden visibility

[Rafael Avila de Espindola via llvm-commits]

LGTM

Sriraman Tallam via Phabricator <reviews at reviews.llvm.org> writes:

> tmsriram updated this revision to Diff 121930.
> tmsriram added a comment.
>
> Fix comment as suggested.
>
>
> https://reviews.llvm.org/D39625
>
> Files:
>   lib/Target/TargetMachine.cpp
>   lib/Target/X86/X86Subtarget.cpp
>   test/CodeGen/X86/no-plt.ll
>
>
> Index: test/CodeGen/X86/no-plt.ll
> ===================================================================
> --- test/CodeGen/X86/no-plt.ll
> +++ test/CodeGen/X86/no-plt.ll
> @@ -6,18 +6,25 @@
>  define i32 @main() #0 {
>  ; X64: callq *_Z3foov at GOTPCREL(%rip)
>  ; X64: callq _Z3barv
> +; X64: callq _Z3bazv
>  
>  entry:
>    %retval = alloca i32, align 4
>    store i32 0, i32* %retval, align 4
>    %call1 = call i32 @_Z3foov()
>    %call2 = call i32 @_Z3barv()
> +  %call3 = call i32 @_Z3bazv()
>    ret i32 0
>  }
>  
>  ; Function Attrs: nonlazybind
>  declare i32 @_Z3foov() #1
>  
>  declare i32 @_Z3barv() #2
>  
> +; Function Attrs: nonlazybind
> +declare hidden i32 @_Z3bazv() #3
> +
> +
>  attributes #1 = { nonlazybind }
> +attributes #3 = { nonlazybind }
> Index: lib/Target/X86/X86Subtarget.cpp
> ===================================================================
> --- lib/Target/X86/X86Subtarget.cpp
> +++ lib/Target/X86/X86Subtarget.cpp
> @@ -144,15 +144,6 @@
>  unsigned char
>  X86Subtarget::classifyGlobalFunctionReference(const GlobalValue *GV,
>                                                const Module &M) const {
> -  const Function *F = dyn_cast_or_null<Function>(GV);
> -
> -  // Do not use the PLT when explicitly told to do so for ELF 64-bit
> -  // target.
> -  if (isTargetELF() && is64Bit() && F &&
> -      F->hasFnAttribute(Attribute::NonLazyBind) &&
> -      GV->isDeclarationForLinker())
> -    return X86II::MO_GOTPCREL;
> -
>    if (TM.shouldAssumeDSOLocal(M, GV))
>      return X86II::MO_NO_FLAG;
>  
> @@ -162,12 +153,16 @@
>      return X86II::MO_DLLIMPORT;
>    }
>  
> +  const Function *F = dyn_cast_or_null<Function>(GV);
> +
>    if (isTargetELF()) {
>      if (is64Bit() && F && (CallingConv::X86_RegCall == F->getCallingConv()))
>        // According to psABI, PLT stub clobbers XMM8-XMM15.
>        // In Regcall calling convention those registers are used for passing
>        // parameters. Thus we need to prevent lazy binding in Regcall.
>        return X86II::MO_GOTPCREL;
> +    if (F && F->hasFnAttribute(Attribute::NonLazyBind) && is64Bit())
> +      return X86II::MO_GOTPCREL;
>      return X86II::MO_PLT;
>    }
>  
> Index: lib/Target/TargetMachine.cpp
> ===================================================================
> --- lib/Target/TargetMachine.cpp
> +++ lib/Target/TargetMachine.cpp
> @@ -167,6 +167,13 @@
>      if (GV && !GV->isDeclarationForLinker())
>        return true;
>  
> +    // A symbol marked nonlazybind should not be accessed with a plt. If the
> +    // symbol turns out to be external, the linker will convert a direct
> +    // access to an access via the plt, so don't assume it is local.
> +    const Function *F = dyn_cast_or_null<Function>(GV);
> +    if (F && F->hasFnAttribute(Attribute::NonLazyBind))
> +      return false;
> +
>      bool IsTLS = GV && GV->isThreadLocal();
>      bool IsAccessViaCopyRelocs =
>          Options.MCOptions.MCPIECopyRelocations && GV && isa<GlobalVariable>(GV);
>
>
> Index: test/CodeGen/X86/no-plt.ll
> ===================================================================
> --- test/CodeGen/X86/no-plt.ll
> +++ test/CodeGen/X86/no-plt.ll
> @@ -6,18 +6,25 @@
>  define i32 @main() #0 {
>  ; X64: callq *_Z3foov at GOTPCREL(%rip)
>  ; X64: callq _Z3barv
> +; X64: callq _Z3bazv
>  
>  entry:
>    %retval = alloca i32, align 4
>    store i32 0, i32* %retval, align 4
>    %call1 = call i32 @_Z3foov()
>    %call2 = call i32 @_Z3barv()
> +  %call3 = call i32 @_Z3bazv()
>    ret i32 0
>  }
>  
>  ; Function Attrs: nonlazybind
>  declare i32 @_Z3foov() #1
>  
>  declare i32 @_Z3barv() #2
>  
> +; Function Attrs: nonlazybind
> +declare hidden i32 @_Z3bazv() #3
> +
> +
>  attributes #1 = { nonlazybind }
> +attributes #3 = { nonlazybind }
> Index: lib/Target/X86/X86Subtarget.cpp
> ===================================================================
> --- lib/Target/X86/X86Subtarget.cpp
> +++ lib/Target/X86/X86Subtarget.cpp
> @@ -144,15 +144,6 @@
>  unsigned char
>  X86Subtarget::classifyGlobalFunctionReference(const GlobalValue *GV,
>                                                const Module &M) const {
> -  const Function *F = dyn_cast_or_null<Function>(GV);
> -
> -  // Do not use the PLT when explicitly told to do so for ELF 64-bit
> -  // target.
> -  if (isTargetELF() && is64Bit() && F &&
> -      F->hasFnAttribute(Attribute::NonLazyBind) &&
> -      GV->isDeclarationForLinker())
> -    return X86II::MO_GOTPCREL;
> -
>    if (TM.shouldAssumeDSOLocal(M, GV))
>      return X86II::MO_NO_FLAG;
>  
> @@ -162,12 +153,16 @@
>      return X86II::MO_DLLIMPORT;
>    }
>  
> +  const Function *F = dyn_cast_or_null<Function>(GV);
> +
>    if (isTargetELF()) {
>      if (is64Bit() && F && (CallingConv::X86_RegCall == F->getCallingConv()))
>        // According to psABI, PLT stub clobbers XMM8-XMM15.
>        // In Regcall calling convention those registers are used for passing
>        // parameters. Thus we need to prevent lazy binding in Regcall.
>        return X86II::MO_GOTPCREL;
> +    if (F && F->hasFnAttribute(Attribute::NonLazyBind) && is64Bit())
> +      return X86II::MO_GOTPCREL;
>      return X86II::MO_PLT;
>    }
>  
> Index: lib/Target/TargetMachine.cpp
> ===================================================================
> --- lib/Target/TargetMachine.cpp
> +++ lib/Target/TargetMachine.cpp
> @@ -167,6 +167,13 @@
>      if (GV && !GV->isDeclarationForLinker())
>        return true;
>  
> +    // A symbol marked nonlazybind should not be accessed with a plt. If the
> +    // symbol turns out to be external, the linker will convert a direct
> +    // access to an access via the plt, so don't assume it is local.
> +    const Function *F = dyn_cast_or_null<Function>(GV);
> +    if (F && F->hasFnAttribute(Attribute::NonLazyBind))
> +      return false;
> +
>      bool IsTLS = GV && GV->isThreadLocal();
>      bool IsAccessViaCopyRelocs =
>          Options.MCOptions.MCPIECopyRelocations && GV && isa<GlobalVariable>(GV);