Add /Zc:DllexportInlines option to clang-cl

[takuto.ikuta]

Summary

This CL adds /Zc:DllexportInlines flag to clang-cl.
When Zc:DllexportInlines- is specified, inline class member function is not exported if the function does not have local static variables.

By not exporting inline function, code for those functions are not generated and that reduces both compile time and obj size. Also this flag does not import inline functions from dllimported class if the function does not have local static variables.

On my 24C48T windows10 machine, build performance of chrome target in chromium repository is like below.
These stats are come with 'target_cpu="x86" enable_nacl = false is_component_build=true dcheck_always_on=true` build config and applied

• https://chromium-review.googlesource.com/c/chromium/src/+/1212379
• https://chromium-review.googlesource.com/c/v8/v8/+/1186017

Below stats were taken with this patch applied on https://github.com/llvm-project/llvm-project-20170507/commit/a05115cd4c57ff76b0f529e38118765b58ed7f2e

config	build time	speedup	build dir size
with patch, PCH on, debug	1h10m0s	x1.13	35.6GB
without patch, PCH on, debug	1h19m17s		49.0GB
with patch, PCH off, debug	1h15m45s	x1.16	33.7GB
without patch, PCH off, debug	1h28m10s		52.3GB
with patch, PCH on, release	1h13m13s	x1.22	26.2GB
without patch, PCH on, release	1h29m57s		37.5GB
with patch, PCH off, release	1h23m38s	x1.32	23.7GB
without patch, PCH off, release	1h50m50s		38.7GB

This patch reduced obj size and the number of exported symbols largely, that improved link time too.
e.g. link time stats of blink_core.dll become like below

	cold disk cache	warm disk cache
with patch, PCH on, debug	71s	30s
without patch, PCH on, debug	111s	48s

This patch's implementation is based on Nico Weber's patch. I modified to support static local variable, added tests and took stats.

Bug: https://bugs.llvm.org/show_bug.cgi?id=33628

[takuto.ikuta]

In D51340#1226989, @takuto.ikuta wrote:

In 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.

[hans]

clang/lib/Driver/ToolChains/Clang.cpp

5508

Huh, does this actually affect whether functions get dllexported or not?

[takuto.ikuta]

clang/lib/Driver/ToolChains/Clang.cpp

5508

Sorry, what you want to ask?

This will used to not add dllexport attr in L5690 of SemaDeclCXX.cpp.

[hans]

clang/lib/Driver/ToolChains/Clang.cpp

5508

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.

[takuto.ikuta]

I'm trying to handle local static var correctly.

[takuto.ikuta]

clang/lib/Driver/ToolChains/Clang.cpp

5508

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.

[rnk]

clang/lib/Driver/ToolChains/Clang.cpp

5508

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.

[takuto.ikuta]

clang/lib/Driver/ToolChains/Clang.cpp

5508

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.

[takuto.ikuta]

Current implementation cannot build chrome when pch is enabled.
undefined symbol error happens during linking blink_modules.dll

[takuto.ikuta]

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?

[takuto.ikuta]

Remove unnecessary willHaveBody check condition

[takuto.ikuta]

Ping?

This patch reduced obj size largely, and I expect this makes distributed build (like goma) faster by reducing data transferred on network.

clang/lib/Driver/ToolChains/Clang.cpp

5508

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.

[hans]

In 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.

[takuto.ikuta]

In D51340#1243453, @hans wrote:

In 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.

[hans]

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 -
<stdin>: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?

clang/lib/Sema/SemaDecl.cpp

12596

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.

clang/lib/Sema/SemaDeclCXX.cpp

5685

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...

[takuto.ikuta]

Extract inline function export check to function

[takuto.ikuta]

Update comment for Sema::ActOnFinishInlineFunctionDef

[takuto.ikuta]

Thank you for review!

In 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 -
<stdin>: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.

clang/lib/Sema/SemaDeclCXX.cpp

5685

Agree. Do you allow me to extract check to function and re-use that?

[takuto.ikuta]

clang/lib/Sema/SemaDecl.cpp

12596

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.

[takuto.ikuta]

Ping?

[hans]

In 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.

[takuto.ikuta]

In D51340#1256299, @hans wrote:

In 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.

[hans]

clang/lib/Sema/SemaDeclCXX.cpp

5582

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?

[takuto.ikuta]

address comment

[takuto.ikuta]

Thank you for review!

clang/lib/Sema/SemaDeclCXX.cpp

5582

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

[hans]

Thanks! I think this is getting close now.

clang/lib/Sema/SemaDecl.cpp

11982

Why does this need to be a loop? I don't think FunctionDecl's can be nested?

12002

Isn't it enough that the static local is exported, does the function itself need to be exported too?

clang/lib/Sema/SemaDeclCXX.cpp

5701

I don't think checking for Microsoft ABI is necessary, just checking the DllExportInlines flag should be enough.

5703

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?

[takuto.ikuta]

address comment

[takuto.ikuta]

Thank you for review! I updated the code.

clang/lib/Sema/SemaDecl.cpp

11982

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; })();
  }
};

12002

Adding dllexport only to variable does not export variable when the function is not used.
But dllimport is not necessary for function, removed.

clang/lib/Sema/SemaDeclCXX.cpp

5703

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.

[hans]

clang/lib/Sema/SemaDecl.cpp

11982

I don't think the lambda (or its static local) should be exported in this case.

12002

Hmm, I wonder if we could fix that instead? That is, export the variable in that case even if the function is not used?

clang/lib/Sema/SemaDeclCXX.cpp

5718

I still don't understand why we need these checks for template instantiations. Why does it matter whether the functions get inlined or not?

[takuto.ikuta]

clang/lib/Sema/SemaDecl.cpp

11982

If we don't export static_x, dllimport library cannot find static_x when linking?

12002

I see. I'll investigate which code path emits static variables

clang/lib/Sema/SemaDeclCXX.cpp

5718

When function of dllimported class is not inlined, such funtion needs to be dllexported from implementation library.

c.h

template<typename T>
class EXPORT C {
 public:
  void f() {}
};

cuser.cc

#include "c.h"

void cuser() {
  C<int> 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.

[hans]

clang/lib/Sema/SemaDecl.cpp

11982

I believe even if C is marked dllimport, the lambda will not be dllimport. The inline definition will be used.

clang/lib/Sema/SemaDeclCXX.cpp

5718

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.

[takuto.ikuta]

Address comment

clang/lib/Sema/SemaDecl.cpp

11982

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.

12002

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.

clang/lib/Sema/SemaDeclCXX.cpp

5718

Hmm, I changed the linkage for functions having DLLExport/ImportStaticLocal Attr.

[takuto.ikuta]

Export function inside explicit template instantiation definition

[takuto.ikuta]

Remove unnecessary attr creation

[takuto.ikuta]

Fix linkage for inline function of explicit template instantiation declaration

[takuto.ikuta]

remove comment out code

[takuto.ikuta]

Hans, I addressed all your comments.
How do you think about current implementation?

clang/lib/Sema/SemaDeclCXX.cpp

5718

I changed linkage in ASTContext so that inline function is emitted when it is necessary when we use fno-dllexport-inlines.

[takuto.ikuta]

ping? Can I go forward in this way?

[hans]

In 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.

clang/include/clang/Basic/Attr.td

2688

The comment should explain what the attribute does, also the dllimport one below.

clang/lib/AST/ASTContext.cpp

9552 ↗

(On Diff #169792)

Can you give an example for why this is needed?

clang/lib/Sema/SemaDecl.cpp

11982

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.

[takuto.ikuta]

Added explanation comment for added attributes and rebased

clang/lib/AST/ASTContext.cpp

9552 ↗

(On Diff #169792)

Sorry, this change does not need. Removed.

clang/lib/Sema/SemaDeclCXX.cpp

5718

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.

[hans]

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 <stdio.h>
#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.

[hans]

clang/lib/Sema/SemaDeclCXX.cpp

5718

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.

[takuto.ikuta]

In 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 <stdio.h>
#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-.

clang/lib/Sema/SemaDeclCXX.cpp

5718

For the following code

template<typename T>
class M{
public:

  T Inclass() {
    static T static_x;
    ++static_x;
    return static_x;
  }
};

template class __declspec(dllexport) M<int>;

extern template class __declspec(dllimport) M<short>;

int f (){
  M<int> mi;
  M<short> ms;
  return mi.Inclass() + ms.Inclass();
}

llvm code without instantiation check become like below. Both inline functions of M<int> and M<short> 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 static local vars will be treated correctly.

$"??4?$M@H@@QEAAAEAV0@AEBV0@@Z" = comdat any

$"?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 dllexport dereferenceable(1) %class.M* @"??4?$M@H@@QEAAAEAV0@AEBV0@@Z"(%class.M* %this, %class.M* dereferenceable(1)) #0 comdat align 2 {
entry:
  %.addr = alloca %class.M*, align 8
  %this.addr = alloca %class.M*, align 8
  store %class.M* %0, %class.M** %.addr, align 8
  store %class.M* %this, %class.M** %this.addr, align 8
  %this1 = load %class.M*, %class.M** %this.addr, align 8
  ret %class.M* %this1
}

; Function Attrs: noinline nounwind optnone
define weak_odr dso_local dllexport 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 dllimport i16 @"?Inclass@?$M@F@@QEAAFXZ"(%class.M.0*) #1

[hans]

$ 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.

clang/lib/Sema/SemaDeclCXX.cpp

5718

Thanks! This seems like a problem with the current code though, and hopefully we can fix it instead of working around it in your patch.

A shorter version of your example:

template <typename T> struct S {
  int foo() { static int x; return x++; }
};

template struct __declspec(dllexport) S<int>;

int f() {
  S<int> a;
  return a.foo();
}

Clang will dllexport S<int>::foo(), but not the static local. That's a bug.

I'll see if I can fix (tracking in https://bugs.llvm.org/show_bug.cgi?id=39496).

[hans]

clang/lib/Sema/SemaDeclCXX.cpp

5718

The fix for that was committed in r345699. I think we can now remove the checks for instantiation here.

[takuto.ikuta]

Rebased to take r345699

[takuto.ikuta]

Thank you for quick fix!

clang/lib/AST/ASTContext.cpp

9552 ↗

(On Diff #169792)

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.

clang/lib/Sema/SemaDeclCXX.cpp

5718

Thanks! I removed this check again.

[hans]

clang/lib/AST/ASTContext.cpp

9552 ↗

(On Diff #169792)

Can you provide a code example of why this is needed?

[takuto.ikuta]

clang/lib/AST/ASTContext.cpp

9552 ↗

(On Diff #169792)

If we don't change function linkage, following code will be compiled like below with -fno-dllexport-inlines.

template<typename>
class M{
public:
  void foo() {}
};

template class __declspec(dllexport) M<int>;

extern template class __declspec(dllimport) M<short>;

void f (){
  M<int> mi;
  mi.foo();

  M<short> 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<short>::foo() is declared, but inline function is not dllexported (See M<int>::foo() is not dllexported). So this code cannot be linked because definition of M<short>::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.

[hans]

clang/lib/AST/ASTContext.cpp

9552 ↗

(On Diff #169792)

Interesting. I wonder how -fvisibility-inlines-hidden handles this...

template <typename> struct S {
  void foo() {}
};

template struct S<int>;

void use() {
  S<int> s;
  s.foo();
}

$ g++ -fvisibility-inlines-hidden -c a.cc && objdump -t a.o | grep _ZN1SIiE3fooEv
0000000000000000 l    d  .text._ZN1SIiE3fooEv	0000000000000000 .text._ZN1SIiE3fooEv
0000000000000000  w    F .text._ZN1SIiE3fooEv	000000000000000b _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.

[takuto.ikuta]

export/import explicit template instantiation function

[takuto.ikuta]

clang/lib/AST/ASTContext.cpp

9552 ↗

(On Diff #169792)

Thank you for confirmation. I revived the check.

[rnk]

clang/lib/AST/ASTContext.cpp

9552 ↗

(On Diff #169792)

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.

[rnk]

clang/include/clang/Basic/LangOptions.h

246

We should define this in the LangOptions.def file.

[takuto.ikuta]

Added option to LangOptions.def

[takuto.ikuta]

Thank you for review!

clang/include/clang/Basic/LangOptions.h

246

Moved.

[hans]

Thank you Takuto! I think the functionality looks great now: it's not too complicated :-)

I only have comments about the test.

clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp

4

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.

7

Instead of using -O0 here and above, consider using -O1 -disable-llvm-passes so we get available_externally functions.

10

This change doesn't have any effect on non-member functions, so I don't think we need these tests.

43

Can you move the checks down to where the variable/function is defined? That makes it easier to read the test I think.

50

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:".

52

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.

59

This doesn't seem to be used for anything?

74

Why foceinline? I think it should be just "inline". This goes for all use of forceinline in this file.

77

Hmm, I would have thought we should export this function.

103

I'm not sure that OutclassDefFunc() adds much value to the test. Also templateValue below. I think they can be removed.

123

We should also have a test with implicit instantiation, and then the inline function should not be exported when using /Zc:dllexportInlines-.

133

Again, I think we can drop the out-of-line function since this change should not affect those.

145

This case is not interesting, since it has nothing to do with dllexport/import, I think it can be removed.

149

Ditto.

166

Again, make the classes "struct" and remove the "public:" lines :-)

167

I don't see any test lines that reference this inner class. I guess it's not interesting?

173

This check looks wrong. I think there should be a dllimport declaration for this function? (Or an available_externally definition if we use -O1?)

182

Let's put this first in the class, since it's the "basic" case, and the static local is more of a special case.

[hans]

clang/include/clang/Driver/CLCompatOptions.td

336

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.

[takuto.ikuta]

Simplify test and fix some behavior.

[takuto.ikuta]

Hans, thank you for review! I addressed all your comment and fixed small behavior.

clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp

77

I see.
Added MD->isThisDeclarationADefinition() check in Sema::checkClassLevelDLLAttribute
This will make change like https://chromium-review.googlesource.com/c/v8/v8/+/1186017 unnecessary.

123

Added test and MD->isImplicitlyInstantiable() check for the test.

173

I think it is OK not dllimporting this function as far as static variable is dllimported and definition of this function is emitted.

[takuto.ikuta]

Added cl driver flag test

[takuto.ikuta]

I missed the comment about driver flag test.

[hans]

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.

clang/lib/Sema/SemaDeclCXX.cpp

5715

The isThisDeclarationADefinition() and isImplicitlyInstantiable() checks don't look right. Just checking isInlined() should be enough.

clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp

10

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.

26

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.

45

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.

48

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.

54

I'd suggest calling it "OutoflineDefFunc()" instead.

66

I don't think there's any reason to dllexport the "user" function.

82

I don't think this instantiation is different from the ​​template class TemplateExportedClass<B22>; one below, so I think we can skip it.

84

There should also be a check to make sure it's exported also in the NOEXPORTINLINE case.

89

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.

[takuto.ikuta]

Address comment

[takuto.ikuta]

added a few more check

[takuto.ikuta]

Thank you for quick review!

clang/lib/Sema/SemaDeclCXX.cpp

5715

I see.

[hans]

Assuming the test still passes when you put the EXPORTINLINE filecheck prefix back, looks good to me!

clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp

10

Did the "EXPORTINLINE" check-prefix get lost here?

[takuto.ikuta]

Fix check-prefix

[takuto.ikuta]

Thank you! I'll submit this if other people do not have any comments.

clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp

10

Good catch!

clang/include/clang/Basic/Attr.td

 }
 
 def DLLExport : InheritableAttr, TargetSpecificAttr<TargetWindows> {
   let Spellings = [Declspec<"dllexport">, GCC<"dllexport">];
   let Subjects = SubjectList<[Function, Var, CXXRecord, ObjCInterface]>;
   let Documentation = [DLLExportDocs];
 }
 
+def DLLExportStaticLocal : InheritableAttr, TargetSpecificAttr<TargetWindows> {
+  // This attribute is used internally only when -fno-dllexport-inlines is
+  // passed. This attribute is added to inline function of class having
+  // dllexport attribute. And if the function has static local variables, this
+  // attribute is used to whether the variables are exported or not. Also if
+  // function has local static variables, the function is dllexported too.
+  let Spellings = [];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}
+
 def DLLImport : InheritableAttr, TargetSpecificAttr<TargetWindows> {
   let Spellings = [Declspec<"dllimport">, GCC<"dllimport">];
   let Subjects = SubjectList<[Function, Var, CXXRecord, ObjCInterface]>;
   let Documentation = [DLLImportDocs];
 
 
   let AdditionalMembers = [{
 private:
   bool PropagatedToBaseTemplate = false;
 
 public:
   void setPropagatedToBaseTemplate() { PropagatedToBaseTemplate = true; }
   bool wasPropagatedToBaseTemplate() { return PropagatedToBaseTemplate; }
   }];
 }
 
+def DLLImportStaticLocal : InheritableAttr, TargetSpecificAttr<TargetWindows> {
+  // This attribute is used internally only when -fno-dllexport-inlines is
+  // passed. This attribute is added to inline function of class having
+  // dllimport attribute. And if the function has static local variables, this
+  // attribute is used to whether the variables are imported or not.
+  let Spellings = [];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}
+
 def SelectAny : InheritableAttr {
   let Spellings = [Declspec<"selectany">, GCC<"selectany">];
   let Documentation = [SelectAnyDocs];
 }
 
 def Thread : Attr {
   let Spellings = [Declspec<"thread">];
   let LangOpts = [MicrosoftExt];

clang/include/clang/Basic/LangOptions.h

public:
   /// Name of the IR file that contains the result of the OpenMP target
   /// host code generation.
   std::string OMPHostIRFile;
 
   /// Indicates whether the front-end is explicitly told that the
   /// input is a header file (i.e. -x c-header).
   bool IsHeaderFile = false;
 
+  /// If set, dllexported classes dllexport their inline methods.
+  bool DllExportInlines = true;
+
   LangOptions();
 
   // Define accessors/mutators for language options of enumeration type.
 #define LANGOPT(Name, Bits, Default, Description)
 #define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
   Type get##Name() const { return static_cast<Type>(Name); } \
   void set##Name(Type Value) { Name = static_cast<unsigned>(Value); }
 #include "clang/Basic/LangOptions.def"
#include "clang/Basic/LangOptions.def"
 
   /// Return the OpenCL C or C++ version as a VersionTuple.
   VersionTuple getOpenCLVersionTuple() const;
 };
 
 /// Floating point control options
 class FPOptions {
 public:
-  FPOptions() : fp_contract(LangOptions::FPC_Off), 
+  FPOptions() : fp_contract(LangOptions::FPC_Off),
                 fenv_access(LangOptions::FEA_Off) {}
 
   // Used for serializing.
   explicit FPOptions(unsigned I)
       : fp_contract(static_cast<LangOptions::FPContractModeKind>(I & 3)),
         fenv_access(static_cast<LangOptions::FEnvAccessModeKind>((I >> 2) & 1))
         {}
 
public:
   }
 
   void setDisallowFEnvAccess() { fenv_access = LangOptions::FEA_Off; }
 
   /// Used to serialize this.
   unsigned getInt() const { return fp_contract | (fenv_access << 2); }
 
 private:
-  /// Adjust BinaryOperator::FPFeatures to match the total bit-field size 
+  /// Adjust BinaryOperator::FPFeatures to match the total bit-field size
   /// of these two.
   unsigned fp_contract : 2;
   unsigned fenv_access : 1;
 };
 
 /// Describes the kind of translation unit being processed.
 enum TranslationUnitKind {
   /// The translation unit is a complete translation unit.

clang/include/clang/Driver/CC1Options.td

 def compress_debug_sections_EQ : Joined<["-"], "compress-debug-sections=">,
     HelpText<"DWARF debug sections compression type">;
 def mno_exec_stack : Flag<["-"], "mnoexecstack">,
   HelpText<"Mark the file as not needing an executable stack">;
 def massembler_fatal_warnings : Flag<["-"], "massembler-fatal-warnings">,
   HelpText<"Make assembler warnings fatal">;
 def mrelax_relocations : Flag<["--"], "mrelax-relocations">,
     HelpText<"Use relaxable elf relocations">;
 def msave_temp_labels : Flag<["-"], "msave-temp-labels">,
   HelpText<"Save temporary labels in the symbol table. "
            "Note this may change .s semantics and shouldn't generally be used "
            "on compiler-generated code.">;
 def mrelocation_model : Separate<["-"], "mrelocation-model">,
   HelpText<"The relocation model to use">, Values<"static,pic,ropi,rwpi,ropi-rwpi,dynamic-no-pic">;
 def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
   HelpText<"Disable implicit builtin knowledge of math functions">;
 }

 def fno_debug_pass_manager : Flag<["-"], "fno-debug-pass-manager">,
     HelpText<"Disables debug printing for the new pass manager">;
 // The driver option takes the key as a parameter to the -msign-return-address=
 // and -mbranch-protection= options, but CC1 has a separate option so we
 // don't have to parse the parameter twice.
 def msign_return_address_key_EQ : Joined<["-"], "msign-return-address-key=">,
     Values<"a_key,b_key">;
 def mbranch_target_enforce : Flag<["-"], "mbranch-target-enforce">;
+def fno_dllexport_inlines : Flag<["-"], "fno-dllexport-inlines">;
 
 //===----------------------------------------------------------------------===//
 // Dependency Output Options
 //===----------------------------------------------------------------------===//
 
 def sys_header_deps : Flag<["-"], "sys-header-deps">,
   HelpText<"Include system headers in dependency output">;
 def module_file_deps : Flag<["-"], "module-file-deps">,

clang/include/clang/Driver/CLCompatOptions.td

def _SLASH_Yc : CLJoined<"Yc">,
   HelpText<"Generate a pch file for all code up to and including <filename>">,
   MetaVarName<"<filename>">;
 def _SLASH_Yu : CLJoined<"Yu">,
   HelpText<"Load a pch file and use it instead of all code up to "
            "and including <filename>">,
   MetaVarName<"<filename>">;
 def _SLASH_Y_ : CLFlag<"Y-">,
   HelpText<"Disable precompiled headers, overrides /Yc and /Yu">;
+def _SLASH_Zc_dllexportInlines : CLFlag<"Zc:dllexportInlines">;
+def _SLASH_Zc_dllexportInlines_ : CLFlag<"Zc:dllexportInlines-">;
 def _SLASH_Fp : CLJoined<"Fp">,
   HelpText<"Set pch filename (with /Yc and /Yu)">, MetaVarName<"<filename>">;
 
 def _SLASH_Gd : CLFlag<"Gd">,
   HelpText<"Set __cdecl as a default calling convention">;
 def _SLASH_Gr : CLFlag<"Gr">,
   HelpText<"Set __fastcall as a default calling convention">;
 def _SLASH_Gz : CLFlag<"Gz">,

clang/lib/Driver/ToolChains/Clang.cpp

else
     VolatileOptionID = options::OPT__SLASH_volatile_iso;
 
   if (Arg *A = Args.getLastArg(options::OPT__SLASH_volatile_Group))
     VolatileOptionID = A->getOption().getID();
 
   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)
     D.Diag(clang::diag::err_drv_argument_not_allowed_with)
         << MostGeneralArg->getAsString(Args) << BestCaseArg->getAsString(Args);
 
   if (MostGeneralArg) {
     Arg *SingleArg = Args.getLastArg(options::OPT__SLASH_vms);

clang/lib/Frontend/CompilerInvocation.cpp

#include "clang/Frontend/LangStandards.def"
       const LangStandard &Std = LangStandard::getLangStandardForKind(LangStd);
       if (!IsInputCompatibleWithStandard(IK, Std)) {
         Diags.Report(diag::err_drv_argument_not_allowed_with)
           << A->getAsString(Args) << GetInputKindName(IK);
       }
     }
   }
 
+  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") {
       Opts.CFProtectionBranch = 1;
     }
   }
   // -cl-std only applies for OpenCL language standards.
   // Override the -std option in this case.

clang/lib/Sema/SemaDecl.cpp

if (VD->getTLSKind() && !hasDependentAlignment(VD) &&
         Diag(VD->getLocation(), diag::err_tls_var_aligned_over_maximum)
           << (unsigned)Context.getDeclAlign(VD).getQuantity() << VD
           << (unsigned)MaxAlignChars.getQuantity();
       }
     }
   }
 
   if (VD->isStaticLocal()) {
-    if (FunctionDecl *FD =
-            dyn_cast_or_null<FunctionDecl>(VD->getParentFunctionOrMethod())) {
+    FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(VD->getParentFunctionOrMethod());
+
+    // Find outermost function when VD is in lambda function.
+    while (FD && !getDLLAttr(FD) &&
+           !FD->hasAttr<DLLExportStaticLocalAttr>() &&
+           !FD->hasAttr<DLLImportStaticLocalAttr>()) {
+      FD = dyn_cast_or_null<FunctionDecl>(FD->getParentFunctionOrMethod());
+    }
+
+    if (FD) {
       // Static locals inherit dll attributes from their function.
       if (Attr *A = getDLLAttr(FD)) {
         auto *NewAttr = cast<InheritableAttr>(A->clone(getASTContext()));
         NewAttr->setInherited(true);
         VD->addAttr(NewAttr);
+      } else if (Attr *A = FD->getAttr<DLLExportStaticLocalAttr>()) {
+        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<DLLExportAttr>())
+          FD->addAttr(NewAttr);
+
+      } else if (Attr *A = FD->getAttr<DLLImportStaticLocalAttr>()) {
+        auto *NewAttr = ::new (getASTContext()) DLLImportAttr(A->getRange(),
+                                                              getASTContext(),
+                                                              A->getSpellingListIndex());
+        NewAttr->setInherited(true);
+        VD->addAttr(NewAttr);
       }
+
       // CUDA 8.0 E.3.9.4: Within the body of a __device__ or __global__
       // function, only __shared__ variables or variables without any device
       // memory qualifiers may be declared with static storage class.
       // Note: It is unclear how a function-scope non-const static variable
       // without device memory qualifier is implemented, therefore only static
       // const variable without device memory qualifier is allowed.
       [&]() {
         if (!getLangOpts().CUDA)
Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Declarator &D,
   D.setFunctionDefinitionKind(FDK_Definition);
   Decl *DP = HandleDeclarator(ParentScope, D, TemplateParameterLists);
   return ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody);
 }
 
 void Sema::ActOnFinishInlineFunctionDef(FunctionDecl *D) {
   Consumer.HandleInlineFunctionDefinition(D);
 }
 
 static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
                              const FunctionDecl*& PossibleZeroParamPrototype) {
   // Don't warn about invalid declarations.
   if (FD->isInvalidDecl())
     return false;
 
   // Or declarations that aren't global.
   if (!FD->isGlobal())

clang/lib/Sema/SemaDeclCXX.cpp

for (Decl *Member : Class->decls()) {
 
     auto *MD = dyn_cast<CXXMethodDecl>(Member);
     if (!MD)
       continue;
 
     if (Member->getAttr<DLLExportAttr>()) {
       if (MD->isUserProvided()) {
         // Instantiate non-default class member functions ...
 
         // .. except for certain kinds of template specializations.
         if (TSK == TSK_ImplicitInstantiation && !ClassAttr->isInherited())
           continue;
 
         S.MarkFunctionReferenced(Class->getLocation(), MD);
 
         // The function will be passed to the consumer when its definition is
         // encountered.
       } else if (!MD->isTrivial() || MD->isExplicitlyDefaulted() ||
                  MD->isCopyAssignmentOperator() ||
                  MD->isMoveAssignmentOperator()) {
         // Synthesize and instantiate non-trivial implicit methods, explicitly
         // defaulted methods, and the copy and move assignment operators. The
         // latter are exported even if they are trivial, because the address of
         // an operator can be taken and should compare equal across libraries.
         DiagnosticErrorTrap Trap(S.Diags);
         S.MarkFunctionReferenced(Class->getLocation(), MD);
         if (Trap.hasErrorOccurred()) {
           S.Diag(ClassAttr->getLocation(), diag::note_due_to_dllexported_class)
               << Class << !S.getLangOpts().CPlusPlus11;
           break;
         }
 
         // There is no later point when we will see the definition of this
if (LastExportedDefaultCtor) {
       S.Diag(CD->getLocation(), diag::note_entity_declared_at)
           << CD->getDeclName();
       return;
     }
     LastExportedDefaultCtor = CD;
   }
 }
 
 void Sema::checkClassLevelCodeSegAttribute(CXXRecordDecl *Class) {
   // Mark any compiler-generated routines with the implicit code_seg attribute.
   for (auto *Method : Class->methods()) {
     if (Method->isUserProvided())
       continue;
     if (Attr *A = getImplicitCodeSegOrSectionAttrForFunction(Method, /*IsDefinition=*/true))
       Method->addAttr(A);
   }
 }
 
 /// Check class-level dllimport/dllexport attribute.
 void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
   Attr *ClassAttr = getDLLAttr(Class);
 
   // MSVC inherits DLL attributes to partial class template specializations.
   if (Context.getTargetInfo().getCXXABI().isMicrosoft() && !ClassAttr) {
     if (auto *Spec = dyn_cast<ClassTemplatePartialSpecializationDecl>(Class)) {
       if (Attr *TemplateAttr =
               getDLLAttr(Spec->getSpecializedTemplate()->getTemplatedDecl())) {
         auto *A = cast<InheritableAttr>(TemplateAttr->clone(getASTContext()));
         A->setInherited(true);
         ClassAttr = A;
       }
     }
   }
 
   if (!ClassAttr)
     return;
 
   if (!Class->isExternallyVisible()) {
     Diag(Class->getLocation(), diag::err_attribute_dll_not_extern)
         << Class << ClassAttr;
     return;
   }
if (MD) {
       if (MD->isInlined()) {
         // MinGW does not import or export inline methods.
         if (!Context.getTargetInfo().getCXXABI().isMicrosoft() &&
             !Context.getTargetInfo().getTriple().isWindowsItaniumEnvironment())
           continue;
 
         // MSVC versions before 2015 don't export the move assignment operators
         // and move constructor, so don't attempt to import/export them if
         // we have a definition.
         auto *Ctor = dyn_cast<CXXConstructorDecl>(MD);
         if ((MD->isMoveAssignmentOperator() ||
              (Ctor && Ctor->isMoveConstructor())) &&
             !getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2015))
           continue;
 
         // MSVC2015 doesn't export trivial defaulted x-tor but copy assign
         // operator is exported anyway.
         if (getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2015) &&
             (Ctor || isa<CXXDestructorDecl>(MD)) && MD->isTrivial())
           continue;
       }
     }
 
     // Don't apply dllimport attributes to static data members of class template
     // instantiations when the attribute is propagated from a derived class.
     if (VD && PropagatedImport)
       continue;
 
     if (!cast<NamedDecl>(Member)->isExternallyVisible())
       continue;
 
     if (!getDLLAttr(Member)) {
-      auto *NewAttr =
-          cast<InheritableAttr>(ClassAttr->clone(getASTContext()));
+      InheritableAttr *NewAttr = nullptr;
+
+      // Do not export/import inline function when -fno-dllexport-inlines is
+      // passed and it is not part of explicit template instantiation.
+      // 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<InheritableAttr>(ClassAttr->clone(getASTContext()));
+      }
+
       NewAttr->setInherited(true);
       Member->addAttr(NewAttr);
 
       if (MD) {
         // Propagate DLLAttr to friend re-declarations of MD that have already
         // been constructed.
         for (FunctionDecl *FD = MD->getMostRecentDecl(); FD;
              FD = FD->getPreviousDecl()) {

clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp

+// 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<typename T>
+class __declspec(dllexport) TemplateExportedClass {
+  void InclassDefFunc() {}
+  void OutclassDefFunc();
+
+  T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template<typename T> void TemplateExportedClass<T>::OutclassDefFunc() {}
+
+class A11{};
+class B22{};
+class C33{};
+class D44{};
+
+// INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VA11@@@@AEAAXXZ"
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?OutclassDefFunc@?$TemplateExportedClass@VA11@@@@AEAAXXZ"
+template class __declspec(dllexport) TemplateExportedClass<A11>;
+
+// INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateExportedClass@VB22@@@@AEAAXXZ"
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?OutclassDefFunc@?$TemplateExportedClass@VB22@@@@AEAAXXZ
+template class TemplateExportedClass<B22>;
+
+
+template<typename T>
+class TemplateNoExportedClass {
+public:
+  void InclassDefFunc() {}
+  void OutclassDefFunc();
+};
+
+template<typename T> void TemplateNoExportedClass<T>::OutclassDefFunc() {}
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@?$TemplateNoExportedClass@VA11@@@@QEAAXXZ"
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?OutclassDefFunc@?$TemplateNoExportedClass@VA11@@@@QEAAXXZ"
+template class __declspec(dllexport) TemplateNoExportedClass<A11>;
+
+// DEFAULT-DAG: define weak_odr dso_local void @"?InclassDefFunc@?$TemplateNoExportedClass@VB22@@@@QEAAXXZ"
+// DEFAULT-DAG: define weak_odr dso_local void @"?OutclassDefFunc@?$TemplateNoExportedClass@VB22@@@@QEAAXXZ"
+template class TemplateNoExportedClass<B22>;
+
+// DEFAULT-DAG: declare dso_local void @"?InclassDefFunc@?$TemplateNoExportedClass@VC33@@@@QEAAXXZ"
+// DEFAULT-NOT: ?OutclassDefFunc@?$TemplateNoExportedClass@VC33@
+extern template class TemplateNoExportedClass<C33>;
+
+// DEFAULT-DAG: declare dllimport void @"?InclassDefFunc@?$TemplateNoExportedClass@VD44@@@@QEAAXXZ"
+// DEFAULT-NOT: ?OutclassDefFunc@?$TemplateNoExportedClass@VD44@
+extern template class __declspec(dllimport) TemplateNoExportedClass<D44>;
+
+void ExplicitTemplateDeclarationUser() {
+  TemplateNoExportedClass<C33> c33;
+  c33.InclassDefFunc();
+
+  TemplateNoExportedClass<D44> d44;
+  d44.InclassDefFunc();
+}
+
+class __declspec(dllimport) ImportedClass {
+public:
+  class ImportedInnerClass {
+   public:
+    void OutClassDefFunc();
+  };
+
+  // INLINE-DAG: declare dllimport i32 @"?InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ"(%class.ImportedClass*) #2
+  // NOINLINE-NOT: declare{{.*}}"?InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ"
+  int InClassDefFuncWithStaticVariable() {
+    static int static_variable = 0;
+    ++static_variable;
+    return static_variable;
+  }
+
+  // NOINLINE-DAG: define linkonce_odr dso_local void @"?InClassDefFunc@ImportedClass@@QEAAXXZ"
+  // INLINE-DAG: declare dllimport void @"?InClassDefFunc@ImportedClass@@QEAAXXZ"
+  void InClassDefFunc() {
+    i.OutClassDefFunc();
+  }
+  ImportedInnerClass i;
+};
+
+int InClassDefFuncUser() {
+  // This is necessary for declare statement of ImportedClass::InClassDefFunc().
+  ImportedClass c;
+  c.InClassDefFunc();
+  return c.InClassDefFuncWithStaticVariable();
+}