[Libomptarget] Fix external visibility for internal variables

[jhuber6]

Summary

After the changes in D117362 made variables declared inside of a target
declare directive visible outside the plugin, some variables inside the
runtime were given visiblity that conflicted with their address space
type. This caused problems when shared or local memory was made
externally visible. This patch fixes this issue by making these
varialbes static within the module, therefore limiting their visibility
to being internal.

[jhuber6]

Add visibility attribute to local data.

[jdoerfert]

LG, even though we need to get rid of the non THREAD_LOCAL globals asap.

[ronlieb]

Hi Joseph,
3 commits came in, one bot exited with a fail
https://lab.llvm.org/buildbot/#/builders/193/builds/4828

[jhuber6]

In D117526#3251720, @ronlieb wrote:

Hi Joseph,
3 commits came in, one bot exited with a fail
https://lab.llvm.org/buildbot/#/builders/193/builds/4828

Is it possible that AMDGPU doesn't support static on these variables? I tested https://reviews.llvm.org/D117362 on my AMDGPU machine and didn't notice any problems, but didn't test with this one applies. I can test locally but it'll take awhile to rebuild on AMDGPU.

[jhuber6]

In D117526#3251720, @ronlieb wrote:

Hi Joseph,
3 commits came in, one bot exited with a fail
https://lab.llvm.org/buildbot/#/builders/193/builds/4828

I get a crash compiling the shared object, can you confirm that on your end? I'm not sure what this would've changed that would break it.

[ronlieb]

i will give it a try locally ...

[ronlieb]

i dont see the failures locally, so i am now getting on the buildbot system to see what happens

[jhuber6]

In D117526#3251828, @ronlieb wrote:

i will give it a try locally ...

The issue I got locally occurs without my patches so I'm assuming it's not related, running this
clang dynamic_module.c -fopenmp -fopenmp-targets=amdgcn -fPIC -shared -o out.so && clang out.so -fopenmp -fopenmp-targets=amdgcn
Gives me the following crash upstream,

clang: /ccs/home/jhuber/llvm/ref/llvm-project/llvm/include/llvm/ADT/SmallVector.h:273: T& llvm::SmallVectorTemplateCommon<T, <template-parameter-1-2> >::operator[](llvm::SmallVectorTemplateCommon<T, <template-parameter-1-2> >::size_type) [with T = clang::driver::InputInfo; <template-parameter-1-2> = void; llvm::SmallVectorTemplateCommon<T, <template-parameter-1-2> >::reference = clang::driver::InputInfo&; llvm::SmallVectorTemplateCommon<T, <template-parameter-1-2> >::size_type = long unsigned int]: Assertion `idx < size()' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: clang out.so -fopenmp -fopenmp-targets=amdgcn
1.	Compilation construction
2.	Building compilation jobs
3.	Building compilation jobs
4.	Building compilation jobs
5.	Building compilation jobs
6.	Building compilation jobs
7.	Building compilation jobs
 #0 0x00007f1cd13d282f PrintStackTraceSignalHandler(void*) Signals.cpp:0:0
 #1 0x00007f1cd13d00d9 SignalHandler(int) Signals.cpp:0:0
 #2 0x00007f1cd8a632d0 __restore_rt (/lib64/libpthread.so.0+0x132d0)
 #3 0x00007f1cd054c420 raise (/lib64/libc.so.6+0x39420)
 #4 0x00007f1cd054da01 abort (/lib64/libc.so.6+0x3aa01)
 #5 0x00007f1cd0544a1a __assert_fail_base (/lib64/libc.so.6+0x31a1a)
 #6 0x00007f1cd0544a92 (/lib64/libc.so.6+0x31a92)
 #7 0x00007f1cd4f41c8d clang::driver::Driver::BuildJobsForActionNoCache(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0xa0c8d)
 #8 0x00007f1cd4f41ec2 clang::driver::Driver::BuildJobsForAction(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0xa0ec2)
 #9 0x00007f1cd4f42366 void llvm::function_ref<void (clang::driver::Action*, clang::driver::ToolChain const*, char const*)>::callback_fn<clang::driver::Driver::BuildJobsForActionNoCache(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const::'lambda'(clang::driver::Action*, clang::driver::ToolChain const*, char const*)>(long, clang::driver::Action*, clang::driver::ToolChain const*, char const*) Driver.cpp:0:0
#10 0x00007f1cd4f176fc clang::driver::OffloadAction::doOnEachDeviceDependence(llvm::function_ref<void (clang::driver::Action*, clang::driver::ToolChain const*, char const*)> const&) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0x766fc)
#11 0x00007f1cd4f3f2fb clang::driver::Driver::BuildJobsForActionNoCache(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0x9e2fb)
#12 0x00007f1cd4f41ec2 clang::driver::Driver::BuildJobsForAction(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0xa0ec2)
#13 0x00007f1cd4f40682 clang::driver::Driver::BuildJobsForActionNoCache(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0x9f682)
#14 0x00007f1cd4f41ec2 clang::driver::Driver::BuildJobsForAction(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0xa0ec2)
#15 0x00007f1cd4f40682 clang::driver::Driver::BuildJobsForActionNoCache(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0x9f682)
#16 0x00007f1cd4f41ec2 clang::driver::Driver::BuildJobsForAction(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0xa0ec2)
#17 0x00007f1cd4f3fed1 clang::driver::Driver::BuildJobsForActionNoCache(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0x9eed1)
#18 0x00007f1cd4f41ec2 clang::driver::Driver::BuildJobsForAction(clang::driver::Compilation&, clang::driver::Action const*, clang::driver::ToolChain const*, llvm::StringRef, bool, bool, char const*, std::map<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, clang::driver::InputInfo, std::less<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::allocator<std::pair<std::pair<clang::driver::Action const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > const, clang::driver::InputInfo> > >&, clang::driver::Action::OffloadKind) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0xa0ec2)
#19 0x00007f1cd4f4265b clang::driver::Driver::BuildJobs(clang::driver::Compilation&) const (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0xa165b)
#20 0x00007f1cd4f43e03 clang::driver::Driver::BuildCompilation(llvm::ArrayRef<char const*>) (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/lib/libclangDriver.so.14git+0xa2e03)
#21 0x000000000040e4b7 main (/autofs/nccs-svm1_home1/jhuber/llvm/ref/clang/bin/clang-14+0x40e4b7)
#22 0x00007f1cd053734a __libc_start_main (/lib64/libc.so.6+0x2434a)
#23 0x000000000041079a _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:122:0
[1]    185031 abort      clang out.so -fopenmp -fopenmp-targets=amdgcn

This doesn't happen using my new driver also.

[JonChesterfield]

In D117526#3251741, @jhuber6 wrote:

In D117526#3251720, @ronlieb wrote:

Hi Joseph,
3 commits came in, one bot exited with a fail
https://lab.llvm.org/buildbot/#/builders/193/builds/4828

Is it possible that AMDGPU doesn't support static on these variables? I tested https://reviews.llvm.org/D117362 on my AMDGPU machine and didn't notice any problems, but didn't test with this one applies. I can test locally but it'll take awhile to rebuild on AMDGPU.

Previously, static shared variables did not work. Seems plausible that static thread local variables won't work either. Address space 5 roughly means __private, i.e. on the stack alloca style. I haven't worked through what this variable is used for yet.

[JonChesterfield]

openmp/libomptarget/DeviceRTL/include/Types.h

195

Why would we want a visibility tag on stack variables? They shouldn't be in the elf at all

openmp/libomptarget/DeviceRTL/include/Types.h

 #define OMP_PRAGMA(STR) __PRAGMA(omp STR)
 
 #define SHARED(NAME)                                                           \
   NAME [[clang::loader_uninitialized]];                                        \
   OMP_PRAGMA(allocate(NAME) allocator(omp_pteam_mem_alloc))
 
 // TODO: clang should use address space 5 for omp_thread_mem_alloc, but right
 //       now that's not the case.
 #define THREAD_LOCAL(NAME)                                                     \
-  NAME [[clang::loader_uninitialized, clang::address_space(5)]]
+  NAME [[clang::loader_uninitialized, clang::address_space(5),                 \
+         gnu::visibility("hidden")]]
 
 // TODO: clang should use address space 4 for omp_const_mem_alloc, maybe it
 //       does?
 #define CONSTANT(NAME)                                                         \
   NAME [[clang::loader_uninitialized, clang::address_space(4)]]
 
 ///}
 
 #endif

openmp/libomptarget/DeviceRTL/src/Workshare.cpp

 // used by dynamic scheduling
 #define FINISHED 0
 #define NOT_FINISHED 1
 #define LAST_CHUNK 2
 
 #pragma omp declare target
 
 // TODO: This variable is a hack inherited from the old runtime.
-uint64_t SHARED(Cnt);
+static uint64_t SHARED(Cnt);
 
 template <typename T, typename ST> struct omptarget_nvptx_LoopSupport {
   ////////////////////////////////////////////////////////////////////////////////
   // Loop with static scheduling with chunk
 
   // Generic implementation of OMP loop scheduling with static policy
   /*! \brief Calculate initial bounds for static loop and stride
    *  @param[in] loc location in code of the call (not used here)

 };
 
 ////////////////////////////////////////////////////////////////////////////////
 // KMP interface implementation (dyn loops)
 ////////////////////////////////////////////////////////////////////////////////
 
 // TODO: This is a stopgap. We probably want to expand the dispatch API to take
 //       an DST pointer which can then be allocated properly without malloc.
-DynamicScheduleTracker *THREAD_LOCAL(ThreadDSTPtr);
+static DynamicScheduleTracker *THREAD_LOCAL(ThreadDSTPtr);
 
 // Create a new DST, link the current one, and define the new as current.
 static DynamicScheduleTracker *pushDST() {
   DynamicScheduleTracker *NewDST = static_cast<DynamicScheduleTracker *>(
       memory::allocGlobal(sizeof(DynamicScheduleTracker), "new DST"));
   *NewDST = DynamicScheduleTracker({0});
   NewDST->NextDST = ThreadDSTPtr;
   ThreadDSTPtr = NewDST;