-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
[clang][OpenMP] Implement isOpenMPCapturingDirective
#97090
Conversation
What is considered "executable" in clang differs slightly from the OpenMP's "executable" category. In addition to the executable category, subsidiary directives, and OMPD_error are considered executable. Implement a function that performs that check.
Check if the given directive can capture variables, and thus needs a captured statement. Simplify some code using this function.
@llvm/pr-subscribers-clang Author: Krzysztof Parzyszek (kparzysz) ChangesCheck if the given directive can capture variables, and thus needs a captured statement. Simplify some code using this function. Full diff: https://github.com/llvm/llvm-project/pull/97090.diff 3 Files Affected:
diff --git a/clang/include/clang/Basic/OpenMPKinds.h b/clang/include/clang/Basic/OpenMPKinds.h
index 6d9d6ebc58e2c..3f21766f392cf 100644
--- a/clang/include/clang/Basic/OpenMPKinds.h
+++ b/clang/include/clang/Basic/OpenMPKinds.h
@@ -376,6 +376,12 @@ bool checkFailClauseParameter(OpenMPClauseKind FailClauseParameter);
/// \return true - if the above condition is met for this directive
/// otherwise - false.
bool isOpenMPExecutableDirective(OpenMPDirectiveKind DKind);
+
+/// Checks if the specified directive need to capture variables.
+/// \param DKind Specified directive.
+/// \return true - if the above condition is met for this directive
+/// otherwise - false.
+bool isOpenMPCapturingDirective(OpenMPDirectiveKind DKind);
}
#endif
diff --git a/clang/lib/Basic/OpenMPKinds.cpp b/clang/lib/Basic/OpenMPKinds.cpp
index 7c8990880fae3..30c34c207ae23 100644
--- a/clang/lib/Basic/OpenMPKinds.cpp
+++ b/clang/lib/Basic/OpenMPKinds.cpp
@@ -709,10 +709,44 @@ bool clang::isOpenMPExecutableDirective(OpenMPDirectiveKind DKind) {
return Cat == Category::Executable || Cat == Category::Subsidiary;
}
+bool clang::isOpenMPCapturingDirective(OpenMPDirectiveKind DKind) {
+ if (isOpenMPExecutableDirective(DKind)) {
+ switch (DKind) {
+ case OMPD_atomic:
+ case OMPD_barrier:
+ case OMPD_cancel:
+ case OMPD_cancellation_point:
+ case OMPD_critical:
+ case OMPD_depobj:
+ case OMPD_error:
+ case OMPD_flush:
+ case OMPD_masked:
+ case OMPD_master:
+ case OMPD_section:
+ case OMPD_taskwait:
+ case OMPD_taskyield:
+ return false;
+ default:
+ return !isOpenMPLoopTransformationDirective(DKind);
+ }
+ }
+ // Non-executable directives.
+ switch (DKind) {
+ case OMPD_metadirective:
+ case OMPD_nothing:
+ return true;
+ default:
+ break;
+ }
+ return false;
+}
+
void clang::getOpenMPCaptureRegions(
SmallVectorImpl<OpenMPDirectiveKind> &CaptureRegions,
OpenMPDirectiveKind DKind) {
assert(unsigned(DKind) < llvm::omp::Directive_enumSize);
+ assert(isOpenMPCapturingDirective(DKind));
+
switch (DKind) {
case OMPD_metadirective:
CaptureRegions.push_back(OMPD_metadirective);
@@ -799,48 +833,18 @@ void clang::getOpenMPCaptureRegions(
case OMPD_for:
case OMPD_for_simd:
case OMPD_sections:
- case OMPD_section:
case OMPD_single:
- case OMPD_master:
- case OMPD_critical:
case OMPD_taskgroup:
case OMPD_distribute:
case OMPD_ordered:
- case OMPD_atomic:
case OMPD_target_data:
case OMPD_distribute_simd:
case OMPD_scope:
case OMPD_dispatch:
CaptureRegions.push_back(OMPD_unknown);
break;
- case OMPD_tile:
- case OMPD_unroll:
- // loop transformations do not introduce captures.
- break;
- case OMPD_threadprivate:
- case OMPD_allocate:
- case OMPD_taskyield:
- case OMPD_barrier:
- case OMPD_error:
- case OMPD_taskwait:
- case OMPD_cancellation_point:
- case OMPD_cancel:
- case OMPD_flush:
- case OMPD_depobj:
- case OMPD_scan:
- case OMPD_declare_reduction:
- case OMPD_declare_mapper:
- case OMPD_declare_simd:
- case OMPD_declare_target:
- case OMPD_end_declare_target:
- case OMPD_requires:
- case OMPD_declare_variant:
- case OMPD_begin_declare_variant:
- case OMPD_end_declare_variant:
- llvm_unreachable("OpenMP Directive is not allowed");
- case OMPD_unknown:
default:
- llvm_unreachable("Unknown OpenMP directive");
+ llvm_unreachable("Unhandled OpenMP directive");
}
}
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index b17c7e2be968e..a741339a7d669 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -4862,11 +4862,7 @@ StmtResult SemaOpenMP::ActOnOpenMPRegionEnd(StmtResult S,
ArrayRef<OMPClause *> Clauses) {
handleDeclareVariantConstructTrait(DSAStack, DSAStack->getCurrentDirective(),
/* ScopeEntry */ false);
- if (DSAStack->getCurrentDirective() == OMPD_atomic ||
- DSAStack->getCurrentDirective() == OMPD_critical ||
- DSAStack->getCurrentDirective() == OMPD_section ||
- DSAStack->getCurrentDirective() == OMPD_master ||
- DSAStack->getCurrentDirective() == OMPD_masked)
+ if (!isOpenMPCapturingDirective(DSAStack->getCurrentDirective()))
return S;
bool ErrorFound = false;
@@ -4909,10 +4905,6 @@ StmtResult SemaOpenMP::ActOnOpenMPRegionEnd(StmtResult S,
}
}
DSAStack->setForceVarCapturing(/*V=*/false);
- } else if (isOpenMPLoopTransformationDirective(
- DSAStack->getCurrentDirective())) {
- assert(CaptureRegions.empty() &&
- "No captured regions in loop transformation directives.");
} else if (CaptureRegions.size() > 1 ||
CaptureRegions.back() != OMPD_unknown) {
if (auto *C = OMPClauseWithPreInit::get(Clause))
@@ -6400,9 +6392,7 @@ StmtResult SemaOpenMP::ActOnOpenMPExecutableDirective(
ClausesWithImplicit.append(Clauses.begin(), Clauses.end());
}
if (AStmt && !SemaRef.CurContext->isDependentContext() &&
- Kind != OMPD_atomic && Kind != OMPD_critical && Kind != OMPD_section &&
- Kind != OMPD_master && Kind != OMPD_masked &&
- !isOpenMPLoopTransformationDirective(Kind)) {
+ isOpenMPCapturingDirective(Kind)) {
assert(isa<CapturedStmt>(AStmt) && "Captured statement expected");
// Check default data sharing attributes for referenced variables.
|
clang/lib/Basic/OpenMPKinds.cpp
Outdated
void clang::getOpenMPCaptureRegions( | ||
SmallVectorImpl<OpenMPDirectiveKind> &CaptureRegions, | ||
OpenMPDirectiveKind DKind) { | ||
assert(unsigned(DKind) < llvm::omp::Directive_enumSize); | ||
assert(isOpenMPCapturingDirective(DKind)); |
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.
Add assertion message
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.
Done.
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.
LG
Check if the given directive can capture variables, and thus needs a captured statement. Simplify some code using this function.
Check if the given directive can capture variables, and thus needs a captured statement. Simplify some code using this function.
Check if the given directive can capture variables, and thus needs a captured statement.
Simplify some code using this function.