[Reland][ADT][ConcurrentHashTable] adapt ConcurrentHashTable and its users to LLVM_ENABLE_THREADS=0 mode.

[avl]

Summary

This patch hides thread specific handling under LLVM_ENABLE_THREADS guard.
It also removes usages of thread_local variables, since it has a weak
support on some platforms. Instead, the patch uses single mutex for locking
allocator. That may be replaced with more effective allocator later.
f.e. D142318

[SeanP]

Thanks for the update. I tried this on z/OS and it is working.

[avl]

Thank you for the review.

[vitalybuka]

This is memory leak now https://lab.llvm.org/buildbot/#/builders/5/builds/32731

[avl]

cure buildbot failure. refactor.

[avl]

Original patch broke buildbots with memory leak. This new version removes usages of thread_local variables and adds checks for LLVM_ENABLE_THREADS

[JDevlieghere]

It's a bit unfortunate that we have to sprinkle LLVM_ENABLE_THREADS throughout the code, but not enough that I think it's worth doing anything fancy like subclassing (I think it would just make things worse). LGTM.

llvm/include/llvm/DWARFLinkerParallel/StringPool.h

25 ↗

(On Diff #511378)

Why not just StringAllocator. Do we expect to have a more "complex" allocator down the line?

[avl]

llvm/include/llvm/DWARFLinkerParallel/StringPool.h

25 ↗

(On Diff #511378)

I am planning to use allocator from D142318 later.

Will rename SimpleStringAllocator to StringAllocator as suggested.

[SeanP]

LGTM

I tested this patch on z/os. Everything is working. Thanks

[avl]

Thanks!

llvm/unittests/ADT/ConcurrentHashtableTest.cpp

 
 protected:
   String(const std::string &Num) { Data += Num; }
 
   std::string Data;
   std::array<char, 0x20> ExtraData;
 };
 
-static thread_local BumpPtrAllocator ThreadLocalAllocator;
+static LLVM_THREAD_LOCAL BumpPtrAllocator *ThreadLocalAllocator = nullptr;
 class PerThreadAllocator : public AllocatorBase<PerThreadAllocator> {
 public:
   inline LLVM_ATTRIBUTE_RETURNS_NONNULL void *Allocate(size_t Size,
                                                        size_t Alignment) {
-    return ThreadLocalAllocator.Allocate(Size, Align(Alignment));
+    return getAllocatorPtr()->Allocate(Size, Align(Alignment));
   }
-  inline size_t getBytesAllocated() const {
-    return ThreadLocalAllocator.getBytesAllocated();
+  inline size_t getBytesAllocated() {
+    return getAllocatorPtr()->getBytesAllocated();
   }
 
   // Pull in base class overloads.
   using AllocatorBase<PerThreadAllocator>::Allocate;
+
+protected:
+  BumpPtrAllocator *getAllocatorPtr() {
+    if (ThreadLocalAllocator == nullptr)
+      ThreadLocalAllocator = new BumpPtrAllocator();
+
+    return ThreadLocalAllocator;
+  }
 } Allocator;
 
 TEST(ConcurrentHashTableTest, AddStringEntries) {
   ConcurrentHashTableByPtr<
       std::string, String, PerThreadAllocator,
       ConcurrentHashTableInfoByPtr<std::string, String, PerThreadAllocator>>
       HashTable(Allocator, 10);