[PATCH] D53153: [OpenCL] Mark namespace scope variables and kernel functions with default visibility

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

In https://reviews.llvm.org/D53153#1263882, @rsmith wrote:

> In https://reviews.llvm.org/D53153#1263848, @scott.linder wrote:
>
> > Beyond constructors/destructors I believe an API which we implement through access to dynamic symbols for global variable is `clGetProgramBuildInfo(..., CL_PROGRAM_BUILD_GLOBAL_VARIABLE_TOTAL_SIZE, ...)` which is defined as "The total amount of storage, in bytes, used by program variables in the global address space."
>
>
> Presumably this is looking at the size of a certain section / segment of the image. It's unclear to me why symbol visibility would affect this in any way.


You're right, the OpenCL APIs do not seem to require access to these symbols, as inspecting !X segments is sufficient. I removed global variable declarations from the patch.

>> As for kernels, an example of an API which we implement through access to kernel symbols is `clGetProgramInfo(..., CL_PROGRAM_KERNEL_NAMES, ...)`
> 
> Sure. `kernel`, like `dllexport` or `visibility(default)`, seems to act as declaring a function to be part of the external interface of the code, and it makes sense for all of those to override a command-line `-fvisibility` flag.
> 
> But it's still unclear to me what effect someone would expect `-fvisibility=hidden` to have when compiling OpenCL code. What are these symbols being hidden-by-default *from*, if not the host-side OpenCL runtime? Is there ever actually have a device-side dynamic linker?

Currently for AMDGPU it is an optimization, because if the symbol is non-preemptible we can emit a static PCREL relocation and avoid a load from the GOT. We do not currently support device-side dynamic linking for e.g. lazy binding, so hiding non-kernel symbols is OK. When we begin to support device-side linking we can reconsider the blanket directive. In general I think someone compiling OpenCL code with this option would not expect `kernel` symbols to be hidden, as you mention; as far as what they intend to hide the symbols from I think that depends on how the implementation handles linking.


https://reviews.llvm.org/D53153