[PATCH] D142318: [Support] Add PerThreadBumpPtrAllocator class.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/Support/PerThreadBumpPtrAllocator.h:31
+public:
+  PerThreadAllocator() { Allocators.resize(parallel::getMaxThreadsNum()); }
+
----------------
initialize `Allocators` instead of initializing it to empty then calling `resize`


================
Comment at: llvm/include/llvm/Support/PerThreadBumpPtrAllocator.h:36
+
+  // Pull in base class overloads.
+  using AllocatorBase<PerThreadAllocator<AllocatorTy>>::Deallocate;
----------------
Delete this comment and the blank line above. One `// Pull in base class overloads.` suffices. Actually the code self complains, so I personally prefer removing the comment completely.


================
Comment at: llvm/include/llvm/Support/PerThreadBumpPtrAllocator.h:94
+protected:
+  SmallVector<AllocatorTy> Allocators;
+};
----------------
Since this cannot be resized, we can use `unique_ptr<AllocatorTy[]>` and a separate size variable.


================
Comment at: llvm/lib/Support/Parallel.cpp:164
+
+size_t getMaxThreadsNum() {
+  return detail::Executor::getDefaultExecutor()->getThreadsNum();
----------------
A thread ID is smaller than this number, so the name seems confusing. Use `getNumThreads`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142318