[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346069: Add /Zc:DllexportInlines option to clang-cl
(authored by tikuta, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51340?vs=172348=172485#toc
Repository:
rC Clang
https://reviews.llvm.org/D51340
Files:
include/clang/Basic/Attr.td
include/clang/Basic/LangOptions.def
include/clang/Driver/CC1Options.td
include/clang/Driver/CLCompatOptions.td
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclCXX.cpp
test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
test/Driver/cl-options.c
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11930,14 +11930,40 @@
assert(VD->isStaticLocal());
auto *FD = dyn_cast_or_null(VD->getParentFunctionOrMethod());
+
+ // Find outermost function when VD is in lambda function.
+ while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
+ !FD->hasAttr()) {
+FD = dyn_cast_or_null(FD->getParentFunctionOrMethod());
+ }
+
if (!FD)
return;
// Static locals inherit dll attributes from their function.
if (Attr *A = getDLLAttr(FD)) {
auto *NewAttr = cast(A->clone(getASTContext()));
NewAttr->setInherited(true);
VD->addAttr(NewAttr);
+ } else if (Attr *A = FD->getAttr()) {
+auto *NewAttr = ::new (getASTContext()) DLLExportAttr(A->getRange(),
+ getASTContext(),
+ A->getSpellingListIndex());
+NewAttr->setInherited(true);
+VD->addAttr(NewAttr);
+
+// Export this function to enforce exporting this static variable even
+// if it is not used in this compilation unit.
+if (!FD->hasAttr())
+ FD->addAttr(NewAttr);
+
+ } else if (Attr *A = FD->getAttr()) {
+auto *NewAttr = ::new (getASTContext()) DLLImportAttr(A->getRange(),
+ getASTContext(),
+ A->getSpellingListIndex());
+NewAttr->setInherited(true);
+VD->addAttr(NewAttr);
}
}
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -5707,8 +5707,28 @@
continue;
if (!getDLLAttr(Member)) {
- auto *NewAttr =
- cast(ClassAttr->clone(getASTContext()));
+ InheritableAttr *NewAttr = nullptr;
+
+ // Do not export/import inline function when -fno-dllexport-inlines is
+ // passed. But add attribute for later local static var check.
+ if (!getLangOpts().DllExportInlines && MD && MD->isInlined() &&
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
+ NewAttr = ::new (getASTContext())
+DLLExportStaticLocalAttr(ClassAttr->getRange(),
+ getASTContext(),
+ ClassAttr->getSpellingListIndex());
+} else {
+ NewAttr = ::new (getASTContext())
+DLLImportStaticLocalAttr(ClassAttr->getRange(),
+ getASTContext(),
+ ClassAttr->getSpellingListIndex());
+}
+ } else {
+NewAttr = cast(ClassAttr->clone(getASTContext()));
+ }
+
NewAttr->setInherited(true);
Member->addAttr(NewAttr);
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2179,6 +2179,9 @@
}
}
+ if (Args.hasArg(OPT_fno_dllexport_inlines))
+Opts.DllExportInlines = false;
+
if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) {
StringRef Name = A->getValue();
if (Name == "full" || Name == "branch") {
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5504,6 +5504,11 @@
if (VolatileOptionID == options::OPT__SLASH_volatile_ms)
CmdArgs.push_back("-fms-volatile");
+ if (Args.hasFlag(options::OPT__SLASH_Zc_dllexportInlines_,
+ options::OPT__SLASH_Zc_dllexportInlines,
+ false))
+CmdArgs.push_back("-fno-dllexport-inlines");
+
Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
if (MostGeneralArg && BestCaseArg)
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -2683,6 +2683,17 @@
let
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. Thank you! I'll submit this if other people does not have other comments. Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:9 +// RUN: -emit-llvm -O1 -o - | \ +// RUN: FileCheck --check-prefix=CHECK %s + hans wrote: > Did the "EXPORTINLINE" check-prefix get lost here? Good catch! https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 172348.
takuto.ikuta marked an inline comment as done.
takuto.ikuta added a comment.
Fix check-prefix
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -490,6 +490,11 @@
// RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
// ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
+// RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
+// NoDllExportInlines: "-fno-dllexport-inlines"
+// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
+// DllExportInlines-NOT: "-fno-dllexport-inlines"
+
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=EXPORTINLINE %s
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// CHECK-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutoflineDefFunc@ExportedClass@@QEAAXXZ"
+ void OutoflineDefFunc();
+};
+
+void ExportedClass::OutoflineDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void ExportedClassUser() {
+ ExportedClass a;
+ a.InclassDefFunc();
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+
+ int InclassDefFuncWithStaticVariable() {
+static int static_x = 0;
+return ++static_x;
+ }
+};
+
+class A11{};
+class B22{};
+
+// CHECK-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11QEAAXXZ"
+// CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ"
+// CHECK-DAG: @"?static_x@?2??InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+template class TemplateExportedClass;
+
+// NOEXPORTINLINE-DAG:
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Assuming the test still passes when you put the EXPORTINLINE filecheck prefix back, looks good to me! Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:9 +// RUN: -emit-llvm -O1 -o - | \ +// RUN: FileCheck --check-prefix=CHECK %s + Did the "EXPORTINLINE" check-prefix get lost here? https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. Thank you for quick review! Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5715 + (MD->isThisDeclarationADefinition() || + MD->isImplicitlyInstantiable()) && + TSK != TSK_ExplicitInstantiationDeclaration && hans wrote: > The isThisDeclarationADefinition() and isImplicitlyInstantiable() checks > don't look right. Just checking isInlined() should be enough. I see. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 172332.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -490,6 +490,11 @@
// RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
// ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
+// RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
+// NoDllExportInlines: "-fno-dllexport-inlines"
+// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
+// DllExportInlines-NOT: "-fno-dllexport-inlines"
+
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK %s
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// CHECK-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutoflineDefFunc@ExportedClass@@QEAAXXZ"
+ void OutoflineDefFunc();
+};
+
+void ExportedClass::OutoflineDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void ExportedClassUser() {
+ ExportedClass a;
+ a.InclassDefFunc();
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+
+ int InclassDefFuncWithStaticVariable() {
+static int static_x = 0;
+return ++static_x;
+ }
+};
+
+class A11{};
+class B22{};
+
+// CHECK-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11QEAAXXZ"
+// CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ"
+// CHECK-DAG: @"?static_x@?2??InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+template class TemplateExportedClass;
+
+// NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@?$TemplateExportedClass@VB22QEAAXXZ"
+// EXPORTINLINE-DAG: define
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 172331.
takuto.ikuta added a comment.
added a few more check
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -490,6 +490,11 @@
// RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
// ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
+// RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
+// NoDllExportInlines: "-fno-dllexport-inlines"
+// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
+// DllExportInlines-NOT: "-fno-dllexport-inlines"
+
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK %s
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// CHECK-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutoflineDefFunc@ExportedClass@@QEAAXXZ"
+ void OutoflineDefFunc();
+};
+
+void ExportedClass::OutoflineDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void ExportedClassUser() {
+ ExportedClass a;
+ a.InclassDefFunc();
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_x@?2??InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+// CHECK-DAG: @"?static_x@?2??InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VB22QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_x = 0;
+return ++static_x;
+ }
+};
+
+class A11{};
+class B22{};
+
+// CHECK-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11QEAAXXZ"
+// CHECK-DAG: define weak_odr dso_local dllexport i32
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 172330.
takuto.ikuta marked 10 inline comments as done.
takuto.ikuta added a comment.
Address comment
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -490,6 +490,11 @@
// RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
// ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
+// RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
+// NoDllExportInlines: "-fno-dllexport-inlines"
+// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
+// DllExportInlines-NOT: "-fno-dllexport-inlines"
+
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,130 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK %s
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// CHECK-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutoflineDefFunc@ExportedClass@@QEAAXXZ"
+ void OutoflineDefFunc();
+};
+
+void ExportedClass::OutoflineDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void ExportedClassUser() {
+ ExportedClass a;
+ a.InclassDefFunc();
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+
+ int InclassDefFuncWithStaticVariable() {
+// CHECK-DAG: @"?static_x@?2??InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_x = 0;
+return ++static_x;
+ }
+};
+
+class A11{};
+class B22{};
+
+// CHECK-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11QEAAXXZ"
+// CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@?$TemplateExportedClass@VA11QEAAHXZ"
+template class TemplateExportedClass;
+
+// NOEXPORTINLINE-DAG: define linkonce_odr
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added a comment.
Thanks! I don't think the new isThisDeclarationADefinition() and
isImplicitlyInstantiable() checks are right. Besides that, I only have a few
more comments about the test.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5715
+ (MD->isThisDeclarationADefinition() ||
+ MD->isImplicitlyInstantiable()) &&
+ TSK != TSK_ExplicitInstantiationDeclaration &&
The isThisDeclarationADefinition() and isImplicitlyInstantiable() checks don't
look right. Just checking isInlined() should be enough.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:9
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=STATIC %s
+
Why this separate STATIC invocation? It looks just the same as the invocation
above.
Oh I see, it's because the test is mixing -DAG and -NOT lines, which breaks
things.
I have a suggestion for avoiding the -NOT lines below, and that means this
separate invocation to check for statics (and the other one below) can be
removed.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:25
+
+ // NOEXPORTINLINE-NOT: ?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void
@"?InclassDefFunc@ExportedClass@@
Instead of a -NOT check, it would be better to "use" the function somewhere so
that it's emitted, and check that it's not dllexport. That will make it easier
to see the difference between NOEXPORTINLINE and EXPORTINLINE.
Also it avoids -NOT checks, which don't interact well with -DAG checks, and
would let you remove the separate STATIC invocation.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:44
+ // CHECK-NOT: InlineOutclassDefFuncWihtoutDefinition
+ inline void InlineOutclassDefFuncWihtoutDefinition();
+
Having an inline function and not defining it like this is not so great,
especially in a dllexport class. I don't think there's any need to change the
behaviour here or test for it.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:47
+ // CHECK-DAG:define weak_odr dso_local dllexport void
@"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
But with -fno-dllexport-inlines, I don't think it should be exported.
It really doesn't matter if the body is inside or outside the class -- it's
still an inline member function, and so the flag should apply.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:53
+ // CHECK-DAG: define dso_local dllexport void
@"?OutclassDefFunc@ExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
I'd suggest calling it "OutoflineDefFunc()" instead.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:65
+
+void __declspec(dllexport) ExportedClassUser() {
+ ExportedClass a;
I don't think there's any reason to dllexport the "user" function.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:81
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void
@"?InclassDefFunc@?$TemplateExportedClass@VA11AEAAXXZ"
+template class __declspec(dllexport) TemplateExportedClass;
+
I don't think this instantiation is different from the `template class
TemplateExportedClass;` one below, so I think we can skip it.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:83
+
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void
@"?InclassDefFunc@?$TemplateExportedClass@VB22AEAAXXZ"
+template class TemplateExportedClass;
There should also be a check to make sure it's exported also in the
NOEXPORTINLINE case.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:88
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void
@"?InclassDefFunc@?$TemplateExportedClass@VC33QEAAXXZ
+TemplateExportedClass c33;
+
Thanks, this is the implicit instantiation case. It would be good to have an
inline function with a static local to make sure it does get exported even with
the flag.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. I missed the comment about driver flag test. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 172317.
takuto.ikuta added a comment.
Added cl driver flag test
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
clang/test/Driver/cl-options.c
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -490,6 +490,11 @@
// RUN: %clang_cl /Zc:threadSafeInit /c -### -- %s 2>&1 | FileCheck -check-prefix=ThreadSafeStatics %s
// ThreadSafeStatics-NOT: "-fno-threadsafe-statics"
+// RUN: %clang_cl /Zc:dllexportInlines- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoDllExportInlines %s
+// NoDllExportInlines: "-fno-dllexport-inlines"
+// RUN: %clang_cl /Zc:dllexportInlines /c -### -- %s 2>&1 | FileCheck -check-prefix=DllExportInlines %s
+// DllExportInlines-NOT: "-fno-dllexport-inlines"
+
// RUN: %clang_cl /Zi /c -### -- %s 2>&1 | FileCheck -check-prefix=Zi %s
// Zi: "-gcodeview"
// Zi: "-debug-info-kind=limited"
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,142 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=STATIC %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=STATIC %s
+
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-NOT: ?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// STATIC-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// STATIC-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // CHECK-NOT: InlineOutclassDefFuncWihtoutDefinition
+ inline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // CHECK-DAG:define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutclassDefFunc@ExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void ExportedClass::OutclassDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) ExportedClassUser() {
+ ExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+};
+
+class A11{};
+class B22{};
+class C33{};
+class D44{};
+
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
Hans, thank you for review! I addressed all your comment and fixed small
behavior.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:76
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
hans wrote:
> Hmm, I would have thought we should export this function.
I see.
Added `MD->isThisDeclarationADefinition() ` check in
`Sema::checkClassLevelDLLAttribute`
This will make change like
https://chromium-review.googlesource.com/c/v8/v8/+/1186017 unnecessary.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:122
+template class TemplateExportedClass;
+
+
hans wrote:
> We should also have a test with implicit instantiation, and then the inline
> function should not be exported when using /Zc:dllexportInlines-.
Added test and `MD->isImplicitlyInstantiable()` check for the test.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:172
+ // INLINE-DAG: declare dllimport i32
@"?InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ"(%class.ImportedClass*)
#2
+ // NOINLINE-NOT:
declare{{.*}}"?InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ"
+ int InClassDefFuncWithStaticVariable() {
hans wrote:
> This check looks wrong. I think there should be a dllimport declaration for
> this function? (Or an available_externally definition if we use -O1?)
I think it is OK not dllimporting this function as far as static variable is
dllimported and definition of this function is emitted.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 172304.
takuto.ikuta marked 17 inline comments as done.
takuto.ikuta added a comment.
Simplify test and fix some behavior.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,142 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=CHECK --check-prefix=NOEXPORTINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -fno-dllexport-inlines -emit-llvm -O1 -o - |\
+// RUN: FileCheck --check-prefix=STATIC %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=CHECK %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -disable-llvm-passes\
+// RUN: -emit-llvm -O1 -o - | \
+// RUN: FileCheck --check-prefix=STATIC %s
+
+
+
+struct __declspec(dllexport) ExportedClass {
+
+ // NOEXPORTINLINE-NOT: ?InclassDefFunc@ExportedClass@@
+ // EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@ExportedClass@@
+ void InclassDefFunc() {}
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+// STATIC-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+// STATIC-DAG: @"?static_x@?2???R@?0??InclassDefFunctWithLambdaStaticVariable@ExportedClass@@QEAAHXZ@QEBA?A?@@XZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // CHECK-NOT: InlineOutclassDefFuncWihtoutDefinition
+ inline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // CHECK-DAG:define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@ExportedClass@@QEAAXXZ
+ inline void InlineOutclassDefFunc();
+
+ // CHECK-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@ExportedClass@@QEAAHXZ"
+ inline int InlineOutclassDefFuncWithStaticVariable();
+
+ // CHECK-DAG: define dso_local dllexport void @"?OutclassDefFunc@ExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void ExportedClass::OutclassDefFunc() {}
+
+inline void ExportedClass::InlineOutclassDefFunc() {}
+
+inline int ExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) ExportedClassUser() {
+ ExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+struct __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+};
+
+class A11{};
+class B22{};
+class C33{};
+class D44{};
+
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11AEAAXXZ"
+template class __declspec(dllexport) TemplateExportedClass;
+
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VB22AEAAXXZ"
+template class TemplateExportedClass;
+
+// NOEXPORTINLINE-DAG: define linkonce_odr dso_local void @"?InclassDefFunc@?$TemplateExportedClass@VC33QEAAXXZ"
+// EXPORTINLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VC33QEAAXXZ
+TemplateExportedClass c33;
+
+void c33user() {
+ c33.InclassDefFunc();
+}
+
+
+template
+struct TemplateNoExportedClass {
+ void InclassDefFunc() {}
+ int InclassDefFuncWithStaticLocal() {
+static int static_x;
+return ++static_x;
+ }
+};
+
+// CHECK-DAG: define weak_odr dso_local dllexport
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments. Comment at: clang/include/clang/Driver/CLCompatOptions.td:336 HelpText<"Disable precompiled headers, overrides /Yc and /Yu">; +def _SLASH_Zc_dllexportInlines : CLFlag<"Zc:dllexportInlines">; +def _SLASH_Zc_dllexportInlines_ : CLFlag<"Zc:dllexportInlines-">; Just one more small thing I remembered: please add a test in tests/Driver/cl-options.c to make sure we translate this to the cc1 flag correctly. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added a comment.
Thank you Takuto! I think the functionality looks great now: it's not too
complicated :-)
I only have comments about the test.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:3
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
Instead of "DEFAULT", I think "CHECK" is the default prefix to use when using
the same prefix for multiple invocations.
Also, instead of INLINE and NOINLINE (which sounds like it's about inlining),
I'd suggest perhaps "EXPORTINLINES" and "NOEXPORTINLINES" or something like
that.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:6
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
Instead of using -O0 here and above, consider using -O1 -disable-llvm-passes so
we get available_externally functions.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:9
+
+// Function
+
This change doesn't have any effect on non-member functions, so I don't think
we need these tests.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:42
+
+// check for local static variables
+// NOINLINE-DAG:
@"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA"
= weak_odr dso_local dllexport global i32 0, comdat, align 4
Can you move the checks down to where the variable/function is defined? That
makes it easier to read the test I think.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:49
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
I think this test case should be first in the file, because it's really showing
the core of this feature.
I would suggest just calling "ExportedClass" to make it simpler (if it was a
template, then we'd put template in the name). Also, I suggest making it a
struct instead so you don't have to write "public:".
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:51
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
This check is only using part of the mangled name..
But I don't think the patch does anything special for this kind of function, so
I'd just skip this test.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:58
+
+ int f();
+
This doesn't seem to be used for anything?
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:73
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
Why __foceinline? I think it should be just "inline". This goes for all use of
__forceinline in this file.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:76
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
Hmm, I would have thought we should export this function.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:102
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
I'm not sure that OutclassDefFunc() adds much value to the test. Also
templateValue below. I think they can be removed.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:122
+template class TemplateExportedClass;
+
+
We should also have a test with implicit instantiation, and then the inline
function should not be exported when using /Zc:dllexportInlines-.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:132
+ }
+ void OutclassDefFunc();
+};
Again, I think we can drop the out-of-line function since this change should
not affect those.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:144
+// DEFAULT-DAG: define weak_odr dso_local void
@"?OutclassDefFunc@?$TemplateNoExportedClass@VB22QEAAXXZ"
+template class TemplateNoExportedClass;
+
This case is not interesting, since it has nothing to do with dllexport/import,
I think it can be removed.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:148
+// DEFAULT-NOT: ?OutclassDefFunc@?$TemplateNoExportedClass@VC33@
+extern template class TemplateNoExportedClass;
+
Ditto.
Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:165
+class
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. Thank you for review! Comment at: clang/include/clang/Basic/LangOptions.h:246 + /// If set, dllexported classes dllexport their inline methods. + bool DllExportInlines = true; + rnk wrote: > We should define this in the LangOptions.def file. Moved. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 172090.
takuto.ikuta marked 2 inline comments as done.
takuto.ikuta added a comment.
Added option to LangOptions.def
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+
+// DEFAULT-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
rnk added inline comments. Comment at: clang/include/clang/Basic/LangOptions.h:246 + /// If set, dllexported classes dllexport their inline methods. + bool DllExportInlines = true; + We should define this in the LangOptions.def file. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
rnk added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > takuto.ikuta wrote:
> > > > > takuto.ikuta wrote:
> > > > > > hans wrote:
> > > > > > > Can you give an example for why this is needed?
> > > > > > Sorry, this change does not need. Removed.
> > > > > Sorry, this change is necessary still.
> > > > >
> > > > > Without this, definition of inline function in explicit template
> > > > > instantiation declaration is not be emitted, due to
> > > > > GVA_AvailableExternally linkage.
> > > > > But we stop exporting definition of inline function in explicit
> > > > > template instantiation definition too.
> > > > >
> > > > > So without this, definition of dllimported inline function of
> > > > > explicit template instantiation declaration won't be available.
> > > > >
> > > > Can you provide a code example of why this is needed?
> > > If we don't change function linkage, following code will be compiled like
> > > below with -fno-dllexport-inlines.
> > >
> > > ```
> > > template
> > > class M{
> > > public:
> > > void foo() {}
> > > };
> > >
> > > template class __declspec(dllexport) M;
> > >
> > > extern template class __declspec(dllimport) M;
> > >
> > > void f (){
> > > M mi;
> > > mi.foo();
> > >
> > > M ms;
> > > ms.foo();
> > > }
> > > ```
> > >
> > > ```
> > > $"?foo@?$M@H@@QEAAXXZ" = comdat any
> > >
> > > ; Function Attrs: noinline nounwind optnone
> > > define weak_odr dso_local void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %this) #0
> > > comdat align 2 {
> > > entry:
> > > %this.addr = alloca %class.M*, align 8
> > > store %class.M* %this, %class.M** %this.addr, align 8
> > > %this1 = load %class.M*, %class.M** %this.addr, align 8
> > > ret void
> > > }
> > >
> > > ; Function Attrs: noinline nounwind optnone
> > > define dso_local void @"?f@@YAXXZ"() #0 {
> > > entry:
> > > %mi = alloca %class.M, align 1
> > > %ms = alloca %class.M.0, align 1
> > > call void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %mi)
> > > call void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0* %ms)
> > > ret void
> > > }
> > >
> > > declare dso_local void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0*) #1
> > > ```
> > >
> > > M::foo() is declared, but inline function is not dllexported (See
> > > M::foo() is not dllexported). So this code cannot be linked because
> > > definition of M::foo does not exist. If the function is properly
> > > inlined, we don't need to have this code. But I'm not sure why the
> > > function is not inlined.
> > Interesting. I wonder how -fvisibility-inlines-hidden handles this...
> >
> >
> > ```
> > template struct S {
> > void foo() {}
> > };
> >
> > template struct S;
> >
> > void use() {
> > S s;
> > s.foo();
> > }
> >
> > $ g++ -fvisibility-inlines-hidden -c a.cc && objdump -t a.o | grep
> > _ZN1SIiE3fooEv
> > ld .text._ZN1SIiE3fooEv
> > .text._ZN1SIiE3fooEv
> > wF .text._ZN1SIiE3fooEv 000b
> > _ZN1SIiE3fooEv <--- Not hidden.
> > ```
> >
> > (If I comment out the explicit instantiation definition above, foo() is
> > hidden as expected.)
> >
> > Okay, it seems that for explicit instantiation definitions,
> > -fvisibility-inlines-hidden does not apply.
> >
> > And thinking more about it, that makes sense.
> >
> > I don't think we should change the linkage like this though, I think we
> > should just not apply the /Zc:dllexportInlines- for explicit instantiation
> > decls and definitions in checkClassLevelDLLAttribute(). I realize you had
> > code to check for this before, but now we have a good and well understood
> > reason.
> Thank you for confirmation. I revived the check.
I agree, this option should not affect explicit instantiations. Inline
functions from explicit instantiations should still be exported+weak_odr, and
imported+available_externally for extern declarations.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta marked an inline comment as done.
takuto.ikuta added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > takuto.ikuta wrote:
> > > > takuto.ikuta wrote:
> > > > > hans wrote:
> > > > > > Can you give an example for why this is needed?
> > > > > Sorry, this change does not need. Removed.
> > > > Sorry, this change is necessary still.
> > > >
> > > > Without this, definition of inline function in explicit template
> > > > instantiation declaration is not be emitted, due to
> > > > GVA_AvailableExternally linkage.
> > > > But we stop exporting definition of inline function in explicit
> > > > template instantiation definition too.
> > > >
> > > > So without this, definition of dllimported inline function of explicit
> > > > template instantiation declaration won't be available.
> > > >
> > > Can you provide a code example of why this is needed?
> > If we don't change function linkage, following code will be compiled like
> > below with -fno-dllexport-inlines.
> >
> > ```
> > template
> > class M{
> > public:
> > void foo() {}
> > };
> >
> > template class __declspec(dllexport) M;
> >
> > extern template class __declspec(dllimport) M;
> >
> > void f (){
> > M mi;
> > mi.foo();
> >
> > M ms;
> > ms.foo();
> > }
> > ```
> >
> > ```
> > $"?foo@?$M@H@@QEAAXXZ" = comdat any
> >
> > ; Function Attrs: noinline nounwind optnone
> > define weak_odr dso_local void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %this) #0
> > comdat align 2 {
> > entry:
> > %this.addr = alloca %class.M*, align 8
> > store %class.M* %this, %class.M** %this.addr, align 8
> > %this1 = load %class.M*, %class.M** %this.addr, align 8
> > ret void
> > }
> >
> > ; Function Attrs: noinline nounwind optnone
> > define dso_local void @"?f@@YAXXZ"() #0 {
> > entry:
> > %mi = alloca %class.M, align 1
> > %ms = alloca %class.M.0, align 1
> > call void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %mi)
> > call void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0* %ms)
> > ret void
> > }
> >
> > declare dso_local void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0*) #1
> > ```
> >
> > M::foo() is declared, but inline function is not dllexported (See
> > M::foo() is not dllexported). So this code cannot be linked because
> > definition of M::foo does not exist. If the function is properly
> > inlined, we don't need to have this code. But I'm not sure why the function
> > is not inlined.
> Interesting. I wonder how -fvisibility-inlines-hidden handles this...
>
>
> ```
> template struct S {
> void foo() {}
> };
>
> template struct S;
>
> void use() {
> S s;
> s.foo();
> }
>
> $ g++ -fvisibility-inlines-hidden -c a.cc && objdump -t a.o | grep
> _ZN1SIiE3fooEv
> ld .text._ZN1SIiE3fooEv
> .text._ZN1SIiE3fooEv
> wF .text._ZN1SIiE3fooEv 000b _ZN1SIiE3fooEv
> <--- Not hidden.
> ```
>
> (If I comment out the explicit instantiation definition above, foo() is
> hidden as expected.)
>
> Okay, it seems that for explicit instantiation definitions,
> -fvisibility-inlines-hidden does not apply.
>
> And thinking more about it, that makes sense.
>
> I don't think we should change the linkage like this though, I think we
> should just not apply the /Zc:dllexportInlines- for explicit instantiation
> decls and definitions in checkClassLevelDLLAttribute(). I realize you had
> code to check for this before, but now we have a good and well understood
> reason.
Thank you for confirmation. I revived the check.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 171916.
takuto.ikuta added a comment.
export/import explicit template instantiation function
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+
+// DEFAULT-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > takuto.ikuta wrote:
> > > > hans wrote:
> > > > > Can you give an example for why this is needed?
> > > > Sorry, this change does not need. Removed.
> > > Sorry, this change is necessary still.
> > >
> > > Without this, definition of inline function in explicit template
> > > instantiation declaration is not be emitted, due to
> > > GVA_AvailableExternally linkage.
> > > But we stop exporting definition of inline function in explicit template
> > > instantiation definition too.
> > >
> > > So without this, definition of dllimported inline function of explicit
> > > template instantiation declaration won't be available.
> > >
> > Can you provide a code example of why this is needed?
> If we don't change function linkage, following code will be compiled like
> below with -fno-dllexport-inlines.
>
> ```
> template
> class M{
> public:
> void foo() {}
> };
>
> template class __declspec(dllexport) M;
>
> extern template class __declspec(dllimport) M;
>
> void f (){
> M mi;
> mi.foo();
>
> M ms;
> ms.foo();
> }
> ```
>
> ```
> $"?foo@?$M@H@@QEAAXXZ" = comdat any
>
> ; Function Attrs: noinline nounwind optnone
> define weak_odr dso_local void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %this) #0
> comdat align 2 {
> entry:
> %this.addr = alloca %class.M*, align 8
> store %class.M* %this, %class.M** %this.addr, align 8
> %this1 = load %class.M*, %class.M** %this.addr, align 8
> ret void
> }
>
> ; Function Attrs: noinline nounwind optnone
> define dso_local void @"?f@@YAXXZ"() #0 {
> entry:
> %mi = alloca %class.M, align 1
> %ms = alloca %class.M.0, align 1
> call void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %mi)
> call void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0* %ms)
> ret void
> }
>
> declare dso_local void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0*) #1
> ```
>
> M::foo() is declared, but inline function is not dllexported (See
> M::foo() is not dllexported). So this code cannot be linked because
> definition of M::foo does not exist. If the function is properly
> inlined, we don't need to have this code. But I'm not sure why the function
> is not inlined.
Interesting. I wonder how -fvisibility-inlines-hidden handles this...
```
template struct S {
void foo() {}
};
template struct S;
void use() {
S s;
s.foo();
}
$ g++ -fvisibility-inlines-hidden -c a.cc && objdump -t a.o | grep
_ZN1SIiE3fooEv
ld .text._ZN1SIiE3fooEv
.text._ZN1SIiE3fooEv
wF .text._ZN1SIiE3fooEv 000b _ZN1SIiE3fooEv
<--- Not hidden.
```
(If I comment out the explicit instantiation definition above, foo() is hidden
as expected.)
Okay, it seems that for explicit instantiation definitions,
-fvisibility-inlines-hidden does not apply.
And thinking more about it, that makes sense.
I don't think we should change the linkage like this though, I think we should
just not apply the /Zc:dllexportInlines- for explicit instantiation decls and
definitions in checkClassLevelDLLAttribute(). I realize you had code to check
for this before, but now we have a good and well understood reason.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
hans wrote:
> takuto.ikuta wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > Can you give an example for why this is needed?
> > > Sorry, this change does not need. Removed.
> > Sorry, this change is necessary still.
> >
> > Without this, definition of inline function in explicit template
> > instantiation declaration is not be emitted, due to GVA_AvailableExternally
> > linkage.
> > But we stop exporting definition of inline function in explicit template
> > instantiation definition too.
> >
> > So without this, definition of dllimported inline function of explicit
> > template instantiation declaration won't be available.
> >
> Can you provide a code example of why this is needed?
If we don't change function linkage, following code will be compiled like below
with -fno-dllexport-inlines.
```
template
class M{
public:
void foo() {}
};
template class __declspec(dllexport) M;
extern template class __declspec(dllimport) M;
void f (){
M mi;
mi.foo();
M ms;
ms.foo();
}
```
```
$"?foo@?$M@H@@QEAAXXZ" = comdat any
; Function Attrs: noinline nounwind optnone
define weak_odr dso_local void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %this) #0
comdat align 2 {
entry:
%this.addr = alloca %class.M*, align 8
store %class.M* %this, %class.M** %this.addr, align 8
%this1 = load %class.M*, %class.M** %this.addr, align 8
ret void
}
; Function Attrs: noinline nounwind optnone
define dso_local void @"?f@@YAXXZ"() #0 {
entry:
%mi = alloca %class.M, align 1
%ms = alloca %class.M.0, align 1
call void @"?foo@?$M@H@@QEAAXXZ"(%class.M* %mi)
call void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0* %ms)
ret void
}
declare dso_local void @"?foo@?$M@F@@QEAAXXZ"(%class.M.0*) #1
```
M::foo() is declared, but inline function is not dllexported (See
M::foo() is not dllexported). So this code cannot be linked because
definition of M::foo does not exist. If the function is properly
inlined, we don't need to have this code. But I'm not sure why the function is
not inlined.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:9552 +// overwrite linkage of explicit template instantiation +// definition/declaration. +return GVA_DiscardableODR; takuto.ikuta wrote: > takuto.ikuta wrote: > > hans wrote: > > > Can you give an example for why this is needed? > > Sorry, this change does not need. Removed. > Sorry, this change is necessary still. > > Without this, definition of inline function in explicit template > instantiation declaration is not be emitted, due to GVA_AvailableExternally > linkage. > But we stop exporting definition of inline function in explicit template > instantiation definition too. > > So without this, definition of dllimported inline function of explicit > template instantiation declaration won't be available. > Can you provide a code example of why this is needed? https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
Thank you for quick fix!
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
takuto.ikuta wrote:
> hans wrote:
> > Can you give an example for why this is needed?
> Sorry, this change does not need. Removed.
Sorry, this change is necessary still.
Without this, definition of inline function in explicit template instantiation
declaration is not be emitted, due to GVA_AvailableExternally linkage.
But we stop exporting definition of inline function in explicit template
instantiation definition too.
So without this, definition of dllimported inline function of explicit template
instantiation declaration won't be available.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
hans wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > takuto.ikuta wrote:
> > > > > takuto.ikuta wrote:
> > > > > > takuto.ikuta wrote:
> > > > > > > hans wrote:
> > > > > > > > takuto.ikuta wrote:
> > > > > > > > > hans wrote:
> > > > > > > > > > I still don't understand why we need these checks for
> > > > > > > > > > template instantiations. Why does it matter whether the
> > > > > > > > > > functions get inlined or not?
> > > > > > > > > When function of dllimported class is not inlined, such
> > > > > > > > > funtion needs to be dllexported from implementation library.
> > > > > > > > >
> > > > > > > > > c.h
> > > > > > > > > ```
> > > > > > > > > template
> > > > > > > > > class EXPORT C {
> > > > > > > > > public:
> > > > > > > > > void f() {}
> > > > > > > > > };
> > > > > > > > > ```
> > > > > > > > >
> > > > > > > > > cuser.cc
> > > > > > > > > ```
> > > > > > > > > #include "c.h"
> > > > > > > > >
> > > > > > > > > void cuser() {
> > > > > > > > > C c;
> > > > > > > > > c.f(); // This function may not be inlined when EXPORT is
> > > > > > > > > __declspec(dllimport), so link may fail.
> > > > > > > > > }
> > > > > > > > > ```
> > > > > > > > >
> > > > > > > > > When cuser.cc and c.h are built to different library,
> > > > > > > > > cuser.cc needs to be able to see dllexported C::f() when
> > > > > > > > > C::f() is not inlined.
> > > > > > > > > This is my understanding.
> > > > > > > > Your example doesn't use explicit instantiation definition or
> > > > > > > > declaration, so doesn't apply here I think.
> > > > > > > >
> > > > > > > > As long as the dllexport and dllimport attributes matches it's
> > > > > > > > fine. It doesn't matter whether the function gets inlined or
> > > > > > > > not, the only thing that matters is that if it's marked
> > > > > > > > dllimport on the "consumer side", it must be dllexport when
> > > > > > > > building the dll.
> > > > > > > Hmm, I changed the linkage for functions having
> > > > > > > DLLExport/ImportStaticLocal Attr.
> > > > > > >
> > > > > > I changed linkage in ASTContext so that inline function is emitted
> > > > > > when it is necessary when we use fno-dllexport-inlines.
> > > > > I revived explicit template instantiation check.
> > > > > Found that this is necessary.
> > > > >
> > > > > For explicit template instantiation, inheriting dll attribute from
> > > > > function for local static var is run before inheriting dll attribute
> > > > > from class for member functions. So I cannot detect local static var
> > > > > of inline function after class level dll attribute processing for
> > > > > explicit template instantiation.
> > > > >
> > > > Oh I see, it's a static local problem..
> > > > Can you provide a concrete example that does not work without your
> > > > check?
> > > > Maybe this is the right thing to do, but I would like to understand
> > > > exactly what the problem is.
> > > For the following code
> > > ```
> > > template
> > > class M{
> > > public:
> > >
> > > T Inclass() {
> > > static T static_x;
> > > ++static_x;
> > > return static_x;
> > > }
> > > };
> > >
> > > template class __declspec(dllexport) M;
> > >
> > > extern template class __declspec(dllimport) M;
> > >
> > > int f (){
> > > M mi;
> > > M ms;
> > > return mi.Inclass() + ms.Inclass();
> > > }
> > >
> > > ```
> > >
> > > llvm code without instantiation check become like below. Both inline
> > > functions of M and M is not dllimported/exported.
> > > ```
> > > $"?Inclass@?$M@H@@QEAAHXZ" = comdat any
> > >
> > > $"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = comdat any
> > >
> > > @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = linkonce_odr dso_local
> > > global i32 0, comdat, align 4
> > >
> > > ; Function Attrs: noinline nounwind optnone
> > > define weak_odr dso_local i32 @"?Inclass@?$M@H@@QEAAHXZ"(%class.M* %this)
> > > #0 comdat align 2 {
> > >
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 171877.
takuto.ikuta added a comment.
Rebased to take r345699
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,199 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+
+// DEFAULT-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VA11QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_x@?2??InclassDefFuncWithStaticLocal@?$TemplateNoExportedClass@VD44QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > takuto.ikuta wrote:
> > > > takuto.ikuta wrote:
> > > > > takuto.ikuta wrote:
> > > > > > hans wrote:
> > > > > > > takuto.ikuta wrote:
> > > > > > > > hans wrote:
> > > > > > > > > I still don't understand why we need these checks for
> > > > > > > > > template instantiations. Why does it matter whether the
> > > > > > > > > functions get inlined or not?
> > > > > > > > When function of dllimported class is not inlined, such funtion
> > > > > > > > needs to be dllexported from implementation library.
> > > > > > > >
> > > > > > > > c.h
> > > > > > > > ```
> > > > > > > > template
> > > > > > > > class EXPORT C {
> > > > > > > > public:
> > > > > > > > void f() {}
> > > > > > > > };
> > > > > > > > ```
> > > > > > > >
> > > > > > > > cuser.cc
> > > > > > > > ```
> > > > > > > > #include "c.h"
> > > > > > > >
> > > > > > > > void cuser() {
> > > > > > > > C c;
> > > > > > > > c.f(); // This function may not be inlined when EXPORT is
> > > > > > > > __declspec(dllimport), so link may fail.
> > > > > > > > }
> > > > > > > > ```
> > > > > > > >
> > > > > > > > When cuser.cc and c.h are built to different library, cuser.cc
> > > > > > > > needs to be able to see dllexported C::f() when C::f() is not
> > > > > > > > inlined.
> > > > > > > > This is my understanding.
> > > > > > > Your example doesn't use explicit instantiation definition or
> > > > > > > declaration, so doesn't apply here I think.
> > > > > > >
> > > > > > > As long as the dllexport and dllimport attributes matches it's
> > > > > > > fine. It doesn't matter whether the function gets inlined or not,
> > > > > > > the only thing that matters is that if it's marked dllimport on
> > > > > > > the "consumer side", it must be dllexport when building the dll.
> > > > > > Hmm, I changed the linkage for functions having
> > > > > > DLLExport/ImportStaticLocal Attr.
> > > > > >
> > > > > I changed linkage in ASTContext so that inline function is emitted
> > > > > when it is necessary when we use fno-dllexport-inlines.
> > > > I revived explicit template instantiation check.
> > > > Found that this is necessary.
> > > >
> > > > For explicit template instantiation, inheriting dll attribute from
> > > > function for local static var is run before inheriting dll attribute
> > > > from class for member functions. So I cannot detect local static var of
> > > > inline function after class level dll attribute processing for explicit
> > > > template instantiation.
> > > >
> > > Oh I see, it's a static local problem..
> > > Can you provide a concrete example that does not work without your check?
> > > Maybe this is the right thing to do, but I would like to understand
> > > exactly what the problem is.
> > For the following code
> > ```
> > template
> > class M{
> > public:
> >
> > T Inclass() {
> > static T static_x;
> > ++static_x;
> > return static_x;
> > }
> > };
> >
> > template class __declspec(dllexport) M;
> >
> > extern template class __declspec(dllimport) M;
> >
> > int f (){
> > M mi;
> > M ms;
> > return mi.Inclass() + ms.Inclass();
> > }
> >
> > ```
> >
> > llvm code without instantiation check become like below. Both inline
> > functions of M and M is not dllimported/exported.
> > ```
> > $"?Inclass@?$M@H@@QEAAHXZ" = comdat any
> >
> > $"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = comdat any
> >
> > @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = linkonce_odr dso_local global
> > i32 0, comdat, align 4
> >
> > ; Function Attrs: noinline nounwind optnone
> > define weak_odr dso_local i32 @"?Inclass@?$M@H@@QEAAHXZ"(%class.M* %this)
> > #0 comdat align 2 {
> > entry:
> > %this.addr = alloca %class.M*, align 8
> > store %class.M* %this, %class.M** %this.addr, align 8
> > %this1 = load %class.M*, %class.M** %this.addr, align 8
> > %0 = load i32, i32* @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA", align 4
> > %inc = add nsw i32 %0, 1
> > store i32 %inc, i32* @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA", align 4
> > %1 = load i32, i32* @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA", align 4
> > ret i32 %1
> > }
> >
> > ; Function Attrs: noinline nounwind optnone
> > define dso_local i32 @"?f@@YAHXZ"() #0 {
> > entry:
> > %mi = alloca %class.M, align 1
> > %ms = alloca %class.M.0, align 1
> > %call = call i32 @"?Inclass@?$M@H@@QEAAHXZ"(%class.M* %mi)
> > %call1 = call i16 @"?Inclass@?$M@F@@QEAAFXZ"(%class.M.0* %ms)
> > %conv = sext i16 %call1 to i32
> > %add = add nsw i32 %call, %conv
> > ret i32 %add
> > }
> >
> > declare dso_local i16 @"?Inclass@?$M@F@@QEAAFXZ"(%class.M.0*) #1
> > ```
> >
> >
> > With the check, both functions are dllimported/exported and
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added a comment.
>> $ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o
>> lib.so lib.cc
>> $ g++ main.cc lib.so
>> /tmp/cc557J3i.o: In function `S::bar()':
>> main.cc:(.text._ZN1S3barEv[_ZN1S3barEv]+0xd): undefined reference to
>> `foo()'
>>
>>
>> So this is a known issue with `-fvisibility-inlines-hidden`. This doesn't
>> come up often in practice, but when it does the developer needs to deal with
>> it.
>
> Yeah, that is the reason of few chromium code changes.
> https://chromium-review.googlesource.com/c/chromium/src/+/1212379
Ah, thanks! I hadn't seen what the fixes would look like.
>> However, the static local problem is much scarier, because that leads to
>> run-time bugs:
>
> Currently this CL doesn't take care of inline function that is not member of
> a class.
>
> `lib.h`:
>
> #ifndef LIB_H
> #define LIB_H
>
> struct __attribute__((visibility("default"))) S {
> int bar() {
> static int x = 0; return x++;
> }
> int baz();
> };
>
> #endif
>
>
> `lib.cc`:
>
> #include "lib.h"
>
> int S::baz() {
> return bar();
> }
>
>
> Then, static local in inline function is treated correctly.
I think it's possible to get the same problem with member functions, but that
doesn't matter, it's an existing problem so we don't need to solve it, just be
aware.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > takuto.ikuta wrote:
> > > > takuto.ikuta wrote:
> > > > > hans wrote:
> > > > > > takuto.ikuta wrote:
> > > > > > > hans wrote:
> > > > > > > > I still don't understand why we need these checks for template
> > > > > > > > instantiations. Why does it matter whether the functions get
> > > > > > > > inlined or not?
> > > > > > > When function of dllimported class is not inlined, such funtion
> > > > > > > needs to be dllexported from implementation library.
> > > > > > >
> > > > > > > c.h
> > > > > > > ```
> > > > > > > template
> > > > > > > class EXPORT C {
> > > > > > > public:
> > > > > > > void f() {}
> > > > > > > };
> > > > > > > ```
> > > > > > >
> > > > > > > cuser.cc
> > > > > > > ```
> > > > > > > #include "c.h"
> > > > > > >
> > > > > > > void cuser() {
> > > > > > > C c;
> > > > > > > c.f(); // This function may not be inlined when EXPORT is
> > > > > > > __declspec(dllimport), so link may fail.
> > > > > > > }
> > > > > > > ```
> > > > > > >
> > > > > > > When cuser.cc and c.h are built to different library, cuser.cc
> > > > > > > needs to be able to see dllexported C::f() when C::f() is not
> > > > > > > inlined.
> > > > > > > This is my understanding.
> > > > > > Your example doesn't use explicit instantiation definition or
> > > > > > declaration, so doesn't apply here I think.
> > > > > >
> > > > > > As long as the dllexport and dllimport attributes matches it's
> > > > > > fine. It doesn't matter whether the function gets inlined or not,
> > > > > > the only thing that matters is that if it's marked dllimport on the
> > > > > > "consumer side", it must be dllexport when building the dll.
> > > > > Hmm, I changed the linkage for functions having
> > > > > DLLExport/ImportStaticLocal Attr.
> > > > >
> > > > I changed linkage in ASTContext so that inline function is emitted when
> > > > it is necessary when we use fno-dllexport-inlines.
> > > I revived explicit template instantiation check.
> > > Found that this is necessary.
> > >
> > > For explicit template instantiation, inheriting dll attribute from
> > > function for local static var is run before inheriting dll attribute from
> > > class for member functions. So I cannot detect local static var of inline
> > > function after class level dll attribute processing for explicit template
> > > instantiation.
> > >
> > Oh I see, it's a static local problem..
> > Can you provide a concrete example that does not work without your check?
> > Maybe this is the right thing to do, but I would like to understand exactly
> > what the problem is.
> For the following code
> ```
> template
> class M{
> public:
>
> T Inclass() {
> static T static_x;
> ++static_x;
> return static_x;
> }
> };
>
> template class __declspec(dllexport) M;
>
> extern template class __declspec(dllimport) M;
>
> int f (){
> M mi;
> M ms;
> return mi.Inclass() + ms.Inclass();
> }
>
> ```
>
> llvm code without instantiation check become like below. Both inline
> functions of M and M is not dllimported/exported.
> ```
> $"?Inclass@?$M@H@@QEAAHXZ" = comdat any
>
> $"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = comdat any
>
> @"?static_x@?2??Inclass@?$M@H@@QEAAHXZ@4HA" = linkonce_odr dso_local global
> i32 0, comdat, align 4
>
> ; Function Attrs: noinline nounwind optnone
> define
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
In https://reviews.llvm.org/D51340#1278799, @hans wrote:
> I've been thinking more about your example with static locals in lambda and
> how this works with regular dllexport.
>
> It got me thinking more about the problem of exposing inline functions from a
> library. For example:
>
> `lib.h`:
>
> #ifndef LIB_H
> #define LIB_H
>
> int foo();
>
> struct __declspec(dllimport) S {
> int bar() { return foo(); }
> };
>
> #endif
>
>
>
> `lib.cc`:
>
> #include "lib.h"
>
> int foo() { return 123; }
>
>
> `main.cc`:
>
> #include "lib.h"
>
> int main() {
> S s;
> return s.bar();
> }
>
>
> Here, Clang will not emit the body of `S::bar()`, because it references the
> non-dllimport function `foo()`, and trying to referencing `foo()` outside the
> library would result in a link error. This is what the
> `DLLImportFunctionVisitor` is used for. For the same reason, MSVC will also
> not inline dllimport functions.
>
> Now, with `/Zc:dllexportInlines-`, we no longer mark `S::bar()` dllimport,
> and so we do emit it, causing that link error. The same problem happens with
> `-fvisibility-inlines-hidden`: substitute the `declspec` above for
> `__attribute__((visibility("default")))` above and try it:
>
> $ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o
> lib.so lib.cc
> $ g++ main.cc lib.so
> /tmp/cc557J3i.o: In function `S::bar()':
> main.cc:(.text._ZN1S3barEv[_ZN1S3barEv]+0xd): undefined reference to `foo()'
>
>
> So this is a known issue with `-fvisibility-inlines-hidden`. This doesn't
> come up often in practice, but when it does the developer needs to deal with
> it.
Yeah, that is the reason of few chromium code changes.
https://chromium-review.googlesource.com/c/chromium/src/+/1212379
> However, the static local problem is much scarier, because that leads to
> run-time bugs:
>
> `lib.h`:
>
> #ifndef LIB_H
> #define LIB_H
>
> inline int foo() { static int x = 0; return x++; }
>
> struct __attribute__((visibility("default"))) S {
> int bar() { return foo(); }
> int baz();
> };
>
> #endif
>
>
> `lib.cc`:
>
> #include "lib.h"
>
> int S::baz() { return foo(); }
>
>
> `main.cc`:
>
> #include
> #include "lib.h"
>
> int main() {
> S s;
> printf("s.bar(): %d\n", s.bar());
> printf("s.baz(): %d\n", s.baz());
> return 0;
> }
>
>
> If we build the program normally, we get the expected output:
>
> $ g++ lib.cc main.cc && ./a.out
> s.bar(): 0
> s.baz(): 1
>
>
> but building as a shared library shows the problem:
>
> $ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o
> lib.so lib.cc
> $ g++ main.cc lib.so
> $ LD_LIBRARY_PATH=$PWD:$LD_LIBRARY_PATH ./a.out
> s.bar(): 0
> s.baz(): 0
>
>
> Oops.
>
> This does show that it's a pre-existing problem with the model of not
> exporting inline functions though. I don't think we need to solve this
> specifically for Windows, I think we should match what
> `-fvisibility-inlines-hidden` is doing.
Currently this CL doesn't take care of inline function that is not member of a
class.
`lib.h`:
#ifndef LIB_H
#define LIB_H
struct __attribute__((visibility("default"))) S {
int bar() {
static int x = 0; return x++;
}
int baz();
};
#endif
`lib.cc`:
#include "lib.h"
int S::baz() {
return bar();
}
Then, static local in inline function is treated correctly.
$ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o lib.so
lib.cc
$ g++ main.cc lib.so
$ LD_LIBRARY_PATH=$PWD:$LD_LIBRARY_PATH ./a.out
s.bar(): 0
s.baz(): 1
This is the same behavior with `/Zc:dllexportInlines-`.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
hans wrote:
> takuto.ikuta wrote:
> > takuto.ikuta wrote:
> > > takuto.ikuta wrote:
> > > > hans wrote:
> > > > > takuto.ikuta wrote:
> > > > > > hans wrote:
> > > > > > > I still don't understand why we need these checks for template
> > > > > > > instantiations. Why does it matter whether the functions get
> > > > > > > inlined or not?
> > > > > > When function of dllimported class is not inlined, such funtion
> > > > > > needs to be dllexported from implementation library.
> > > > > >
> > > > > > c.h
> > > > > > ```
> > > > > > template
> > > > > > class EXPORT C {
> > > > > > public:
> > > > > > void f() {}
> > > > > > };
> > > > > > ```
> > > > > >
> > > > > > cuser.cc
> > > > > > ```
> > > > > > #include "c.h"
> > > > > >
> > > > > > void cuser() {
> > > > > > C c;
> > > > > > c.f(); // This function may not be inlined when EXPORT is
> > > > > > __declspec(dllimport), so link may fail.
> > > > > > }
> > > > > > ```
> > > > > >
> > > > > > When cuser.cc and c.h are built to
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
takuto.ikuta wrote:
> takuto.ikuta wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > takuto.ikuta wrote:
> > > > > hans wrote:
> > > > > > I still don't understand why we need these checks for template
> > > > > > instantiations. Why does it matter whether the functions get
> > > > > > inlined or not?
> > > > > When function of dllimported class is not inlined, such funtion needs
> > > > > to be dllexported from implementation library.
> > > > >
> > > > > c.h
> > > > > ```
> > > > > template
> > > > > class EXPORT C {
> > > > > public:
> > > > > void f() {}
> > > > > };
> > > > > ```
> > > > >
> > > > > cuser.cc
> > > > > ```
> > > > > #include "c.h"
> > > > >
> > > > > void cuser() {
> > > > > C c;
> > > > > c.f(); // This function may not be inlined when EXPORT is
> > > > > __declspec(dllimport), so link may fail.
> > > > > }
> > > > > ```
> > > > >
> > > > > When cuser.cc and c.h are built to different library, cuser.cc needs
> > > > > to be able to see dllexported C::f() when C::f() is not inlined.
> > > > > This is my understanding.
> > > > Your example doesn't use explicit instantiation definition or
> > > > declaration, so doesn't apply here I think.
> > > >
> > > > As long as the dllexport and dllimport attributes matches it's fine. It
> > > > doesn't matter whether the function gets inlined or not, the only thing
> > > > that matters is that if it's marked dllimport on the "consumer side",
> > > > it must be dllexport when building the dll.
> > > Hmm, I changed the linkage for functions having
> > > DLLExport/ImportStaticLocal Attr.
> > >
> > I changed linkage in ASTContext so that inline function is emitted when it
> > is necessary when we use fno-dllexport-inlines.
> I revived explicit template instantiation check.
> Found that this is necessary.
>
> For explicit template instantiation, inheriting dll attribute from function
> for local static var is run before inheriting dll attribute from class for
> member functions. So I cannot detect local static var of inline function
> after class level dll attribute processing for explicit template
> instantiation.
>
Oh I see, it's a static local problem..
Can you provide a concrete example that does not work without your check?
Maybe this is the right thing to do, but I would like to understand exactly
what the problem is.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added a comment.
I've been thinking more about your example with static locals in lambda and how
this works with regular dllexport.
It got me thinking more about the problem of exposing inline functions from a
library. For example:
`lib.h`:
#ifndef LIB_H
#define LIB_H
int foo();
struct __declspec(dllimport) S {
int bar() { return foo(); }
};
#endif
`lib.cc`:
#include "lib.h"
int foo() { return 123; }
`main.cc`:
#include "lib.h"
int main() {
S s;
return s.bar();
}
Here, Clang will not emit the body of `S::bar()`, because it references the
non-dllimport function `foo()`, and trying to referencing `foo()` outside the
library would result in a link error. This is what the
`DLLImportFunctionVisitor` is used for. For the same reason, MSVC will also not
inline dllimport functions.
Now, with `/Zc:dllexportInlines-`, we no longer mark `S::bar()` dllimport, and
so we do emit it, causing that link error. The same problem happens with
`-fvisibility-inlines-hidden`: substitute the `declspec` above for
`__attribute__((visibility("default")))` above and try it:
$ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o lib.so
lib.cc
$ g++ main.cc lib.so
/tmp/cc557J3i.o: In function `S::bar()':
main.cc:(.text._ZN1S3barEv[_ZN1S3barEv]+0xd): undefined reference to `foo()'
So this is a known issue with `-fvisibility-inlines-hidden`. This doesn't come
up often in practice, but when it does the developer needs to deal with it.
However, the static local problem is much scarier, because that leads to
run-time bugs:
`lib.h`:
#ifndef LIB_H
#define LIB_H
inline int foo() { static int x = 0; return x++; }
struct __attribute__((visibility("default"))) S {
int bar() { return foo(); }
int baz();
};
#endif
`lib.cc`:
#include "lib.h"
int S::baz() { return foo(); }
`main.cc`:
#include
#include "lib.h"
int main() {
S s;
printf("s.bar(): %d\n", s.bar());
printf("s.baz(): %d\n", s.baz());
return 0;
}
If we build the program normally, we get the expected output:
$ g++ lib.cc main.cc && ./a.out
s.bar(): 0
s.baz(): 1
but building as a shared library shows the problem:
$ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o lib.so
lib.cc
$ g++ main.cc lib.so
$ LD_LIBRARY_PATH=$PWD:$LD_LIBRARY_PATH ./a.out
s.bar(): 0
s.baz(): 0
Oops.
This does show that it's a pre-existing problem with the model of not exporting
inline functions though. I don't think we need to solve this specifically for
Windows, I think we should match what `-fvisibility-inlines-hidden` is doing.
And what about the lambdas?
`lib.h`:
#ifndef LIB_H
#define LIB_H
struct __attribute__((visibility("default"))) S {
int bar() { return ([]() { static int x; return x++; })(); }
int baz();
};
#endif
`lib.cc`:
#include "lib.h"
int S::baz() { return bar(); }
$ g++ -fvisibility=hidden -fvisibility-inlines-hidden -fpic -shared -o lib.so
lib.cc && g++ main.cc lib.so && LD_LIBRARY_PATH=$PWD:$LD_LIBRARY_PATH ./a.out
s.bar(): 0
s.baz(): 1
Interesting, the lambda function and its static local are not hidden.
Either that's because they're applying the "static locals of hidden functions
in visible decl contexts are visible" thing. Or alternatively, they never
applied the hidden visibility to the lambda in the first place.
I'm not entirely sure what this means for the dllexport case though. Maybe the
loop in the current patch is fine.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 171466.
takuto.ikuta marked an inline comment as done.
takuto.ikuta added a comment.
Added explanation comment for added attributes and rebased
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,181 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
hans wrote:
> Can you give an example for why this is needed?
Sorry, this change does not need. Removed.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
takuto.ikuta wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > takuto.ikuta wrote:
> > > > hans wrote:
> > > > > I still don't understand why we need these checks for template
> > > > > instantiations. Why does it matter whether the functions get inlined
> > > > > or not?
> > > > When function of dllimported class is not inlined, such funtion needs
> > > > to be dllexported from implementation library.
> > > >
> > > > c.h
> > > > ```
> > > > template
> > > > class EXPORT C {
> > > > public:
> > > > void f() {}
> > > > };
> > > > ```
> > > >
> > > > cuser.cc
> > > > ```
> > > > #include "c.h"
> > > >
> > > > void cuser() {
> > > > C c;
> > > > c.f(); // This function may not be inlined when EXPORT is
> > > > __declspec(dllimport), so link may fail.
> > > > }
> > > > ```
> > > >
> > > > When cuser.cc and c.h are built to different library, cuser.cc needs to
> > > > be able to see dllexported C::f() when C::f() is not inlined.
> > > > This is my understanding.
> > > Your example doesn't use explicit instantiation definition or
> > > declaration, so doesn't apply here I think.
> > >
> > > As long as the dllexport and dllimport attributes matches it's fine. It
> > > doesn't matter whether the function gets inlined or not, the only thing
> > > that matters is that if it's marked dllimport on the "consumer side", it
> > > must be dllexport when building the dll.
> > Hmm, I changed the linkage for functions having DLLExport/ImportStaticLocal
> > Attr.
> >
> I changed linkage in ASTContext so that inline function is emitted when it is
> necessary when we use fno-dllexport-inlines.
I revived explicit template instantiation check.
Found that this is necessary.
For explicit template instantiation, inheriting dll attribute from function for
local static var is run before inheriting dll attribute from class for member
functions. So I cannot detect local static var of inline function after class
level dll attribute processing for explicit template instantiation.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added a comment.
In https://reviews.llvm.org/D51340#1266312, @takuto.ikuta wrote:
> Hans, I addressed all your comments.
> How do you think about current implementation?
Just a few questions, but I think it's pretty good.
Comment at: clang/include/clang/Basic/Attr.td:2688
+ // This attribute is used internally only when -fno-dllexport-inlines is
+ // passed.
+ let Spellings = [];
The comment should explain what the attribute does, also the dllimport one
below.
Comment at: clang/lib/AST/ASTContext.cpp:9552
+// overwrite linkage of explicit template instantiation
+// definition/declaration.
+return GVA_DiscardableODR;
Can you give an example for why this is needed?
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > takuto.ikuta wrote:
> > > > > hans wrote:
> > > > > > Why does this need to be a loop? I don't think FunctionDecl's can
> > > > > > be nested?
> > > > > This is for static local var in lambda function.
> > > > > static_x's ParentFunction does not become f().
> > > > > ```
> > > > > class __declspec(dllexport) C {
> > > > > int f() {
> > > > > return ([]() { static int static_x; return ++static_x; })();
> > > > > }
> > > > > };
> > > > > ```
> > > > I don't think the lambda (or its static local) should be exported in
> > > > this case.
> > > If we don't export static_x, dllimport library cannot find static_x when
> > > linking?
> > >
> > >
> > I believe even if C is marked dllimport, the lambda will not be dllimport.
> > The inline definition will be used.
> Do you say importing/implementation library should have different instance of
> static_x here?
> Current clang does not generate such obj.
>
> I think static_x should be dllimported. But without this loop, static_x
> cannot find FD of C::f() having DLLImport/ExportStaticLocalAttr and static_x
> won't be treated as imported/exported variable.
>
> And if static_x is not exported, importing library and implementation library
> will not have the same instance of static_x when C::f() is inlined.
Look at what MSVC does:
```
>type a.cc
struct __declspec(dllexport) S {
int f() {
static int y;
return ([]() { static int static_x; return ++static_x; })() + y;
}
};
void use(S *s) { s->f(); }
>cl -c a.cc && dumpbin /directives a.obj
Microsoft (R) C/C++ Optimizing Compiler Version 19.12.25834 for x86
Copyright (C) Microsoft Corporation. All rights reserved.
a.cc
Microsoft (R) COFF/PE Dumper Version 14.12.25834.0
Copyright (C) Microsoft Corporation. All rights reserved.
Dump of file a.obj
File Type: COFF OBJECT
Linker Directives
-
/DEFAULTLIB:LIBCMT
/DEFAULTLIB:OLDNAMES
/EXPORT:?f@S@@QAEHXZ
/EXPORT:??4S@@QAEAAU0@ABU0@@Z
/EXPORT:??4S@@QAEAAU0@$$QAU0@@Z
/EXPORT:?y@?1??f@S@@QAEHXZ@4HA,DATA
Summary
8 .bss
50 .chks64
64 .debug$S
A6 .drectve
6A .text$mn
```
As you can see S::f and S::f::y is exported, but the lambda and its static
local are not.
Oh, but I guess you're asking what if we don't dllexport S::f anymore because
we're not dllexporting inline methods anymore... hmm, yes, then I do suppose it
needs to be exported so maybe this is right after all.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. ping? Can I go forward in this way? https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
Herald added a subscriber: nhaehnle.
Hans, I addressed all your comments.
How do you think about current implementation?
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > I still don't understand why we need these checks for template
> > > > instantiations. Why does it matter whether the functions get inlined or
> > > > not?
> > > When function of dllimported class is not inlined, such funtion needs to
> > > be dllexported from implementation library.
> > >
> > > c.h
> > > ```
> > > template
> > > class EXPORT C {
> > > public:
> > > void f() {}
> > > };
> > > ```
> > >
> > > cuser.cc
> > > ```
> > > #include "c.h"
> > >
> > > void cuser() {
> > > C c;
> > > c.f(); // This function may not be inlined when EXPORT is
> > > __declspec(dllimport), so link may fail.
> > > }
> > > ```
> > >
> > > When cuser.cc and c.h are built to different library, cuser.cc needs to
> > > be able to see dllexported C::f() when C::f() is not inlined.
> > > This is my understanding.
> > Your example doesn't use explicit instantiation definition or declaration,
> > so doesn't apply here I think.
> >
> > As long as the dllexport and dllimport attributes matches it's fine. It
> > doesn't matter whether the function gets inlined or not, the only thing
> > that matters is that if it's marked dllimport on the "consumer side", it
> > must be dllexport when building the dll.
> Hmm, I changed the linkage for functions having DLLExport/ImportStaticLocal
> Attr.
>
I changed linkage in ASTContext so that inline function is emitted when it is
necessary when we use fno-dllexport-inlines.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 169792.
takuto.ikuta added a comment.
remove comment out code
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,174 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 169791.
takuto.ikuta added a comment.
Fix linkage for inline function of explicit template instantiation declaration
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,174 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 169783.
takuto.ikuta added a comment.
Remove unnecessary attr creation
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,167 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 169652.
takuto.ikuta retitled this revision from "[WIP] Add /Zc:DllexportInlines option
to clang-cl" to "Add /Zc:DllexportInlines option to clang-cl".
takuto.ikuta added a comment.
Export function inside explicit template instantiation definition
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,167 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > takuto.ikuta wrote:
> > > > hans wrote:
> > > > > Why does this need to be a loop? I don't think FunctionDecl's can be
> > > > > nested?
> > > > This is for static local var in lambda function.
> > > > static_x's ParentFunction does not become f().
> > > > ```
> > > > class __declspec(dllexport) C {
> > > > int f() {
> > > > return ([]() { static int static_x; return ++static_x; })();
> > > > }
> > > > };
> > > > ```
> > > I don't think the lambda (or its static local) should be exported in this
> > > case.
> > If we don't export static_x, dllimport library cannot find static_x when
> > linking?
> >
> >
> I believe even if C is marked dllimport, the lambda will not be dllimport.
> The inline definition will be used.
Do you say importing/implementation library should have different instance of
static_x here?
Current clang does not generate such obj.
I think static_x should be dllimported. But without this loop, static_x cannot
find FD of C::f() having DLLImport/ExportStaticLocalAttr and static_x won't be
treated as imported/exported variable.
And if static_x is not exported, importing library and implementation library
will not have the same instance of static_x when C::f() is inlined.
Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+// Function having static local variables should be exported.
+auto *ExportAttr =
cast(NewAttr->clone(getASTContext()));
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > Isn't it enough that the static local is exported, does the function
> > > > itself need to be exported too?
> > > Adding dllexport only to variable does not export variable when the
> > > function is not used.
> > > But dllimport is not necessary for function, removed.
> > Hmm, I wonder if we could fix that instead? That is, export the variable in
> > that case even if the function is not used?
> I see. I'll investigate which code path emits static variables
static local variable seems to be exported in
https://github.com/llvm-project/llvm-project-20170507/blob/f54514c50350743d3d1a3c5aa80cbe4bb449eb71/clang/lib/CodeGen/CGDecl.cpp#L378
But emitting static local var only by bypassing function emission seems
difficult.
Let me leave as-is here.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > I still don't understand why we need these checks for template
> > > instantiations. Why does it matter whether the functions get inlined or
> > > not?
> > When function of dllimported class is not inlined, such funtion needs to be
> > dllexported from implementation library.
> >
> > c.h
> > ```
> > template
> > class EXPORT C {
> > public:
> > void f() {}
> > };
> > ```
> >
> > cuser.cc
> > ```
> > #include "c.h"
> >
> > void cuser() {
> > C c;
> > c.f(); // This function may not be inlined when EXPORT is
> > __declspec(dllimport), so link may fail.
> > }
> > ```
> >
> > When cuser.cc and c.h are built to different library, cuser.cc needs to be
> > able to see dllexported C::f() when C::f() is not inlined.
> > This is my understanding.
> Your example doesn't use explicit instantiation definition or declaration, so
> doesn't apply here I think.
>
> As long as the dllexport and dllimport attributes matches it's fine. It
> doesn't matter whether the function gets inlined or not, the only thing that
> matters is that if it's marked dllimport on the "consumer side", it must be
> dllexport when building the dll.
Hmm, I changed the linkage for functions having DLLExport/ImportStaticLocal
Attr.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 169365.
takuto.ikuta added a comment.
Address comment
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/AST/ASTContext.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,164 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > Why does this need to be a loop? I don't think FunctionDecl's can be
> > > > nested?
> > > This is for static local var in lambda function.
> > > static_x's ParentFunction does not become f().
> > > ```
> > > class __declspec(dllexport) C {
> > > int f() {
> > > return ([]() { static int static_x; return ++static_x; })();
> > > }
> > > };
> > > ```
> > I don't think the lambda (or its static local) should be exported in this
> > case.
> If we don't export static_x, dllimport library cannot find static_x when
> linking?
>
>
I believe even if C is marked dllimport, the lambda will not be dllimport. The
inline definition will be used.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
takuto.ikuta wrote:
> hans wrote:
> > I still don't understand why we need these checks for template
> > instantiations. Why does it matter whether the functions get inlined or not?
> When function of dllimported class is not inlined, such funtion needs to be
> dllexported from implementation library.
>
> c.h
> ```
> template
> class EXPORT C {
> public:
> void f() {}
> };
> ```
>
> cuser.cc
> ```
> #include "c.h"
>
> void cuser() {
> C c;
> c.f(); // This function may not be inlined when EXPORT is
> __declspec(dllimport), so link may fail.
> }
> ```
>
> When cuser.cc and c.h are built to different library, cuser.cc needs to be
> able to see dllexported C::f() when C::f() is not inlined.
> This is my understanding.
Your example doesn't use explicit instantiation definition or declaration, so
doesn't apply here I think.
As long as the dllexport and dllimport attributes matches it's fine. It doesn't
matter whether the function gets inlined or not, the only thing that matters is
that if it's marked dllimport on the "consumer side", it must be dllexport when
building the dll.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > Why does this need to be a loop? I don't think FunctionDecl's can be
> > > nested?
> > This is for static local var in lambda function.
> > static_x's ParentFunction does not become f().
> > ```
> > class __declspec(dllexport) C {
> > int f() {
> > return ([]() { static int static_x; return ++static_x; })();
> > }
> > };
> > ```
> I don't think the lambda (or its static local) should be exported in this
> case.
If we don't export static_x, dllimport library cannot find static_x when
linking?
Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+// Function having static local variables should be exported.
+auto *ExportAttr =
cast(NewAttr->clone(getASTContext()));
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > Isn't it enough that the static local is exported, does the function
> > > itself need to be exported too?
> > Adding dllexport only to variable does not export variable when the
> > function is not used.
> > But dllimport is not necessary for function, removed.
> Hmm, I wonder if we could fix that instead? That is, export the variable in
> that case even if the function is not used?
I see. I'll investigate which code path emits static variables
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
hans wrote:
> I still don't understand why we need these checks for template
> instantiations. Why does it matter whether the functions get inlined or not?
When function of dllimported class is not inlined, such funtion needs to be
dllexported from implementation library.
c.h
```
template
class EXPORT C {
public:
void f() {}
};
```
cuser.cc
```
#include "c.h"
void cuser() {
C c;
c.f(); // This function may not be inlined when EXPORT is
__declspec(dllimport), so link may fail.
}
```
When cuser.cc and c.h are built to different library, cuser.cc needs to be able
to see dllexported C::f() when C::f() is not inlined.
This is my understanding.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
takuto.ikuta wrote:
> hans wrote:
> > Why does this need to be a loop? I don't think FunctionDecl's can be nested?
> This is for static local var in lambda function.
> static_x's ParentFunction does not become f().
> ```
> class __declspec(dllexport) C {
> int f() {
> return ([]() { static int static_x; return ++static_x; })();
> }
> };
> ```
I don't think the lambda (or its static local) should be exported in this case.
Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+// Function having static local variables should be exported.
+auto *ExportAttr =
cast(NewAttr->clone(getASTContext()));
takuto.ikuta wrote:
> hans wrote:
> > Isn't it enough that the static local is exported, does the function itself
> > need to be exported too?
> Adding dllexport only to variable does not export variable when the function
> is not used.
> But dllimport is not necessary for function, removed.
Hmm, I wonder if we could fix that instead? That is, export the variable in
that case even if the function is not used?
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+ TSK != TSK_ExplicitInstantiationDeclaration &&
+ TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {
I still don't understand why we need these checks for template instantiations.
Why does it matter whether the functions get inlined or not?
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
Thank you for review! I updated the code.
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
hans wrote:
> Why does this need to be a loop? I don't think FunctionDecl's can be nested?
This is for static local var in lambda function.
static_x's ParentFunction does not become f().
```
class __declspec(dllexport) C {
int f() {
return ([]() { static int static_x; return ++static_x; })();
}
};
```
Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+// Function having static local variables should be exported.
+auto *ExportAttr =
cast(NewAttr->clone(getASTContext()));
hans wrote:
> Isn't it enough that the static local is exported, does the function itself
> need to be exported too?
Adding dllexport only to variable does not export variable when the function is
not used.
But dllimport is not necessary for function, removed.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5705
+TSK != TSK_ExplicitInstantiationDeclaration &&
+TSK != TSK_ExplicitInstantiationDefinition) {
+ if (ClassExported) {
hans wrote:
> But I don't understand why the template stuff is checked here...
>
> The way I imagined this, we'd only need to change the code when creating
> NewAttr below..
> Something like
>
> ```
> NewAttr = ClassAtr->clone()...
> if (!getLandOpts().DllExportInlines() && Member is an inline method)
> NewAttr = our new dllimport/export static locals attribute
> ```
>
> What do you think?
> But I don't understand why the template stuff is checked here...
Templated inline function is not always inlined, it seems depending on
optimization level.
I updated the code as you wrote in later part of comment.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 169179.
takuto.ikuta marked an inline comment as done.
takuto.ikuta added a comment.
address comment
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,163 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added a comment.
Thanks! I think this is getting close now.
Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+ !FD->hasAttr() &&
Why does this need to be a loop? I don't think FunctionDecl's can be nested?
Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+// Function having static local variables should be exported.
+auto *ExportAttr =
cast(NewAttr->clone(getASTContext()));
Isn't it enough that the static local is exported, does the function itself
need to be exported too?
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5703
+if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+!getLangOpts().DllExportInlines &&
+TSK != TSK_ExplicitInstantiationDeclaration &&
I don't think checking for Microsoft ABI is necessary, just checking the
DllExportInlines flag should be enough.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5705
+TSK != TSK_ExplicitInstantiationDeclaration &&
+TSK != TSK_ExplicitInstantiationDefinition) {
+ if (ClassExported) {
But I don't understand why the template stuff is checked here...
The way I imagined this, we'd only need to change the code when creating
NewAttr below..
Something like
```
NewAttr = ClassAtr->clone()...
if (!getLandOpts().DllExportInlines() && Member is an inline method)
NewAttr = our new dllimport/export static locals attribute
```
What do you think?
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
Thank you for review!
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5599
+bool Sema::isInlineFunctionDLLExportable(const CXXMethodDecl *MD) {
+ assert(MD->isInlined());
hans wrote:
> Okay, breaking out this logic is a little better, but I still don't like that
> we now have split the "inherit dllexport attribute" in two places: one for
> non-inline functions and one for inline (even if they both call this
> function). It feels like it will be hard to maintain.
>
> Here is another idea:
>
> When we inherit the dllexport attribute to class members, if
> getLangOpts().DllExportInlines is false, we don't put dllexport on inline
> functions, but instead we put a new attribute "dllexportstaticlocals".
>
> That attribute only has the effect that it makes static locals exported. We
> would check for it when computing the linkage of the static local, similar to
> how it works in hidden functions.
>
> This has two benefits: it doesn't complicate the "inherit dllexport" code
> very much, and it avoids the need for a separate AST walk of the function.
>
> What do you think?
I implemented your idea the way I understood.
Use new attribute to check static local var later.
Due to difference of treating between linkage and dll attribute, I inherit dll
attribute in Sema/SemaDecl.cpp instead of AST/Decl.cpp
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 168979.
takuto.ikuta added a comment.
address comment
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,160 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local dllexport void
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5599
+bool Sema::isInlineFunctionDLLExportable(const CXXMethodDecl *MD) {
+ assert(MD->isInlined());
Okay, breaking out this logic is a little better, but I still don't like that
we now have split the "inherit dllexport attribute" in two places: one for
non-inline functions and one for inline (even if they both call this function).
It feels like it will be hard to maintain.
Here is another idea:
When we inherit the dllexport attribute to class members, if
getLangOpts().DllExportInlines is false, we don't put dllexport on inline
functions, but instead we put a new attribute "dllexportstaticlocals".
That attribute only has the effect that it makes static locals exported. We
would check for it when computing the linkage of the static local, similar to
how it works in hidden functions.
This has two benefits: it doesn't complicate the "inherit dllexport" code very
much, and it avoids the need for a separate AST walk of the function.
What do you think?
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1256299, @hans wrote: > In https://reviews.llvm.org/D51340#1254898, @takuto.ikuta wrote: > > > Ping? > > > Sorry, I know I'm being slow to look at this :-( I have not forgotten, it's > just a lot of other things happening at the same time. I see. Sorry for rushing you. I wait until you have time. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added a comment. In https://reviews.llvm.org/D51340#1254898, @takuto.ikuta wrote: > Ping? Sorry, I know I'm being slow to look at this :-( I have not forgotten, it's just a lot of other things happening at the same time. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. Ping? https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/Sema/SemaDecl.cpp:12624
+ isa(D)) {
+CXXMethodDecl *MD = dyn_cast(D);
+CXXRecordDecl *Class = MD->getParent();
hans wrote:
> Hmm, now we're adding an AST walk over all inline methods which probably
> slows us down a bit. Not sure I have any better ideas though.
>
> In any case, ActOnFinishInlineFunctionDef needs a comment explaining why it's
> doing this.
Added comment. I think typical code does not have static variables in inline
function and this check is worth to be done for the performance improvement.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
Thank you for review!
In https://reviews.llvm.org/D51340#1246508, @hans wrote:
> The static local stuff still makes me nervous. How much does Chromium break
> if we just ignore the problem? And how does -fvisibility-inlines-hidden
> handle the issue?
I'm not sure how much chromium will break, if we ignore static local variables.
But ignoring static local var may cause some unhappy behavior and experience to
developer.
I'd like to have check for local static variables as
-fvisibility-inlines-hidden does.
-fvisibility-inlines-hidden checks static local around here.
https://github.com/llvm-project/llvm-project-20170507/blob/77b7ca42b0ea496373c5e5a9710b1fe11e1fba68/clang/lib/AST/Decl.cpp#L1265
> Also, Sema already has a check for static locals in C inline functions:
>
> $ echo "inline void f() { static int x; }" | bin/clang -c -x c -
> :1:19: warning: non-constant static local variable in inline
> function may be different in different files [-Wstatic-local-in-inline]
> inline void f() { static int x; }
> ^
>
>
>
> could we reuse that check somehow for this case with static locals in
> dllexport inline methods?
I think we can do the same thing when we are given -fno-dllexport-inlines
around here.
https://github.com/llvm-project/llvm-project-20170507/blob/77b7ca42b0ea496373c5e5a9710b1fe11e1fba68/clang/lib/Sema/SemaDecl.cpp#L6661
But I bit prefer to do export functions with static local variables. Because
that is consistent with -fvisibility-inlines-hidden and we won't need more
changes to chromium than the CLs in description.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5702
+!getLangOpts().DllExportInlines &&
+Class->getTemplateSpecializationKind() !=
TSK_ExplicitInstantiationDeclaration &&
+Class->getTemplateSpecializationKind() !=
TSK_ExplicitInstantiationDefinition) {
hans wrote:
> What worries me is that now we have logic in two different places about
> inheriting the dll attribute.
>
> The new place in ActOnFinishInlineFunctionDef doesn't have the same checks
> (checking for MS ABI and the template specialization kind) which means the
> logic for when to inherit is already a little out of sync...
Agree. Do you allow me to extract check to function and re-use that?
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 167683.
takuto.ikuta added a comment.
Update comment for Sema::ActOnFinishInlineFunctionDef
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/include/clang/Sema/Sema.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 167682.
takuto.ikuta added a comment.
Extract inline function export check to function
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/include/clang/Sema/Sema.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added a comment.
The static local stuff still makes me nervous. How much does Chromium break if
we just ignore the problem? And how does -fvisibility-inlines-hidden handle the
issue?
Also, Sema already has a check for static locals in C inline functions:
$ echo "inline void f() { static int x; }" | bin/clang -c -x c -
:1:19: warning: non-constant static local variable in inline function
may be different in different files [-Wstatic-local-in-inline]
inline void f() { static int x; }
^
could we reuse that check somehow for this case with static locals in dllexport
inline methods?
Comment at: clang/lib/Sema/SemaDecl.cpp:12624
+ isa(D)) {
+CXXMethodDecl *MD = dyn_cast(D);
+CXXRecordDecl *Class = MD->getParent();
Hmm, now we're adding an AST walk over all inline methods which probably slows
us down a bit. Not sure I have any better ideas though.
In any case, ActOnFinishInlineFunctionDef needs a comment explaining why it's
doing this.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5702
+!getLangOpts().DllExportInlines &&
+Class->getTemplateSpecializationKind() !=
TSK_ExplicitInstantiationDeclaration &&
+Class->getTemplateSpecializationKind() !=
TSK_ExplicitInstantiationDefinition) {
What worries me is that now we have logic in two different places about
inheriting the dll attribute.
The new place in ActOnFinishInlineFunctionDef doesn't have the same checks
(checking for MS ABI and the template specialization kind) which means the
logic for when to inherit is already a little out of sync...
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1243453, @hans wrote: > In https://reviews.llvm.org/D51340#1243331, @takuto.ikuta wrote: > > > Ping? > > > > This patch reduced obj size largely, and I expect this makes distributed > > build (like goma) faster by reducing data transferred on network. > > > I'll try to look at it this week. > > Have you confirmed on some Chromium file, e.g. stroke_opacity_custom.cc > (go/stroke-opactity-custom) that the number of functions that we codegen > decreases as expected? I'd expect this to save a lot of compile time. Yes, stroke_opacity_custom.obj export 18 symbols, reduced from 781 in PCH build, compile time is reduced from 8.2 seconds to 6.7 seconds. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added a comment. In https://reviews.llvm.org/D51340#1243331, @takuto.ikuta wrote: > Ping? > > This patch reduced obj size largely, and I expect this makes distributed > build (like goma) faster by reducing data transferred on network. I'll try to look at it this week. Have you confirmed on some Chromium file, e.g. stroke_opacity_custom.cc (go/stroke-opactity-custom) that the number of functions that we codegen decreases as expected? I'd expect this to save a lot of compile time. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment.
Ping?
This patch reduced obj size largely, and I expect this makes distributed build
(like goma) faster by reducing data transferred on network.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244
+ false))
+CmdArgs.push_back("-fvisibility-inlines-hidden");
+
takuto.ikuta wrote:
> rnk wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > takuto.ikuta wrote:
> > > > > hans wrote:
> > > > > > Huh, does this actually affect whether functions get dllexported or
> > > > > > not?
> > > > > Sorry, what you want to ask?
> > > > >
> > > > > This will used to not add dllexport attr in L5690 of SemaDeclCXX.cpp.
> > > > >
> > > > Oops, I didn't see that. I'm glad to see this is looking so simple :-)
> > > >
> > > > Actually, I don't think we should the same flag name for this, since
> > > > "hidden" is an ELF concept, not a COFF one, just that we should match
> > > > the behaviour of the flag.
> > > >
> > > > Hmm, or do people use -fvisibility-inlines-hidden on MinGW or
> > > > something? Where does the hidden-dllimport.cpp file come from?
> > > >
> > > > Also, is it the case that -fvisibility-inlines-hidden just ignores the
> > > > problem of static local variables? If that's the case we can probably
> > > > do it too, we just have to be sure, and document it eventually.
> > > >
> > > I confirmed that -fvisibility-inlines-hidden treats local static var
> > > correctly in linux.
> > > So I'm trying to export inline functions if it has local static variables.
> > This sounds like it would be really hard in general, since you can hide
> > static locals almost anywhere:
> > ```
> > struct Foo {
> > static int foo() {
> > return ([]() { static int x; return ++x; })();
> > }
> > };
> > ```
> > Can we reuse the RecursiveASTVisitor @hans added to check if we can emit
> > dllimport inline functions as available_externally definitions? I think it
> > will find exactly the circumstances we are looking for, i.e. export all the
> > things that cannot be emitted inline in other DLLs.
> Actually, StmtVisitor added dll export attribute to local static variable in
> lambda function. And static variables seems exported.
>
> But I found other issue in current implementation, some cxx method decls
> passed to emitting function before local static variable checker adds dll
> export attributes. In such case local static variables are not exported and
> link failure happens.
>
> So let me try to use DLLImportFunctionVisitor in
> CodeGenModule::shouldEmitFunction for exported function instead of
> processing/checking dll attribute around SemaDeclCXX.
I think avoiding dll export is better to be done before we reached around
CodeGenModule. Also removing dll attribute later made it difficult to pass
tests.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 166450.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?OutclassDefFunc@?$TemplateExportedClass@VA11@@
+template class __declspec(dllexport)
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 166448.
takuto.ikuta edited the summary of this revision.
takuto.ikuta added a comment.
Remove unnecessary willHaveBody check condition
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// DEFAULT-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. PTAL again. I confirmed that current patch can link chrome and functions with local static variable are exported. But current ToT clang was not improved well by this patch. I guess there is some change recently making effect of this patch smaller. Or chromium has many inline functions with static local variable? https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 166087.
takuto.ikuta retitled this revision from "[WIP] Add /Zc:DllexportInlines option
to clang-cl" to "Add /Zc:DllexportInlines option to clang-cl".
takuto.ikuta edited the summary of this revision.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// DEFAULT-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+ static int static_variable = 0;
+ return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+ NoTemplateExportedClass a;
+ a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244
+ false))
+CmdArgs.push_back("-fvisibility-inlines-hidden");
+
hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > Huh, does this actually affect whether functions get dllexported or not?
> > Sorry, what you want to ask?
> >
> > This will used to not add dllexport attr in L5690 of SemaDeclCXX.cpp.
> >
> Oops, I didn't see that. I'm glad to see this is looking so simple :-)
>
> Actually, I don't think we should the same flag name for this, since "hidden"
> is an ELF concept, not a COFF one, just that we should match the behaviour of
> the flag.
>
> Hmm, or do people use -fvisibility-inlines-hidden on MinGW or something?
> Where does the hidden-dllimport.cpp file come from?
>
> Also, is it the case that -fvisibility-inlines-hidden just ignores the
> problem of static local variables? If that's the case we can probably do it
> too, we just have to be sure, and document it eventually.
>
I confirmed that -fvisibility-inlines-hidden treats local static var correctly
in linux.
So I'm trying to export inline functions if it has local static variables.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 164822.
takuto.ikuta added a comment.
I'm trying to handle local static var correctly.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Driver/CLCompatOptions.td
clang/include/clang/Sema/Sema.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
clang/test/CodeGenCXX/hidden-dllimport.cpp
Index: clang/test/CodeGenCXX/hidden-dllimport.cpp
===
--- clang/test/CodeGenCXX/hidden-dllimport.cpp
+++ clang/test/CodeGenCXX/hidden-dllimport.cpp
@@ -1,8 +1,8 @@
// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fvisibility-inlines-hidden -o - %s | FileCheck %s
-// We used to declare this hidden dllimport, which is contradictory.
+// We don't declare this hidden dllimport.
-// CHECK: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
+// CHECK-NOT: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
struct __attribute__((dllimport)) foo {
void bar() {}
Index: clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-import-fvisibility-inlines-hidden.cpp
@@ -0,0 +1,162 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fvisibility-inlines-hidden -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// NOINLINE-DAG: define weak_odr hidden dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+// INLINE-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+ static int static_variable = 0;
+ ++static_variable;
+ return static_variable;
+}
+
+int ImportedFunctionUser() {
+ return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// DEFAULT-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+// DEFAULT-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// local static var in ImportedClass
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+// INLINE-NOT: static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // INLINE-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ // NOINLINE-DAG: define weak_odr hidden dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+ __forceinline void InlineOutclassDefFunc();
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+ void OutclassDefFunc();
+};
+
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244
+ false))
+CmdArgs.push_back("-fvisibility-inlines-hidden");
+
takuto.ikuta wrote:
> hans wrote:
> > Huh, does this actually affect whether functions get dllexported or not?
> Sorry, what you want to ask?
>
> This will used to not add dllexport attr in L5690 of SemaDeclCXX.cpp.
>
Oops, I didn't see that. I'm glad to see this is looking so simple :-)
Actually, I don't think we should the same flag name for this, since "hidden"
is an ELF concept, not a COFF one, just that we should match the behaviour of
the flag.
Hmm, or do people use -fvisibility-inlines-hidden on MinGW or something? Where
does the hidden-dllimport.cpp file come from?
Also, is it the case that -fvisibility-inlines-hidden just ignores the problem
of static local variables? If that's the case we can probably do it too, we
just have to be sure, and document it eventually.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244
+ false))
+CmdArgs.push_back("-fvisibility-inlines-hidden");
+
hans wrote:
> Huh, does this actually affect whether functions get dllexported or not?
Sorry, what you want to ask?
This will used to not add dllexport attr in L5690 of SemaDeclCXX.cpp.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
hans added inline comments.
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244
+ false))
+CmdArgs.push_back("-fvisibility-inlines-hidden");
+
Huh, does this actually affect whether functions get dllexported or not?
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1226989, @takuto.ikuta wrote: > In https://reviews.llvm.org/D51340#1222013, @hans wrote: > > > Did both your builds use PCH? It'd be interesting to see the difference > > without PCH too; the effect should be even larger. > > > Added stats of without PCH build. > > > The summary should probably reference > > https://bugs.llvm.org/show_bug.cgi?id=33628 and it needs to mention how it > > affects dllimport too. > > Added to description, thanks! > > > Okay, after reading through the patch, it seems we're still marking class > > members dllexport, and then you selectively remove the attribute later. > > That does feel a little bit backward... Does -fvisibility-inlines-hidden > > also have the static local problem, or how does that flag handle it? > > Ah, maybe I can get performance improvement just support > fvisibility-inlines-hidden in clang-cl. Let me try. I just support fvisibility-inlines-hidden in clang-cl and that looks to work intended. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 164379.
takuto.ikuta edited the summary of this revision.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-inline.cpp
clang/test/CodeGenCXX/hidden-dllimport.cpp
Index: clang/test/CodeGenCXX/hidden-dllimport.cpp
===
--- clang/test/CodeGenCXX/hidden-dllimport.cpp
+++ clang/test/CodeGenCXX/hidden-dllimport.cpp
@@ -1,8 +1,8 @@
// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fvisibility-inlines-hidden -o - %s | FileCheck %s
-// We used to declare this hidden dllimport, which is contradictory.
+// We don't declare this hidden dllimport.
-// CHECK: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
+// CHECK-NOT: declare dllimport void @"?bar@foo@@QEAAXXZ"(%struct.foo*)
struct __attribute__((dllimport)) foo {
void bar() {}
Index: clang/test/CodeGenCXX/dllexport-no-inline.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-inline.cpp
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fvisibility-inlines-hidden -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// NOINLINE-DAG: define weak_odr hidden dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+// INLINE-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+// NOINLINE-DAG: @"?static_const_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HB" = linkonce_odr dso_local constant i32 1, comdat, align 4
+// INLINE-DAG: @"?static_const_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HB" = weak_odr dso_local dllexport constant i32 1, comdat, align 4
+// NOINLINE-DAG: @"?static_const_array@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4QBHB" = linkonce_odr dso_local constant [3 x i32] [i32 1, i32 2, i32 3], comdat, align 4
+// INLINE-DAG: @"?static_const_array@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4QBHB" = weak_odr dso_local dllexport constant [3 x i32] [i32 1, i32 2, i32 3], comdat, align 4
+// NOINLINE-DAG: @"?static_variable_non_const_cse@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 4, comdat, align 4
+// INLINE-DAG: @"?static_variable_non_const_cse@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 4, comdat, align 4
+// NOINLINE-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = linkonce_odr dso_local global i32 0, comdat, align 4
+// INLINE-DAG: @"?static_variable@?1??InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+ void InclassDefFunc() {}
+
+ int f();
+
+ // NOINLINE-NOT: InclassDefFuncWithStaticVariable@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+ int InclassDefFuncWithStaticVariable() {
+static int static_variable = f();
+static const int static_const_variable = 1;
+// DEFAULT-NOT: static_constexpr_variable
+static constexpr int static_constexpr_variable = 2;
+static const int static_const_array[] = {1, 2, 3};
+static int static_variable_non_const_cse = 4;
+
+++static_variable_non_const_cse;
+++static_variable;
+return static_const_variable + static_constexpr_variable +
+
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta marked 2 inline comments as done. takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1222013, @hans wrote: > Did both your builds use PCH? It'd be interesting to see the difference > without PCH too; the effect should be even larger. Added stats of without PCH build. > The summary should probably reference > https://bugs.llvm.org/show_bug.cgi?id=33628 and it needs to mention how it > affects dllimport too. Added to description, thanks! > Okay, after reading through the patch, it seems we're still marking class > members dllexport, and then you selectively remove the attribute later. That > does feel a little bit backward... Does -fvisibility-inlines-hidden also have > the static local problem, or how does that flag handle it? Ah, maybe I can get performance improvement just support fvisibility-inlines-hidden in clang-cl. Let me try. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta marked 4 inline comments as done.
takuto.ikuta added inline comments.
Comment at: clang/include/clang/Basic/LangOptions.def:117
LANGOPT(Digraphs , 1, 0, "digraphs")
+LANGOPT(DllexportInlines , 1, 1, "If dllexport a class should dllexport
implicit inline methods in the Microsoft ABI")
BENIGN_LANGOPT(HexFloats , 1, C99, "C99 hexadecimal float constants")
hans wrote:
> Same comment about "implicit" here as above.
>
> And is the "In the MS ABI" part of the comment important? Does the flag
> change depending on ABI?
>
>
> And is the "In the MS ABI" part of the comment important? Does the flag
> change depending on ABI?
Currently it works only for Microsoft ABI.
In this patch, I want to focus only on Microsoft ABI.
Comment at: clang/include/clang/Basic/LangOptions.h:217
+ /// If set, dllexported classes dllexport their implicit inline methods.
+ bool DllexportInlines = true;
hans wrote:
> not sure what you mean by implicit here.. this applies to all inline defined
> member functions.
Dropped.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5542
+// or checkClassLevelDLLAttribute?
+if (MD->isInlined() && getDLLAttr(Member)) {
+ const Stmt* Body = nullptr;
hans wrote:
> Hmm, yes I think the intention was to not put the attribute on the members so
> I'm not sure why this is needed?
> Hmm, yes I think the intention was to not put the attribute on the members so
> I'm not sure why this is needed?
I changed the code around here from dropping DLL attribute to warning static
local variable.
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5556
+ Body && !Checker.Visit(Body)) {
+MD->dropAttr();
+ }
takuto.ikuta wrote:
> I noticed that this does not work correctly yet.
> Occasionally, some functions are not inlined but not exported, and those
> cannot be linked.
I gave up to support local static variable.
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta updated this revision to Diff 164353.
takuto.ikuta edited the summary of this revision.
takuto.ikuta added a comment.
Herald added a subscriber: dschuff.
Make patch closer to Nico's original implementation, but warns local static
variable instead of detecting it.
In checkClassLevelDLLAttribute, inline function definition is not fully parsed
and I cannot make test passed in other way adding export only to inline
function having local static variables.
https://reviews.llvm.org/D51340
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/CLCompatOptions.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CodeGenCXX/dllexport-no-inline.cpp
Index: clang/test/CodeGenCXX/dllexport-no-inline.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-inline.cpp
@@ -0,0 +1,131 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - -verify -DNOEXPORT_INLINE | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc \
+// RUN: -emit-llvm -O0 -o - | \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+
+// Class member function
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+ NoTemplateExportedClass() = default;
+
+ // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+ // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@AEAAXXZ"
+ void InclassDefFunc() {}
+
+ int f();
+
+ // FIX: make this DEFAULT-DAG.
+ // INLINE-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@AEAAHXZ"
+ int InclassDefFuncWithStaticVariable() {
+#ifdef NOEXPORT_INLINE
+// expected-warning@+2 {{static 'static_variable' used in an inline function of exported class isn't exported due to -fno-dllexport-inlines}}
+#endif
+static int static_variable = f();
+
+static const int static_const_variable = 1; // expected-no-warning
+constexpr int static_constexpr_variable = 2; // expected-no-warning
+static const int static_const_array[] = {1, 2, 3}; // expected-no-warning
+
+return ++static_variable + static_const_variable + static_constexpr_variable +
+static_const_array[0];
+ }
+
+ // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+ __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+ // FIX: Do not export this when -ms-no-dllexport-inline.
+ // DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InlineOutclassDefFunc@NoTemplateExportedClass@@AEAAXXZ"
+ __forceinline void InlineOutclassDefFunc();
+
+ // FIX: Do not export this when -ms-no-dllexport-inline is given and warn for local static var.
+ // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@AEAAHXZ"
+ __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+ // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@AEAAXXZ"
+ void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+return ++static_variable;
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+ void InclassDefFunc() {}
+ void OutclassDefFunc();
+
+ T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?OutclassDefFunc@?$TemplateExportedClass@VA11@@
+template class __declspec(dllexport) TemplateExportedClass;
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VB22@@
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?OutclassDefFunc@?$TemplateExportedClass@VB22@@
+template class TemplateExportedClass;
+
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1225805, @takuto.ikuta wrote: > I found that current ToT with original Nico's patch does not allow to link > ui_base.dll > > https://github.com/atetubou/llvm-project-20170507/tree/totwin_dbg_1236_nico > gives the following link error > https://pastebin.com/9LVRbRVn > > I will do bisection to find when some inline functions are not inlined. Sorry, this is due to my local build config of chromium. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
takuto.ikuta added a comment. I found that current ToT with original Nico's patch does not allow to link ui_base.dll https://github.com/atetubou/llvm-project-20170507/tree/totwin_dbg_1236_nico gives the following link error https://pastebin.com/9LVRbRVn I will do bisection to find when some inline functions are not inlined. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl
probinson added subscribers: cfe-commits, probinson. probinson added a comment. +cfe-commits https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
