subject:"\[PATCH\] D43362\: Simplify setting dso

[PATCH] D43362: Simplify setting dso_local. NFC.

2018-02-22 Thread Rafael Avila de Espindola via Phabricator via cfe-commits

espindola closed this revision.
espindola added a comment.

r325846


https://reviews.llvm.org/D43362



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43362: Simplify setting dso_local. NFC.

2018-02-22 Thread Eric Christopher via Phabricator via cfe-commits

echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for the cleanup.


https://reviews.llvm.org/D43362



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D43362: Simplify setting dso_local. NFC.

2018-02-20 Thread Rafael Avila de Espindola via cfe-commits

ping

Rafael Avila de Espindola via Phabricator 
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(D))
> -if (VD->getTLSKind() == VarDecl::TLS_None &&
> +  if (auto *Var = dyn_cast(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(D) && !CGOpts.NoPLT && RM == llvm::Reloc::Static)
> +  if (isa(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
> ==

[PATCH] D43362: Simplify setting dso_local. NFC.

2018-02-15 Thread Rafael Avila de Espindola via Phabricator via cfe-commits

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(D))
-if (VD->getTLSKind() == VarDecl::TLS_None &&
+  if (auto *Var = dyn_cast(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(D) && !CGOpts.NoPLT && RM == llvm::Reloc::Static)
+  if (isa(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 setDSOLo

[PATCH] D43362: Simplify setting dso_local. NFC.

[PATCH] D43362: Simplify setting dso_local. NFC.

Re: [PATCH] D43362: Simplify setting dso_local. NFC.

[PATCH] D43362: Simplify setting dso_local. NFC.

4 matches

Site Navigation

Mail list logo

Footer information