Silence Thread Sanitizer in InitRandom
#4281
Conversation
Either making the shared variables constant (not sensible in the function signature) or using `firstprivate` should silence the warnings we see.
|
It does not seem to make thread sanitizer happy. Another thing you can try is to turn |
Src/Base/AMReX_Random.cpp
Outdated
| #ifdef AMREX_USE_OMP | ||
| #pragma omp parallel | ||
| #pragma omp parallel for | ||
| #endif | ||
| for (int tid = 0; tid < nthreads; tid++) { |
There was a problem hiding this comment.
| #ifdef AMREX_USE_OMP | |
| #pragma omp parallel | |
| #pragma omp parallel for | |
| #endif | |
| for (int tid = 0; tid < nthreads; tid++) { | |
| for (int tid = 0; tid < nthreads; tid++) |
With the for loop, the parallel region isn't even required anymore.
There was a problem hiding this comment.
Yes, I was thinking the same. Not sure it matters...
|
I think it's a false positive. In the past on our own machines, we had to compile gcc from source with certain flags to make thread sanitizer work for OpenMP. For the WarpX CI, maybe similar things need to be done for clang. Not sure if this old stackoverflow post is still relevant. https://stackoverflow.com/questions/33004809/can-i-use-thread-sanitizer-for-openmp-programs For this particular case, we could simply not using openmp. However, this is probably the first OMP parallel region in the code. I believe the later ones would fail too even if we remove OMP here. |
|
Agreed, I would not change it and rather make sure an OpenMP-enabled sanitizer is used. |
Summary
Either making the shared variables constant (not sensible in the function signature) or using
firstprivateshould silence the warnings we see.Additional background
Fix #4279
cc @EZoni
Checklist
The proposed changes: