[go: nahoru, domu]

Skip to content
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

Merged
merged 7 commits into from
Jul 1, 2024

Conversation

kparzysz
Copy link
Contributor

Check if the given directive can capture variables, and thus needs a captured statement.

Simplify some code using this function.

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.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang labels Jun 28, 2024
@llvmbot
Copy link
Collaborator
llvmbot commented Jun 28, 2024

@llvm/pr-subscribers-clang

Author: Krzysztof Parzyszek (kparzysz)

Changes

Check 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:

  • (modified) clang/include/clang/Basic/OpenMPKinds.h (+6)
  • (modified) clang/lib/Basic/OpenMPKinds.cpp (+35-31)
  • (modified) clang/lib/Sema/SemaOpenMP.cpp (+2-12)
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.

void clang::getOpenMPCaptureRegions(
SmallVectorImpl<OpenMPDirectiveKind> &CaptureRegions,
OpenMPDirectiveKind DKind) {
assert(unsigned(DKind) < llvm::omp::Directive_enumSize);
assert(isOpenMPCapturingDirective(DKind));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add assertion message

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Base automatically changed from users/kparzysz/spr/c01-is-exec to main June 29, 2024 14:37
Copy link
Member
@alexey-bataev alexey-bataev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

@kparzysz kparzysz merged commit deda50f into main Jul 1, 2024
7 checks passed
@kparzysz kparzysz deleted the users/kparzysz/spr/c02-is-capturing branch July 1, 2024 20:46
kirillpyasecky pushed a commit to kirillpyasecky/llvm-project that referenced this pull request Jul 3, 2024
Check if the given directive can capture variables, and thus needs a
captured statement.

Simplify some code using this function.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
Check if the given directive can capture variables, and thus needs a
captured statement.

Simplify some code using this function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants