[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

2022-06-09 Thread David Tenty via Phabricator via cfe-commits 
daltenty marked an inline comment as done.
daltenty added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1228
   GV->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);
-else if (D->hasAttr() && !GV->isDeclarationForLinker())
+else if ((D->hasAttr() ||
+  shouldMapVisibilityToDLLExport(D)) &&

MaskRay wrote:
> Restrict this to AIX/XCOFF.
> 
> Really other binary format users will not need this.
I believe this is now done, via us now doing an early check on the setting of 
the LangOpt (which will always be `None` on non-AIX). No need for an extra 
check on the binary format.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126340/new/

https://reviews.llvm.org/D126340

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

[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

2022-06-09 Thread David Tenty via Phabricator via cfe-commits 
daltenty updated this revision to Diff 435609.
daltenty added a comment.

- Don't try to export internal linkage RTTI types
- Make shouldMapVisibilityToDLLExport inline and provide a short-circuit 
evaluation in the case where no mapping is in effect. This should hopefully 
solve the compile time issues and asserts we are seeing on other platforms, as 
if the option isn't in effect we don't try to do any additional linkage 
computations over what we would normally do. Tested via profiling building 
tramp3d-v4 and our calls to the linkage computer fall back to what we were 
getting before this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126340/new/

https://reviews.llvm.org/D126340

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/mdefault-visibility-export-mapping.c
  clang/test/CodeGenCXX/mdefault-visibility-export-mapping-alias.cpp
  clang/test/CodeGenCXX/mdefault-visibility-export-mapping-rtti.cpp
  clang/test/CodeGenCXX/mdefault-visibility-export-mapping.cpp

Index: clang/test/CodeGenCXX/mdefault-visibility-export-mapping.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/mdefault-visibility-export-mapping.cpp
@@ -0,0 +1,119 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix %s -S -emit-llvm -o - | \
+// RUN:   FileCheck -check-prefixes=CHECK,UNSPECIFIED-DEF,EXPLICIT-DEF %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix %s -mdefault-visibility-export-mapping=none -S -emit-llvm -o - | \
+// RUN:   FileCheck -check-prefixes=CHECK,UNSPECIFIED-DEF,EXPLICIT-DEF %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix %s -mdefault-visibility-export-mapping=explicit -S -emit-llvm -o - | \
+// RUN:   FileCheck -check-prefixes=CHECK,UNSPECIFIED-DEF,EXPLICIT-EXP %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix %s -mdefault-visibility-export-mapping=all -S -emit-llvm -o - | \
+// RUN:   FileCheck -check-prefixes=CHECK,UNSPECIFIED-EXP,EXPLICIT-EXP %s
+
+struct A {};
+
+template 
+class B {
+public:
+  T x;
+  B(T _x) : x(_x) {}
+  ~B() {}
+  void func(T x) {}
+};
+
+template 
+class __attribute__((visibility("default"))) C {
+public:
+  T x;
+  C(T _x) : x(_x) {}
+  ~C() {}
+  void func(T x) {}
+};
+
+class D {
+public:
+  ~D();
+};
+
+D::~D() {}
+
+extern template class B;
+extern template class C;
+
+void func() {
+  B x({});
+  C y({});
+  x.func({});
+  y.func({});
+  B xi(0);
+  C yi(0);
+  xi.func(0);
+  yi.func(0);
+  D z;
+}
+
+// D::~D() (base object destructor)
+// UNSPECIFIED-DEF:  define void @_ZN1DD2Ev(
+// UNSPECIFIED-EXP:  define dllexport void @_ZN1DD2Ev(
+
+// D::~D() (complete object destructor)
+// UNSPECIFIED-DEF:  define void @_ZN1DD1Ev(
+// UNSPECIFIED-EXP:  define dllexport void @_ZN1DD1Ev(
+
+// UNSPECIFIED-DEF: define void @_Z4funcv(
+// UNSPECIFIED-EXP: define dllexport void @_Z4funcv(
+
+// B::B(A) (complete object constructor)
+// UNSPECIFIED-DEF: define linkonce_odr void @_ZN1BI1AEC1ES0_(
+// UNSPECIFIED-EXP: define linkonce_odr dllexport void @_ZN1BI1AEC1ES0_(
+
+// C::C(A) (complete object constructor)
+// EXPLICIT-DEF: define linkonce_odr void @_ZN1CI1AEC1ES0_(
+// EXPLICIT-EXP: define linkonce_odr dllexport void @_ZN1CI1AEC1ES0_(
+
+// B::func(A)
+// UNSPECIFIED-DEF: define linkonce_odr void @_ZN1BI1AE4funcES0_(
+// UNSPECIFIED-EXP: define linkonce_odr dllexport void @_ZN1BI1AE4funcES0_(
+
+// C::func(A)
+// EXPLICIT-DEF: define linkonce_odr void @_ZN1CI1AE4funcES0_(
+// EXPLICIT-EXP: define linkonce_odr dllexport void @_ZN1CI1AE4funcES0_(
+
+// B::B(int) (complete object constructor)
+// CHECK: declare void @_ZN1BIiEC1Ei
+
+// C::C(int) (complete object constructor)
+// CHECK: declare void @_ZN1CIiEC1Ei
+
+// B::func(int)
+// CHECK: declare void @_ZN1BIiE4funcEi
+
+// C::func(int)
+// CHECK: declare void @_ZN1CIiE4funcEi
+
+// C::~C() (complete object destructor)
+// CHECK: declare void @_ZN1CIiED1Ev
+
+// B::~B() (complete object destructor)
+// CHECK: declare void @_ZN1BIiED1Ev
+
+// C::~c() (complete object destructor)
+// EXPLICIT-DEF: define linkonce_odr void @_ZN1CI1AED1Ev(
+// EXPLICIT-EXP: define linkonce_odr dllexport void @_ZN1CI1AED1Ev(
+
+// B::~B() (complete object destructor)
+// UNSPECIFIED-DEF: define linkonce_odr void @_ZN1BI1AED1Ev(
+// UNSPECIFIED-EXP: define linkonce_odr dllexport void @_ZN1BI1AED1Ev(
+
+// B::B(A) (base object constructor)
+// UNSPECIFIED-DEF: define linkonce_odr void @_ZN1BI1AEC2ES0_(
+// UNSPECIFIED-EXP: define linkonce_odr dllexport void @_ZN1BI1AEC2ES0_(
+
+// B::~B() (base object destructor)
+// UNSPECIFIED-DEF: define linkonce_odr void @_ZN1BI1AED2Ev(
+// UNSPECIFIED-EXP: define linkonce_odr dllexport void @_ZN1BI1AED2Ev(
+
+// C::C(A) (base object

[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

2022-06-02 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1228
   GV->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);
-else if (D->hasAttr() && !GV->isDeclarationForLinker())
+else if ((D->hasAttr() ||
+  shouldMapVisibilityToDLLExport(D)) &&

Restrict this to AIX/XCOFF.

Really other binary format users will not need this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126340/new/

https://reviews.llvm.org/D126340

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

[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

2022-06-02 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

When you reland this, please reland 4463bd0f89181234e0cef982e21de2e96038f873 
 at the 
same time. Else, this breaks tests in some build configurations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126340/new/

https://reviews.llvm.org/D126340

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

[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

2022-06-02 Thread Hans Wennborg via Phabricator via cfe-commits 
hans added a comment.

We're hitting an assertion in Chromium due to this:

  llvm/clang/lib/AST/Decl.cpp:1510: clang::LinkageInfo 
clang::LinkageComputer::getLVForDecl(const clang::NamedDecl *, 
clang::LVComputationKind): Assertion `D->getCachedLinkage() == LV.getLinkage()' 
failed.

See https://bugs.chromium.org/p/chromium/issues/detail?id=1331274#c2 for a 
reproducer.

I've reverted this in d42fe9aa84203a8f51b43a901d72fdc39eea69f7 
 in the 
meantime.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126340/new/

https://reviews.llvm.org/D126340

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

[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

2022-06-02 Thread David Tenty via Phabricator via cfe-commits 
daltenty added a comment.

In D126340#3552645 , @nikic wrote:

> It looks like this causes a major compile-time regression in the clang 
> frontend: 
> http://llvm-compile-time-tracker.com/compare.php?from=6232a8f3d61e5856c17e7b314385e9ea8068cdc1=8c8a2679a20f621994fa904bcfc68775e7345edc=instructions
>  For example, tramp3d-v4 is up 5% in `-O0` builds.
>
> Not familiar with this area, but my first guess would be that 
> getLinkageAndVisibility() is actually expensive and you're introducing a ton 
> of calls to it?

@nikic thanks for catching this! Looking into it now, I'll put in a revert if I 
can't get things sorted shortly


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126340/new/

https://reviews.llvm.org/D126340

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

[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

2022-06-02 Thread Nikita Popov via Phabricator via cfe-commits 
nikic added a comment.

It looks like this causes a major compile-time regression in the clang 
frontend: 
http://llvm-compile-time-tracker.com/compare.php?from=6232a8f3d61e5856c17e7b314385e9ea8068cdc1=8c8a2679a20f621994fa904bcfc68775e7345edc=instructions
 For example, tramp3d-v4 is up 5% in `-O0` builds.

Not familiar with this area, but my first guess would be that 
getLinkageAndVisibility() is actually expensive and you're introducing a ton of 
calls to it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126340/new/

https://reviews.llvm.org/D126340

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

[PATCH] D126340: [clang][AIX] add option -mdefault-visibility-export-mapping

2022-06-01 Thread David Tenty via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c8a2679a20f: [clang][AIX] add option 
mdefault-visibility-export-mapping (authored by daltenty).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126340/new/

https://reviews.llvm.org/D126340

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/mdefault-visibility-export-mapping.c
  clang/test/CodeGenCXX/mdefault-visibility-export-mapping-alias.cpp
  clang/test/CodeGenCXX/mdefault-visibility-export-mapping-rtti.cpp
  clang/test/CodeGenCXX/mdefault-visibility-export-mapping.cpp

Index: clang/test/CodeGenCXX/mdefault-visibility-export-mapping.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/mdefault-visibility-export-mapping.cpp
@@ -0,0 +1,119 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix %s -S -emit-llvm -o - | \
+// RUN:   FileCheck -check-prefixes=CHECK,UNSPECIFIED-DEF,EXPLICIT-DEF %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix %s -mdefault-visibility-export-mapping=none -S -emit-llvm -o - | \
+// RUN:   FileCheck -check-prefixes=CHECK,UNSPECIFIED-DEF,EXPLICIT-DEF %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix %s -mdefault-visibility-export-mapping=explicit -S -emit-llvm -o - | \
+// RUN:   FileCheck -check-prefixes=CHECK,UNSPECIFIED-DEF,EXPLICIT-EXP %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix %s -mdefault-visibility-export-mapping=all -S -emit-llvm -o - | \
+// RUN:   FileCheck -check-prefixes=CHECK,UNSPECIFIED-EXP,EXPLICIT-EXP %s
+
+struct A {};
+
+template 
+class B {
+public:
+  T x;
+  B(T _x) : x(_x) {}
+  ~B() {}
+  void func(T x) {}
+};
+
+template 
+class __attribute__((visibility("default"))) C {
+public:
+  T x;
+  C(T _x) : x(_x) {}
+  ~C() {}
+  void func(T x) {}
+};
+
+class D {
+public:
+  ~D();
+};
+
+D::~D() {}
+
+extern template class B;
+extern template class C;
+
+void func() {
+  B x({});
+  C y({});
+  x.func({});
+  y.func({});
+  B xi(0);
+  C yi(0);
+  xi.func(0);
+  yi.func(0);
+  D z;
+}
+
+// D::~D() (base object destructor)
+// UNSPECIFIED-DEF:  define void @_ZN1DD2Ev(
+// UNSPECIFIED-EXP:  define dllexport void @_ZN1DD2Ev(
+
+// D::~D() (complete object destructor)
+// UNSPECIFIED-DEF:  define void @_ZN1DD1Ev(
+// UNSPECIFIED-EXP:  define dllexport void @_ZN1DD1Ev(
+
+// UNSPECIFIED-DEF: define void @_Z4funcv(
+// UNSPECIFIED-EXP: define dllexport void @_Z4funcv(
+
+// B::B(A) (complete object constructor)
+// UNSPECIFIED-DEF: define linkonce_odr void @_ZN1BI1AEC1ES0_(
+// UNSPECIFIED-EXP: define linkonce_odr dllexport void @_ZN1BI1AEC1ES0_(
+
+// C::C(A) (complete object constructor)
+// EXPLICIT-DEF: define linkonce_odr void @_ZN1CI1AEC1ES0_(
+// EXPLICIT-EXP: define linkonce_odr dllexport void @_ZN1CI1AEC1ES0_(
+
+// B::func(A)
+// UNSPECIFIED-DEF: define linkonce_odr void @_ZN1BI1AE4funcES0_(
+// UNSPECIFIED-EXP: define linkonce_odr dllexport void @_ZN1BI1AE4funcES0_(
+
+// C::func(A)
+// EXPLICIT-DEF: define linkonce_odr void @_ZN1CI1AE4funcES0_(
+// EXPLICIT-EXP: define linkonce_odr dllexport void @_ZN1CI1AE4funcES0_(
+
+// B::B(int) (complete object constructor)
+// CHECK: declare void @_ZN1BIiEC1Ei
+
+// C::C(int) (complete object constructor)
+// CHECK: declare void @_ZN1CIiEC1Ei
+
+// B::func(int)
+// CHECK: declare void @_ZN1BIiE4funcEi
+
+// C::func(int)
+// CHECK: declare void @_ZN1CIiE4funcEi
+
+// C::~C() (complete object destructor)
+// CHECK: declare void @_ZN1CIiED1Ev
+
+// B::~B() (complete object destructor)
+// CHECK: declare void @_ZN1BIiED1Ev
+
+// C::~c() (complete object destructor)
+// EXPLICIT-DEF: define linkonce_odr void @_ZN1CI1AED1Ev(
+// EXPLICIT-EXP: define linkonce_odr dllexport void @_ZN1CI1AED1Ev(
+
+// B::~B() (complete object destructor)
+// UNSPECIFIED-DEF: define linkonce_odr void @_ZN1BI1AED1Ev(
+// UNSPECIFIED-EXP: define linkonce_odr dllexport void @_ZN1BI1AED1Ev(
+
+// B::B(A) (base object constructor)
+// UNSPECIFIED-DEF: define linkonce_odr void @_ZN1BI1AEC2ES0_(
+// UNSPECIFIED-EXP: define linkonce_odr dllexport void @_ZN1BI1AEC2ES0_(
+
+// B::~B() (base object destructor)
+// UNSPECIFIED-DEF: define linkonce_odr void @_ZN1BI1AED2Ev(
+// UNSPECIFIED-EXP: define linkonce_odr dllexport void @_ZN1BI1AED2Ev(
+
+// C::C(A) (base object constructor)
+// EXPLICIT-DEF: define linkonce_odr void @_ZN1CI1AEC2ES0_(
+// EXPLICIT-EXP: define linkonce_odr dllexport void @_ZN1CI1AEC2ES0_(
+
+// C::~C() (base object destructor)
+// EXPLICIT-DEF: define linkonce_odr void @_ZN1CI1AED2Ev(
+// EXPLICIT-EXP: define linkonce_odr dllexport void @_ZN1CI1AED2Ev(