[PATCH] D142317: [Support][LLD] Avoid using uninitialized threadIndex.

[Andrew Ng via Phabricator via llvm-commits]

andrewng added a comment.

> I did not find direct specification of that behavior(`thread_local` variables are zero initialized) thus created a patch which initializes it.

Did you see actual issues arising from `threadIndex` being uninitialized? If so, what platform and toolchain were you using?

> Another thing, as mentioned in above quote, is that main thread with zero index clashes with first thread created by  `parallelFor`.

See my suggestion.



================
Comment at: llvm/lib/Support/Parallel.cpp:231-232
     }
     for (; Begin != End; ++Begin)
       Fn(Begin);
     return;
----------------
andrewng wrote:
> Perhaps a "simpler" fix would be not to use the main thread here, i.e. should use `TG.spawn`? However, that assumes that `getThreadIndex()` is only for use in the "`parallelFor`" style functions otherwise the other parallel functions would also need to avoid using the main thread too.
> 
> I'm surprised that this hasn't been causing more issues.
My gut feeling is that this would be the "better" solution, i.e. not including the invoking thread (might not be the main thread) as part of the pool of worker threads. It just feels cleaner to me.

However, can we also assume that the use of `getThreadIndex()` is not supported with regards to `parallel_sort` which I believe can also make use of the invoking thread? I think this would be reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142317