[PATCH] D43362: Simplify setting dso_local. NFC.

[Rafael Avila de Espindola via cfe-commits]

ping

Rafael Avila de Espindola via Phabricator <reviews at reviews.llvm.org>
writes:

> espindola created this revision.
> espindola added a reviewer: rnk.
>
> The value of dso_local can be computed from just IR properties and global information (object file type, command line options, etc).
>
> With this patch we no longer pass in the Decl. It was almost unused and making it fully unused guarantees that dso_local is consistent with the rest of the IR.
>
>
> https://reviews.llvm.org/D43362
>
> Files:
>   lib/CodeGen/CodeGenModule.cpp
>   lib/CodeGen/CodeGenModule.h
>   lib/CodeGen/ItaniumCXXABI.cpp
>
>
> Index: lib/CodeGen/ItaniumCXXABI.cpp
> ===================================================================
> --- lib/CodeGen/ItaniumCXXABI.cpp
> +++ lib/CodeGen/ItaniumCXXABI.cpp
> @@ -3214,10 +3214,10 @@
>      llvmVisibility = CodeGenModule::GetLLVMVisibility(Ty->getVisibility());
>  
>    TypeName->setVisibility(llvmVisibility);
> -  CGM.setDSOLocal(TypeName, Ty->getAsCXXRecordDecl());
> +  CGM.setDSOLocal(TypeName);
>  
>    GV->setVisibility(llvmVisibility);
> -  CGM.setDSOLocal(GV, Ty->getAsCXXRecordDecl());
> +  CGM.setDSOLocal(GV);
>  
>    if (CGM.getTriple().isWindowsItaniumEnvironment()) {
>      auto RD = Ty->getAsCXXRecordDecl();
> Index: lib/CodeGen/CodeGenModule.h
> ===================================================================
> --- lib/CodeGen/CodeGenModule.h
> +++ lib/CodeGen/CodeGenModule.h
> @@ -721,7 +721,7 @@
>    /// Set the visibility for the given LLVM GlobalValue.
>    void setGlobalVisibility(llvm::GlobalValue *GV, const NamedDecl *D) const;
>  
> -  void setDSOLocal(llvm::GlobalValue *GV, const NamedDecl *D) const;
> +  void setDSOLocal(llvm::GlobalValue *GV) const;
>  
>    void setGVProperties(llvm::GlobalValue *GV, const NamedDecl *D) const;
>  
> Index: lib/CodeGen/CodeGenModule.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenModule.cpp
> +++ lib/CodeGen/CodeGenModule.cpp
> @@ -716,7 +716,7 @@
>  }
>  
>  static bool shouldAssumeDSOLocal(const CodeGenModule &CGM,
> -                                 llvm::GlobalValue *GV, const NamedDecl *D) {
> +                                 llvm::GlobalValue *GV) {
>    const llvm::Triple &TT = CGM.getTriple();
>    // Only handle ELF for now.
>    if (!TT.isOSBinFormatELF())
> @@ -746,31 +746,30 @@
>      return false;
>  
>    // If we can use copy relocations we can assume it is local.
> -  if (auto *VD = dyn_cast<VarDecl>(D))
> -    if (VD->getTLSKind() == VarDecl::TLS_None &&
> +  if (auto *Var = dyn_cast<llvm::GlobalVariable>(GV))
> +    if (!Var->isThreadLocal() &&
>          (RM == llvm::Reloc::Static || CGOpts.PIECopyRelocations))
>        return true;
>  
>    // If we can use a plt entry as the symbol address we can assume it
>    // is local.
>    // FIXME: This should work for PIE, but the gold linker doesn't support it.
> -  if (isa<FunctionDecl>(D) && !CGOpts.NoPLT && RM == llvm::Reloc::Static)
> +  if (isa<llvm::Function>(GV) && !CGOpts.NoPLT && RM == llvm::Reloc::Static)
>      return true;
>  
>    // Otherwise don't assue it is local.
>    return false;
>  }
>  
> -void CodeGenModule::setDSOLocal(llvm::GlobalValue *GV,
> -                                const NamedDecl *D) const {
> -  if (shouldAssumeDSOLocal(*this, GV, D))
> +void CodeGenModule::setDSOLocal(llvm::GlobalValue *GV) const {
> +  if (shouldAssumeDSOLocal(*this, GV))
>      GV->setDSOLocal(true);
>  }
>  
>  void CodeGenModule::setGVProperties(llvm::GlobalValue *GV,
>                                      const NamedDecl *D) const {
>    setGlobalVisibility(GV, D);
> -  setDSOLocal(GV, D);
> +  setDSOLocal(GV);
>  }
>  
>  static llvm::GlobalVariable::ThreadLocalMode GetLLVMTLSModel(StringRef S) {
> @@ -2753,14 +2752,15 @@
>      GV->setAlignment(getContext().getDeclAlign(D).getQuantity());
>  
>      setLinkageForGV(GV, D);
> -    setGVProperties(GV, D);
>  
>      if (D->getTLSKind()) {
>        if (D->getTLSKind() == VarDecl::TLS_Dynamic)
>          CXXThreadLocals.push_back(D);
>        setTLSMode(GV, *D);
>      }
>  
> +    setGVProperties(GV, D);
> +
>      // If required by the ABI, treat declarations of static data members with
>      // inline initializers as definitions.
>      if (getContext().isMSStaticDataMemberInlineDefinition(D)) {
>
>
> Index: lib/CodeGen/ItaniumCXXABI.cpp
> ===================================================================
> --- lib/CodeGen/ItaniumCXXABI.cpp
> +++ lib/CodeGen/ItaniumCXXABI.cpp
> @@ -3214,10 +3214,10 @@
>      llvmVisibility = CodeGenModule::GetLLVMVisibility(Ty->getVisibility());
>  
>    TypeName->setVisibility(llvmVisibility);
> -  CGM.setDSOLocal(TypeName, Ty->getAsCXXRecordDecl());
> +  CGM.setDSOLocal(TypeName);
>  
>    GV->setVisibility(llvmVisibility);
> -  CGM.setDSOLocal(GV, Ty->getAsCXXRecordDecl());
> +  CGM.setDSOLocal(GV);
>  
>    if (CGM.getTriple().isWindowsItaniumEnvironment()) {
>      auto RD = Ty->getAsCXXRecordDecl();
> Index: lib/CodeGen/CodeGenModule.h
> ===================================================================
> --- lib/CodeGen/CodeGenModule.h
> +++ lib/CodeGen/CodeGenModule.h
> @@ -721,7 +721,7 @@
>    /// Set the visibility for the given LLVM GlobalValue.
>    void setGlobalVisibility(llvm::GlobalValue *GV, const NamedDecl *D) const;
>  
> -  void setDSOLocal(llvm::GlobalValue *GV, const NamedDecl *D) const;
> +  void setDSOLocal(llvm::GlobalValue *GV) const;
>  
>    void setGVProperties(llvm::GlobalValue *GV, const NamedDecl *D) const;
>  
> Index: lib/CodeGen/CodeGenModule.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenModule.cpp
> +++ lib/CodeGen/CodeGenModule.cpp
> @@ -716,7 +716,7 @@
>  }
>  
>  static bool shouldAssumeDSOLocal(const CodeGenModule &CGM,
> -                                 llvm::GlobalValue *GV, const NamedDecl *D) {
> +                                 llvm::GlobalValue *GV) {
>    const llvm::Triple &TT = CGM.getTriple();
>    // Only handle ELF for now.
>    if (!TT.isOSBinFormatELF())
> @@ -746,31 +746,30 @@
>      return false;
>  
>    // If we can use copy relocations we can assume it is local.
> -  if (auto *VD = dyn_cast<VarDecl>(D))
> -    if (VD->getTLSKind() == VarDecl::TLS_None &&
> +  if (auto *Var = dyn_cast<llvm::GlobalVariable>(GV))
> +    if (!Var->isThreadLocal() &&
>          (RM == llvm::Reloc::Static || CGOpts.PIECopyRelocations))
>        return true;
>  
>    // If we can use a plt entry as the symbol address we can assume it
>    // is local.
>    // FIXME: This should work for PIE, but the gold linker doesn't support it.
> -  if (isa<FunctionDecl>(D) && !CGOpts.NoPLT && RM == llvm::Reloc::Static)
> +  if (isa<llvm::Function>(GV) && !CGOpts.NoPLT && RM == llvm::Reloc::Static)
>      return true;
>  
>    // Otherwise don't assue it is local.
>    return false;
>  }
>  
> -void CodeGenModule::setDSOLocal(llvm::GlobalValue *GV,
> -                                const NamedDecl *D) const {
> -  if (shouldAssumeDSOLocal(*this, GV, D))
> +void CodeGenModule::setDSOLocal(llvm::GlobalValue *GV) const {
> +  if (shouldAssumeDSOLocal(*this, GV))
>      GV->setDSOLocal(true);
>  }
>  
>  void CodeGenModule::setGVProperties(llvm::GlobalValue *GV,
>                                      const NamedDecl *D) const {
>    setGlobalVisibility(GV, D);
> -  setDSOLocal(GV, D);
> +  setDSOLocal(GV);
>  }
>  
>  static llvm::GlobalVariable::ThreadLocalMode GetLLVMTLSModel(StringRef S) {
> @@ -2753,14 +2752,15 @@
>      GV->setAlignment(getContext().getDeclAlign(D).getQuantity());
>  
>      setLinkageForGV(GV, D);
> -    setGVProperties(GV, D);
>  
>      if (D->getTLSKind()) {
>        if (D->getTLSKind() == VarDecl::TLS_Dynamic)
>          CXXThreadLocals.push_back(D);
>        setTLSMode(GV, *D);
>      }
>  
> +    setGVProperties(GV, D);
> +
>      // If required by the ABI, treat declarations of static data members with
>      // inline initializers as definitions.
>      if (getContext().isMSStaticDataMemberInlineDefinition(D)) {