[PATCH] D153318: [llvm] Refactor BalancedPartitioning for fixing build failure with MSVC

Ellis Hoag via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 20 18:12:31 PDT 2023


ellis added a comment.

In D153318#4436375 <https://reviews.llvm.org/D153318#4436375>, @kamleshbhalui wrote:

> ThreadPool.h ends up including immintrin.h from MSVC after builtin generation in clang and It has definitions for replacing one builtins with another.
> Clang maintain a enum for all generated builtin but due to above macro replacement enum ends up having two element with same name and it causes multiple definitions error.
> There are some other instances where ThreadPool.h is not included directly(i.e. Debuginfod.h).

I'm not understanding why this change fixes that since we are still including `ThreadPool.h` in `BalancedPartitioning.cpp`. This seems like something that should be fixed in `ThreadPool.h` itself.



================
Comment at: llvm/include/llvm/Support/BalancedPartitioning.h:118
   struct BPThreadPool {
-    ThreadPool TheThreadPool;
+    ThreadPool &TheThreadPool;
     std::mutex mtx;
----------------
Why not keep `ThreadPool` owned by `BPThreadPool`? Why make this an alias?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153318



More information about the llvm-commits mailing list