[Hexagon][runtime] Make HexagonThreadManager::CheckSemaphore thread safe

[janetsc]

Ensure that only one thread can add a semaphore if it doesn't already exist.

cc: @JosephTheOctonaut

[tvm-bot]

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

• cc @Icemist, @areusch, @ibsidorenko, @mehrdadh, @quic-sanirudh _{See #10317 for details}

_{Generated by tvm-bot}

[csullivan]

LGTM, thanks!

[Lunderberg]

I think there's still a case where this could result in a race condition, though it's much less likely than before. The race condition would occur between one thread that is reading semaphores_[0] inside HexagonThreadManager::Signal, while another thread is writing to semaphores_[1] inside HexagonThreadManager::CheckSemaphore. The const methods of std::unordered_map are safe to run at the same time as other const methods, but aren't safe to run at the same time as non-const methods.

I don't think we need to change anything now, but if we have mystery multi-threading bugs in the future, it may be worth revisiting. The changes that we could make would be as follows:

1. Change signature from void CheckSemaphor(unsigned syncID), the function could instead be qurt_sem_t* GetSemaphore(unsigned syncID). This way, we don't need the call to semaphores_[syncID] inside HexagonThreadManager::Signal and HexagonThreadManager::Wait.
2. Thread-local cache of the map from syncID to qurt_sem_t*. The thread-local cache can be checked without a mutex, preserving the lockfree fast path. If the thread-local cache doesn't have the desired semaphore, then a mutex can protect the global semaphore map, to either read from it (if already created on another thread) or write to it (if not already found).

[janetsc]

That's a good point - thank you! I'll file a task for this.