[PATCH] D53153: [OpenCL] Mark kernel functions with default visibility

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D53153#1353256 <https://reviews.llvm.org/D53153#1353256>, @scott.linder wrote:

> In D53153#1317977 <https://reviews.llvm.org/D53153#1317977>, @rjmccall wrote:
>
> > I think `-fvisibility=hidden` isn't good enough because you want to infer hidden visibility even for external symbol references, and that's just not how global visibility works.  But after this discussion, I'm prepared to accept that (1) we should have some sort of single-image compiler mode that implies that all symbols are defined within the image and (2) you can make your toolchain imply that together with `-fvisibility=hidden` and then have specific symbols opt in to non-hidden visibility if they need to be accessible to whatever loader / runtime you have.
>
>
> It seems that explicit visibility attributes on external symbol references (e.g. `__attribute__((visibility("hidden"))) extern int foo;`) are respected in Clang, so I don't understand the rationale for global visibility controls not applying to them as well. Can you describe why this is the case?


Well, one answer is that that's the behavior that GCC defined when they added global visibility controls, and it would be unreasonable for Clang to deviate in such a fundamental way.  However, we do deviate in some other ways in our interpretation of visibility attributes, so that's not a totally satisfactory answer.

The stronger answer is that GCC has a very good reason for this behavior.  Global visibility controls apply to all code in the translation unit.  While programmers often mentally distinguish between "project" headers (which they control) and "library" headers (which they do not), that is not a difference that is known to the compiler.  Therefore, if global visibility controls applied to external symbols, the compiler would end up assuming that all external symbols not annotated as `default` are provided by the current image.  That assumption would have been incompatible with essentially all existing headers for dynamic libraries on GCC's core targets, including system headers; in other words, it would have caused ubiquitous miscompiles.*  It would be completely unreasonable to expect all those headers to be changed just for the benefit of a single new compiler feature.  Therefore, global visibility controls do not apply to external symbols because doing so would make it impossible to actually enable global visibility controls.

- GCC's rule helps a great deal even in C++, which is generally more sensitive to global visibility: you can typically use an unannotated C++ header under `-fvisibility=hidden` and have no problems unless you start using its types as exceptions (and GCC tries to work around that, too) or comparing the addresses of inline functions or template instantiations.

John.


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

https://reviews.llvm.org/D53153