[lld] [llvm] [LLD] Avoid non-deterministic relocations processing. (PR #107186)

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 19 13:41:28 PDT 2024


MaskRay wrote:

> > Shall we revert [reviews.llvm.org/D148728](https://reviews.llvm.org/D148728) but possibly keep the `spawn` helper to allow ` tg.spawn([]{fn();}, condition)`? lld is the only `spawn` caller that may specify the `Sequential` parameter.
> > The downside is that we need `threadIndex==0` for the main thread in the sequential case. We cannot keep the `threadIndex != UINT_MAX` assertion. I feel that the value of the assertion is low.
> > If needed, the assertion can be restored using a different mechanism:
> > 
> > * initialize `threadIndex` to UINT_MAX
> > * make the sequential mode temporarily set `threadIndex` to 0, run the task, restore `threadIndex`
> 
> We can not temporarily set `threadIndex` to 0, run the task, restore `threadIndex`. As it could clash with another running thread using thread_index 0.
> 
> The assertion exactly prevent from the situation when we could have thread_index clashed. It seems that assertion has useful value.
> 
> The alternative solution might be to have a separate thread and condition variable for the "sequential" tasks. In this way the thread indexes would not be clashed, "sequential" tasks would have the same order and thread index. I uploaded this possible solution, though it still not fully finished.

Adding to the comment https://github.com/llvm/llvm-project/pull/109084#discussion_r1767560904 : I feel that maintaining the sequential mode in Parallel.cpp adds too much complexity (`WorkQueueSequential`, `hasSequentialTasks()`).
While the overhead might be small, it is measurable (also see https://reviews.llvm.org/D148728#inline-1439381).

My current feeling is that we should restore the original state before https://reviews.llvm.org/D148728 . 

https://github.com/llvm/llvm-project/pull/107186


More information about the llvm-commits mailing list