[PATCH] D148916: [Support][Parallel] Initialize threadIndex and add assertion checking its usage.

[Andrew Ng via Phabricator via llvm-commits]

andrewng added inline comments.


================
Comment at: llvm/include/llvm/Support/Parallel.h:32-37
+#define GET_THREAD_INDEX_IMPL                                                  \
+  assert((threadIndex != UINT_MAX) &&                                          \
+         "getThreadIndex() must be called from the thread created by "         \
+         "ThreadPoolExecutor");                                                \
+  return threadIndex;
+
----------------
I think this can be moved within the `#if LLVM_ENABLE_THREADS` just below.

Perhaps `the` -> `a` in the assert string.


================
Comment at: llvm/lib/Support/Parallel.cpp:103
+      if (parallel::strategy.ThreadsRequested == 1) {
+        parallel::threadIndex = 0;
+        F();
----------------
This setting of `parallel::threadIndex = 0` (also in `llvm::parallelFor`) feels a little "odd". Just wondering if an alternative would be to check `parallel::strategy.ThreadsRequested == 1` in `getThreadIndex()` and return `0` there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148916