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

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

In D53153#1315109 <https://reviews.llvm.org/D53153#1315109>, @rjmccall wrote:

> Okay.  So it's still the case that all symbols will be defined within the linkage unit; it's just that some things might need to get exposed outside of it.
>
> LLVM does provide a `dso_local` attribute which you could use unconditionally on every symbol without actually changing visibility.


It seems like `dso_local` is an IR concept, and is a guarantee of non-preemptability? Marking e.g. a global variable as `dso_local` in IR doesn't seem to affect what LLD infers about it;`computeIsPreemptible` in the ELF writer still believes it is preemptable, and so LLD complains when it attempts to generate a dynamic relocation in a read-only section (i.e. text). Is `dso_local` meant to convey anything to the linker?

If this is the intended behavior we could still tell LLD to assume all defined symbols are not preemptable with something like `-Bsymbolic`. If we support dynamic linking/preemption in the future we will have to revisit this, but at that time visibility will be meaningful.

I suppose one approach is then:

- Remove the implicit `-fvisibility=hidden` in the AMDGPU Clang toolchain
- Add a module pass to mark all global values with `dso_local`
- Add an implicit `-Bsymbolic` to the linker commandline


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

https://reviews.llvm.org/D53153