r318866 - Revert "[CodeGen] Fix vtable not receiving hidden visibility when using push(visibility)"

Petr Hosek via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 22 11:50:17 PST 2017


Author: phosek
Date: Wed Nov 22 11:50:17 2017
New Revision: 318866

URL: http://llvm.org/viewvc/llvm-project?rev=318866&view=rev
Log:
Revert "[CodeGen] Fix vtable not receiving hidden visibility when using push(visibility)"

This reverts commit r318853: tests are failing on Windows bots

Removed:
    cfe/trunk/test/CodeGen/push-hidden-visibility-subclass.cpp
Modified:
    cfe/trunk/lib/CodeGen/CGDecl.cpp
    cfe/trunk/lib/CodeGen/CGVTT.cpp
    cfe/trunk/lib/CodeGen/CGVTables.cpp
    cfe/trunk/lib/CodeGen/CodeGenModule.cpp
    cfe/trunk/lib/CodeGen/CodeGenModule.h
    cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=318866&r1=318865&r2=318866&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Wed Nov 22 11:50:17 2017
@@ -240,7 +240,7 @@ llvm::Constant *CodeGenModule::getOrCrea
       getModule(), LTy, Ty.isConstant(getContext()), Linkage, Init, Name,
       nullptr, llvm::GlobalVariable::NotThreadLocal, TargetAS);
   GV->setAlignment(getContext().getDeclAlign(&D).getQuantity());
-  setGlobalVisibility(GV, &D, ForDefinition);
+  setGlobalVisibility(GV, &D);
 
   if (supportsCOMDAT() && GV->isWeakForLinker())
     GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));

Modified: cfe/trunk/lib/CodeGen/CGVTT.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTT.cpp?rev=318866&r1=318865&r2=318866&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGVTT.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTT.cpp Wed Nov 22 11:50:17 2017
@@ -100,7 +100,7 @@ CodeGenVTables::EmitVTTDefinition(llvm::
     VTT->setComdat(CGM.getModule().getOrInsertComdat(VTT->getName()));
 
   // Set the right visibility.
-  CGM.setGlobalVisibility(VTT, RD, ForDefinition);
+  CGM.setGlobalVisibility(VTT, RD);
 }
 
 llvm::GlobalVariable *CodeGenVTables::GetAddrOfVTT(const CXXRecordDecl *RD) {

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=318866&r1=318865&r2=318866&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Wed Nov 22 11:50:17 2017
@@ -51,7 +51,7 @@ llvm::Constant *CodeGenModule::GetAddrOf
 
 static void setThunkVisibility(CodeGenModule &CGM, const CXXMethodDecl *MD,
                                const ThunkInfo &Thunk, llvm::Function *Fn) {
-  CGM.setGlobalVisibility(Fn, MD, ForDefinition);
+  CGM.setGlobalVisibility(Fn, MD);
 }
 
 static void setThunkProperties(CodeGenModule &CGM, const ThunkInfo &Thunk,
@@ -730,7 +730,7 @@ CodeGenVTables::GenerateConstructionVTab
   // Create the variable that will hold the construction vtable.
   llvm::GlobalVariable *VTable =
     CGM.CreateOrReplaceCXXRuntimeVariable(Name, VTType, Linkage);
-  CGM.setGlobalVisibility(VTable, RD, ForDefinition);
+  CGM.setGlobalVisibility(VTable, RD);
 
   // V-tables are always unnamed_addr.
   VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=318866&r1=318865&r2=318866&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Nov 22 11:50:17 2017
@@ -662,8 +662,7 @@ llvm::ConstantInt *CodeGenModule::getSiz
 }
 
 void CodeGenModule::setGlobalVisibility(llvm::GlobalValue *GV,
-                                        const NamedDecl *D,
-                                        ForDefinition_t IsForDefinition) const {
+                                        const NamedDecl *D) const {
   // Internal definitions always have default visibility.
   if (GV->hasLocalLinkage()) {
     GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
@@ -672,8 +671,7 @@ void CodeGenModule::setGlobalVisibility(
 
   // Set visibility for definitions.
   LinkageInfo LV = D->getLinkageAndVisibility();
-  if (LV.isVisibilityExplicit() ||
-      (IsForDefinition && !GV->hasAvailableExternallyLinkage()))
+  if (LV.isVisibilityExplicit() || !GV->hasAvailableExternallyLinkage())
     GV->setVisibility(GetLLVMVisibility(LV.getVisibility()));
 }
 
@@ -1054,7 +1052,7 @@ void CodeGenModule::SetLLVMFunctionAttri
 void CodeGenModule::SetCommonAttributes(const Decl *D,
                                         llvm::GlobalValue *GV) {
   if (const auto *ND = dyn_cast_or_null<NamedDecl>(D))
-    setGlobalVisibility(GV, ND, ForDefinition);
+    setGlobalVisibility(GV, ND);
   else
     GV->setVisibility(llvm::GlobalValue::DefaultVisibility);
 
@@ -1110,8 +1108,8 @@ void CodeGenModule::SetInternalFunctionA
   setNonAliasAttributes(D, F);
 }
 
-static void setLinkageForGV(llvm::GlobalValue *GV,
-                            const NamedDecl *ND) {
+static void setLinkageAndVisibilityForGV(llvm::GlobalValue *GV,
+                                         const NamedDecl *ND) {
   // Set linkage and visibility in case we never see a definition.
   LinkageInfo LV = ND->getLinkageAndVisibility();
   if (!isExternallyVisible(LV.getLinkage())) {
@@ -1127,6 +1125,10 @@ static void setLinkageForGV(llvm::Global
       // separate linkage types for this.
       GV->setLinkage(llvm::GlobalValue::ExternalWeakLinkage);
     }
+
+    // Set visibility on a declaration only if it's explicit.
+    if (LV.isVisibilityExplicit())
+      GV->setVisibility(CodeGenModule::GetLLVMVisibility(LV.getVisibility()));
   }
 }
 
@@ -1195,8 +1197,7 @@ void CodeGenModule::SetFunctionAttribute
   // Only a few attributes are set on declarations; these may later be
   // overridden by a definition.
 
-  setLinkageForGV(F, FD);
-  setGlobalVisibility(F, FD, NotForDefinition);
+  setLinkageAndVisibilityForGV(F, FD);
 
   if (FD->getAttr<PragmaClangTextSectionAttr>()) {
     F->addFnAttr("implicit-section-name");
@@ -2433,8 +2434,7 @@ CodeGenModule::GetOrCreateLLVMGlobal(Str
 
     GV->setAlignment(getContext().getDeclAlign(D).getQuantity());
 
-    setLinkageForGV(GV, D);
-    setGlobalVisibility(GV, D, NotForDefinition);
+    setLinkageAndVisibilityForGV(GV, D);
 
     if (D->getTLSKind()) {
       if (D->getTLSKind() == VarDecl::TLS_Dynamic)
@@ -3253,7 +3253,7 @@ void CodeGenModule::EmitGlobalFunctionDe
   setFunctionDLLStorageClass(GD, Fn);
 
   // FIXME: this is redundant with part of setFunctionDefinitionAttributes
-  setGlobalVisibility(Fn, D, ForDefinition);
+  setGlobalVisibility(Fn, D);
 
   MaybeHandleStaticInExternC(D, Fn);
 
@@ -3849,7 +3849,7 @@ ConstantAddress CodeGenModule::GetAddrOf
       getModule(), Type, Constant, Linkage, InitialValue, Name.c_str(),
       /*InsertBefore=*/nullptr, llvm::GlobalVariable::NotThreadLocal, TargetAS);
   if (emitter) emitter->finalize(GV);
-  setGlobalVisibility(GV, VD, ForDefinition);
+  setGlobalVisibility(GV, VD);
   GV->setAlignment(Align.getQuantity());
   if (supportsCOMDAT() && GV->isWeakForLinker())
     GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=318866&r1=318865&r2=318866&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h Wed Nov 22 11:50:17 2017
@@ -710,8 +710,7 @@ public:
   llvm::ConstantInt *getSize(CharUnits numChars);
 
   /// Set the visibility for the given LLVM GlobalValue.
-  void setGlobalVisibility(llvm::GlobalValue *GV, const NamedDecl *D,
-                           ForDefinition_t IsForDefinition) const;
+  void setGlobalVisibility(llvm::GlobalValue *GV, const NamedDecl *D) const;
 
   /// Set the TLS mode for the given LLVM GlobalValue for the thread-local
   /// variable declaration D.

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=318866&r1=318865&r2=318866&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Wed Nov 22 11:50:17 2017
@@ -1527,7 +1527,7 @@ void ItaniumCXXABI::emitVTableDefinition
     VTable->setComdat(CGM.getModule().getOrInsertComdat(VTable->getName()));
 
   // Set the right visibility.
-  CGM.setGlobalVisibility(VTable, RD, ForDefinition);
+  CGM.setGlobalVisibility(VTable, RD);
 
   // Use pointer alignment for the vtable. Otherwise we would align them based
   // on the size of the initializer which doesn't make sense as only single
@@ -1637,7 +1637,6 @@ llvm::GlobalVariable *ItaniumCXXABI::get
   VTable = CGM.CreateOrReplaceCXXRuntimeVariable(
       Name, VTableType, llvm::GlobalValue::ExternalLinkage);
   VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
-  CGM.setGlobalVisibility(VTable, RD, NotForDefinition);
 
   if (RD->hasAttr<DLLImportAttr>())
     VTable->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);

Removed: cfe/trunk/test/CodeGen/push-hidden-visibility-subclass.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/push-hidden-visibility-subclass.cpp?rev=318865&view=auto
==============================================================================
--- cfe/trunk/test/CodeGen/push-hidden-visibility-subclass.cpp (original)
+++ cfe/trunk/test/CodeGen/push-hidden-visibility-subclass.cpp (removed)
@@ -1,21 +0,0 @@
-// RUN: %clang -S -emit-llvm -o %t %s
-// RUN: FileCheck --input-file=%t %s
-
-#pragma GCC visibility push(hidden)
-
-struct Base {
-  virtual ~Base() = default;
-  virtual void* Alloc() = 0;
-};
-
-class Child : public Base {
-public:
-  Child() = default;
-  void* Alloc();
-};
-
-void test() {
-  Child x;
-}
-
-// CHECK: @_ZTV5Child = external hidden unnamed_addr constant




More information about the cfe-commits mailing list