llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.62k stars 11.83k forks source link

combined target teams implements shared clause as firstprivate #39600

Open jdenny-ornl opened 5 years ago

jdenny-ornl commented 5 years ago
Bugzilla Link 40253
Version unspecified
OS Linux
CC @alexey-bataev,@hfinkel

Extended Description

Clang implements shared(n) on omp target teams as firstprivate instead of shared. For example, n is not shared in the following example:

$ cat test.c
#include <omp.h>
#include <stdio.h>
int main() {
  int n = 0;
  #pragma omp target teams shared(n) num_teams(2)
  #pragma omp parallel num_threads(1)
  {
    #pragma omp atomic update
    ++n;
    printf("n=%d, team=%d\n", n, omp_get_team_num());
  }
  return 0;
}

$ clang -fopenmp test.c && ./a.out
n=1, team=0
n=1, team=1

However, if I split the target teams directive into two directives, I see results indicating n is shared:

$ cat test.c
#include <omp.h>
#include <stdio.h>
int main() {
  int n = 0;
  #pragma omp target 
  #pragma omp teams shared(n) num_teams(2)
  #pragma omp parallel num_threads(1)
  {
    #pragma omp atomic update
    ++n;
    printf("n=%d, team=%d\n", n, omp_get_team_num());
  }
  return 0;
}

$ clang -fopenmp test.c && ./a.out
n=1, team=0
n=2, team=1

The LLVM IR shows that n is passed to the teams by value in the first case and by pointer in the second.

This bugzilla was suggested at:

https://reviews.llvm.org/D56113#1345047

alexey-bataev commented 2 years ago

mentioned in issue llvm/llvm-bugzilla-archive#43175

alexey-bataev commented 4 years ago

The reproducer I reported here still reproduces the bug for me at fbaf835c5c51.

Oops, marked the wrong bug as a duplicate, thanks for reopening.

jdenny-ornl commented 4 years ago

The reproducer I reported here still reproduces the bug for me at fbaf835c5c51.

alexey-bataev commented 4 years ago

It was already fixed.

This bug has been marked as a duplicate of bug llvm/llvm-bugzilla-archive#43175