Skip to content

Commit 57a90ed

Browse files
authored
[OpenMP][GPU][FIX] Enable generic barriers in single threaded contexts (#140786)
The generic GPU barrier implementation checked if it was the main thread in generic mode to identify single threaded regions. This doesn't work since inside of a non-active (=sequential) parallel, that thread becomes the main thread of a team, and is not the main thread in generic mode. At least that is the implementation of the APIs today. To identify single threaded regions we now check the team size explicitly. This exposed three other issues; one is, for now, expected and not a bug, the second one is a bug and has a FIXME in the single_threaded_for_barrier_hang_1.c file, and the final one is also benign as described in the end. The non-bug issue comes up if we ever initialize a thread state. Afterwards we will never run any region in parallel. This is a little conservative, but I guess thread states are really bad for performance anyway. The bug comes up if we optimize single_threaded_for_barrier_hang_1 and execute it in Generic-SPMD mode. For some reason we loose all the updates to b. This looks very much like a compiler bug, but could also be another logic issue in the runtime. Needs to be investigated. Issue number 3 comes up if we have nested parallels inside of a target region. The clang SPMD-check logic gets confused, determines SPMD (which is fine) but picks an unreasonable thread count. This is all benign, I think, just weird: ``` #pragma omp target teams #pragma omp parallel num_threads(64) #pragma omp parallel num_threads(10) {} ``` Was launched with 10 threads, not 64.
1 parent 0dfdf7e commit 57a90ed

File tree

3 files changed

+49
-3
lines changed

3 files changed

+49
-3
lines changed

offload/DeviceRTL/src/Synchronization.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -303,12 +303,14 @@ int32_t __kmpc_cancel_barrier(IdentTy *Loc, int32_t TId) {
303303
}
304304

305305
void __kmpc_barrier(IdentTy *Loc, int32_t TId) {
306-
if (mapping::isMainThreadInGenericMode())
307-
return __kmpc_flush(Loc);
308-
309306
if (mapping::isSPMDMode())
310307
return __kmpc_barrier_simple_spmd(Loc, TId);
311308

309+
// Generic parallel regions are run with multiple of the warp size or single
310+
// threaded, in the latter case we need to stop here.
311+
if (omp_get_num_threads() == 1)
312+
return __kmpc_flush(Loc);
313+
312314
impl::namedBarrier();
313315
}
314316

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: %libomptarget-compile-run-and-check-generic
2+
// RUN: %libomptarget-compileopt-run-and-check-generic
3+
4+
#include <omp.h>
5+
#include <stdio.h>
6+
7+
int main() {
8+
int b = 0;
9+
10+
#pragma omp target map(tofrom : b)
11+
for (int i = 1; i <= 10; ++i) {
12+
#pragma omp parallel num_threads(10) reduction(+ : b)
13+
#pragma omp for
14+
for (int k = 0; k < 10; ++k)
15+
++b;
16+
}
17+
18+
// CHECK: b: 100
19+
printf("b: %i\n", b);
20+
return 0;
21+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// RUN: %libomptarget-compile-run-and-check-generic
2+
// FIXME: This fails with optimization enabled and prints b: 0
3+
// FIXME: RUN: %libomptarget-compileopt-run-and-check-generic
4+
5+
#include <omp.h>
6+
#include <stdio.h>
7+
8+
int main() {
9+
int b = 0;
10+
11+
#pragma omp target map(tofrom : b) thread_limit(256)
12+
for (int i = 1; i <= 1; ++i) {
13+
#pragma omp parallel num_threads(64) reduction(+ : b)
14+
#pragma omp parallel num_threads(10) reduction(+ : b)
15+
#pragma omp for
16+
for (int k = 0; k < 10; ++k)
17+
++b;
18+
}
19+
20+
// CHECK: b: 640
21+
printf("b: %i\n", b);
22+
return 0;
23+
}

0 commit comments

Comments
 (0)