[Openmp-commits] [PATCH] D64080: [OPENMP]Make __kmpc_push_tripcount thread safe.

[Alexey Bataev via Phabricator via Openmp-commits]

ABataev added a comment.

In D64080#1569363 <https://reviews.llvm.org/D64080#1569363>, @hfinkel wrote:

> In D64080#1569334 <https://reviews.llvm.org/D64080#1569334>, @ABataev wrote:
>
> > In D64080#1569319 <https://reviews.llvm.org/D64080#1569319>, @hfinkel wrote:
> >
> > > In D64080#1569299 <https://reviews.llvm.org/D64080#1569299>, @ABataev wrote:
> > >
> > > > In D64080#1569266 <https://reviews.llvm.org/D64080#1569266>, @hfinkel wrote:
> > > >
> > > > > In D64080#1568348 <https://reviews.llvm.org/D64080#1568348>, @ABataev wrote:
> > > > >
> > > > > > Added note.
> > > > >
> > > > >
> > > > > The patch description says that you add a trip count "per thread", but the code is adding a trip count per device. Could this still cause a problem if multiple threads have target regions for the same device?
> > > >
> > > >
> > > > No. We already have tripcount per device: each device stores its unique tripcount value, but only one. I extended the existing solution to have the tripcount not only per device, but also per host thread.
> > >
> > >
> > > Would there be any advantage to using `__kmpc_threadprivate_register` and `__kmpc_threadprivate` (or similar) instead of storing a map with thread ids?
> >
> >
> > Hard to tell, but definitely ee increase dependency between libomptarget and libomp. and,  seems to me, threadprivate interface also uses some kind of hash table. We can use unordered_map instead.
>
>
> Given that we expect this to be temporary, I'm okay with it either way. unordered_map is probably a little bit better than std::map. I expect the number of entries in the map to be small because __kmpc_global_thread_num returns OpenMP thread ids, not system TIDs, right?


Yes.


Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D64080