-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[Clang][OpenMP] Bug fix Default clause variable category #165276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[Clang][OpenMP] Bug fix Default clause variable category #165276
Conversation
variable category.
2) Adding a new test case
clang/test/OpenMP/parallel_default_variableCategory_codegen.cpp
3) Taking care of new comments in
llvm#157063
|
@llvm/pr-subscribers-clang Author: None (SunilKuravinakop) ChangesIn the default clause, check for allocatable type in the variable category has been made more robust. Taking care of new comments in the previous "Support for Default clause variable category" 157063. Full diff: https://github.com/llvm/llvm-project/pull/165276.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 5b5b1b685e153..f677be7f02583 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -1314,6 +1314,22 @@ static std::string getOpenMPClauseNameForDiag(OpenMPClauseKind C) {
return getOpenMPClauseName(C).str();
}
+bool isAllocatableType(QualType QT) {
+ if (QT->isPointerType())
+ return true;
+ QT = QT.getCanonicalType().getUnqualifiedType();
+ if (const CXXRecordDecl *RD = QT->getAsCXXRecordDecl()) {
+ if (isa<ClassTemplateSpecializationDecl>(RD)) {
+ std::string QName = RD->getQualifiedNameAsString();
+ return (QName == "std::vector" || QName == "vector" ||
+ QName == "std::unique_ptr" || QName == "unique_ptr" ||
+ QName == "std::shared_ptr" || QName == "shared_ptr" ||
+ QName == "llvm::SmallVector" || QName == "SmallVector");
+ }
+ }
+ return false;
+}
+
DSAStackTy::DSAVarData DSAStackTy::getDSA(const_iterator &Iter,
ValueDecl *D) const {
D = getCanonicalDecl(D);
@@ -1370,20 +1386,19 @@ DSAStackTy::DSAVarData DSAStackTy::getDSA(const_iterator &Iter,
DefaultDataSharingAttributes IterDA = Iter->DefaultAttr;
switch (Iter->DefaultVCAttr) {
case DSA_VC_aggregate:
- if (!VD->getType()->isAggregateType())
+ if (!D->getType()->isAggregateType())
IterDA = DSA_none;
break;
case DSA_VC_allocatable:
- if (!(VD->getType()->isPointerType() ||
- VD->getType()->isVariableArrayType()))
+ if (!isAllocatableType(D->getType()))
IterDA = DSA_none;
break;
case DSA_VC_pointer:
- if (!VD->getType()->isPointerType())
+ if (!D->getType()->isPointerType())
IterDA = DSA_none;
break;
case DSA_VC_scalar:
- if (!VD->getType()->isScalarType())
+ if (!D->getType()->isScalarType())
IterDA = DSA_none;
break;
case DSA_VC_all:
diff --git a/clang/test/OpenMP/parallel_default_variableCategory_codegen.cpp b/clang/test/OpenMP/parallel_default_variableCategory_codegen.cpp
new file mode 100644
index 0000000000000..b0674158f57e5
--- /dev/null
+++ b/clang/test/OpenMP/parallel_default_variableCategory_codegen.cpp
@@ -0,0 +1,117 @@
+// RUN: %clangxx -DOMP60 -Xclang -verify -Wno-vla -fopenmp -fopenmp-version=60 -x c++ -S -emit-llvm %s -o - | FileCheck --check-prefixes=OMP60 %s
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+#include <vector>
+
+int global;
+#define VECTOR_SIZE 4
+int main (int argc, char **argv) {
+ int i,n;
+ int x;
+
+ n = VECTOR_SIZE;
+
+ #pragma omp parallel masked firstprivate(x) num_threads(2)
+ {
+ int *xPtr = nullptr;
+ // scalar
+ #pragma omp task default(shared:scalar)
+ {
+ xPtr = &x;
+ }
+ #pragma omp taskwait
+
+ // pointer
+ #pragma omp task default(shared:pointer) shared(x)
+ {
+ xPtr = &x;
+ }
+ #pragma omp taskwait
+ }
+
+ int *aggregate[VECTOR_SIZE] = {0,0,0,0};
+ std::vector<int *> arr(VECTOR_SIZE,0);
+
+ #pragma omp parallel masked num_threads(2)
+ {
+ // aggregate
+ #pragma omp task default(shared:aggregate)
+ for(i=0;i<n;i++) {
+ aggregate[i] = &x;
+ }
+ #pragma omp taskwait
+
+ #pragma omp task default(shared:aggregate) shared(x)
+ for(i=0;i<n;i++) {
+ aggregate[i] = &x;
+ }
+ #pragma omp taskwait
+
+ // allocatable
+ #pragma omp task default(shared:allocatable)
+ for(i=0;i<n;i++) {
+ arr[i] = &x;
+ }
+ #pragma omp taskwait
+
+ #pragma omp task default(shared:allocatable) shared(x)
+ for(i=0;i<n;i++) {
+ arr[i] = &x;
+ }
+ #pragma omp taskwait
+
+ // all
+ #pragma omp task default(shared:all)
+ for(i=0;i<n;i++) {
+ aggregate[i] = &x;
+ }
+ #pragma omp taskwait
+ }
+}
+
+#endif
+
+// OMP60-LABEL: define {{.*}}main.omp_outlined{{.*}}
+// OMP60-NEXT: entry:
+// OMP60-NEXT: [[DOTGLOBAL_TID__ADDR:%.*]] = alloca ptr, align 8
+// OMP60-NEXT: [[DOTBOUND_TID__ADDR:%.*]] = alloca ptr, align 8
+// OMP60: [[X_ADDR:%.*]] = alloca{{.*}}
+// OMP60: [[xPTR:%.*]] = alloca{{.*}}
+// OMP60: store ptr null, ptr [[xPTR]]{{.*}}
+// OMP60: store ptr [[xPTR]]{{.*}}
+// OMP60: store ptr [[X_ADDR]]{{.*}}
+// OMP60-NEXT: {{.*}}call{{.*}}__kmpc_omp_task_alloc{{.*}}
+// OMP60: ret void
+//
+// OMP60: define {{.*}}main.omp_outlined{{.*}}
+// OMP60-NEXT: entry:
+// OMP60-NEXT: [[DOTGLOBAL_TID__ADDR:%.*]] = alloca ptr, align 8
+// OMP60-NEXT: [[DOTBOUND_TID__ADDR:%.*]] = alloca ptr, align 8
+// OMP60: [[I_ADDR:%.*]] = alloca{{.*}}
+// OMP60-NEXT: [[N_ADDR:%.*]] = alloca{{.*}}
+// OMP60-NEXT: [[AGGREGATE_ADDR:%.*]] = alloca{{.*}}
+// OMP60-NEXT: [[X_ADDR:%.*]] = alloca{{.*}}
+// OMP60-NEXT: [[ARR_ADDR:%.*]] = alloca{{.*}}
+// OMP60: [[TMP0:%.*]] = load{{.*}}[[I_ADDR]]
+// OMP60-NEXT: [[TMP1:%.*]] = load{{.*}}[[N_ADDR]]
+// OMP60-NEXT: [[TMP2:%.*]] = load{{.*}}[[AGGREGATE_ADDR]]
+// OMP60-NEXT: [[TMP3:%.*]] = load{{.*}}[[X_ADDR]]
+// OMP60-NEXT: [[TMP4:%.*]] = load{{.*}}[[ARR_ADDR]]
+// OMP60: store ptr [[TMP2]]{{.*}}
+// OMP60-NEXT: {{.*}}call{{.*}}__kmpc_omp_task_alloc{{.*}}
+// OMP60: store ptr [[TMP2]]{{.*}}
+// OMP60: store ptr [[TMP3]]{{.*}}
+// OMP60-NEXT: {{.*}}call{{.*}}__kmpc_omp_task_alloc{{.*}}
+// OMP60: store ptr [[TMP4]]{{.*}}
+// OMP60-NEXT: {{.*}}call{{.*}}__kmpc_omp_task_alloc{{.*}}
+// OMP60: store ptr [[TMP4]]{{.*}}
+// OMP60: store ptr [[TMP3]]{{.*}}
+// OMP60-NEXT: {{.*}}call{{.*}}__kmpc_omp_task_alloc{{.*}}
+// OMP60: store ptr [[TMP0]]{{.*}}
+// OMP60: store ptr [[TMP1]]{{.*}}
+// OMP60: store ptr [[TMP2]]{{.*}}
+// OMP60: store ptr [[TMP3]]{{.*}}
+// OMP60-NEXT: {{.*}}call{{.*}}__kmpc_omp_task_alloc{{.*}}
+// OMP60: ret void
|
| bool isAllocatableType(QualType QT) { | ||
| if (QT->isPointerType()) | ||
| return true; | ||
| QT = QT.getCanonicalType().getUnqualifiedType(); | ||
| if (const CXXRecordDecl *RD = QT->getAsCXXRecordDecl()) { | ||
| if (isa<ClassTemplateSpecializationDecl>(RD)) { | ||
| std::string QName = RD->getQualifiedNameAsString(); | ||
| return (QName == "std::vector" || QName == "vector" || | ||
| QName == "std::unique_ptr" || QName == "unique_ptr" || | ||
| QName == "std::shared_ptr" || QName == "shared_ptr" || | ||
| QName == "llvm::SmallVector" || QName == "SmallVector"); | ||
| } | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poor check, it should not check the names, should check the members
In the default clause, check for allocatable type in the variable category has been made more robust. Taking care of new comments in the previous "Support for Default clause variable category" 157063.