[Openmp-commits] [openmp] Draft: [OpenMP] Fix td_tdg_task_id underflow with taskloop and taskgraph (PR #150877)

[Josep Pinot via Openmp-commits]

jpinot wrote:

> Even with a localized counter as implemented with your latest commit, I think this decrement can be problematic.
> 
> Is it really important to avoid spurious holes in the numbering of task IDs? As I understand, this is just used for accessing the `record_map` entries (which is actually a vector, not a map). Checking the code, I realized that for resizing the vector, the code will do a lot of rather small allocs for `successorList`, that might never be used. I would recommend to delay these allocs until the entry is really used.
> 
> A bigger issue is, that the accesses to the `record_map` are not protected. If one thread triggers the resize code for the vector, other threads might still access the vector and writes might get lost.

Yes, the decrement is bit of a haky solution. The problem is that the `td_tdg_task_id` is also use as a `record_map` [index](https://github.com/llvm/llvm-project/blob/main/openmp/runtime/src/kmp_tasking.cpp#L1791) , Conesquently, (1) need to be sequential from 0 for each tdg, and (2) free of any spurious holes; in current implementation (could be converted to a proper map and hash the tasks id's).

Regarding the `record_map` not being protected, I already have changes for this(protecting the resize of `record_map` and `successors`) and will try to create a draft pull request today so you could also have a look.

I'll also take a look at the `successorList` as you suggested.

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