Skip to content

[clang][OpenMP] Implement isOpenMPExecutableDirective #97089

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

Merged
merged 3 commits into from
Jun 29, 2024

Conversation

kparzysz
Copy link
Contributor

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.

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.
@kparzysz kparzysz requested a review from alexey-bataev June 28, 2024 17:58
@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
Member

llvmbot commented Jun 28, 2024

@llvm/pr-subscribers-clang

Author: Krzysztof Parzyszek (kparzysz)

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/97089.diff

4 Files Affected:

  • (modified) clang/include/clang/Basic/OpenMPKinds.h (+8)
  • (modified) clang/lib/Basic/OpenMPKinds.cpp (+7)
  • (modified) clang/lib/Parse/ParseOpenMP.cpp (+2-4)
  • (modified) clang/lib/Sema/SemaOpenMP.cpp (+2)
diff --git a/clang/include/clang/Basic/OpenMPKinds.h b/clang/include/clang/Basic/OpenMPKinds.h
index d127498774c7f..6d9d6ebc58e2c 100644
--- a/clang/include/clang/Basic/OpenMPKinds.h
+++ b/clang/include/clang/Basic/OpenMPKinds.h
@@ -368,6 +368,14 @@ bool needsTaskBasedThreadLimit(OpenMPDirectiveKind DKind);
 /// is restricted only to memory order clauses of "OMPC_acquire",
 /// "OMPC_relaxed" and "OMPC_seq_cst".
 bool checkFailClauseParameter(OpenMPClauseKind FailClauseParameter);
+
+/// Checks if the specified directive is considered as "executable". This
+/// combines the OpenMP categories of "executable" and "subsidiary", plus
+/// any other directives that are should be treated as executable.
+/// \param DKind Specified directive.
+/// \return true - if the above condition is met for this directive
+/// otherwise - false.
+bool isOpenMPExecutableDirective(OpenMPDirectiveKind DKind);
 }
 
 #endif
diff --git a/clang/lib/Basic/OpenMPKinds.cpp b/clang/lib/Basic/OpenMPKinds.cpp
index b3e9affbb3e58..7c8990880fae3 100644
--- a/clang/lib/Basic/OpenMPKinds.cpp
+++ b/clang/lib/Basic/OpenMPKinds.cpp
@@ -702,6 +702,13 @@ bool clang::needsTaskBasedThreadLimit(OpenMPDirectiveKind DKind) {
          DKind == OMPD_target_parallel_loop;
 }
 
+bool clang::isOpenMPExecutableDirective(OpenMPDirectiveKind DKind) {
+  if (DKind == OMPD_error)
+    return true;
+  Category Cat = getDirectiveCategory(DKind);
+  return Cat == Category::Executable || Cat == Category::Subsidiary;
+}
+
 void clang::getOpenMPCaptureRegions(
     SmallVectorImpl<OpenMPDirectiveKind> &CaptureRegions,
     OpenMPDirectiveKind DKind) {
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 00f9ebb65d876..326cd22ff9005 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2397,10 +2397,8 @@ Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
 StmtResult Parser::ParseOpenMPExecutableDirective(
     ParsedStmtContext StmtCtx, OpenMPDirectiveKind DKind, SourceLocation Loc,
     bool ReadDirectiveWithinMetadirective) {
-  assert((DKind == OMPD_error ||
-          getDirectiveCategory(DKind) == Category::Executable ||
-          getDirectiveCategory(DKind) == Category::Subsidiary) &&
-         "Directive with an unexpected category");
+  assert(isOpenMPExecutableDirective(DKind) && "Unexpected directive category");
+
   bool HasAssociatedStatement = true;
   Association Assoc = getDirectiveAssociation(DKind);
 
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 29104b2c0de94..b17c7e2be968e 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -6357,6 +6357,8 @@ StmtResult SemaOpenMP::ActOnOpenMPExecutableDirective(
     OpenMPDirectiveKind CancelRegion, ArrayRef<OMPClause *> Clauses,
     Stmt *AStmt, SourceLocation StartLoc, SourceLocation EndLoc,
     OpenMPDirectiveKind PrevMappedDirective) {
+  assert(isOpenMPExecutableDirective(Kind) && "Unexpected directive category");
+
   StmtResult Res = StmtError();
   OpenMPBindClauseKind BindKind = OMPC_BIND_unknown;
   llvm::SmallVector<OMPClause *> ClausesWithoutBind;

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 with nits


/// Checks if the specified directive is considered as "executable". This
/// combines the OpenMP categories of "executable" and "subsidiary", plus
/// any other directives that are should be treated as executable.
Copy link
Member

Choose a reason for hiding this comment

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

that are should

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

/// combines the OpenMP categories of "executable" and "subsidiary", plus
/// any other directives that are should be treated as executable.
/// \param DKind Specified directive.
/// \return true - if the above condition is met for this directive
Copy link
Member

Choose a reason for hiding this comment

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

condition is met

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure? "...condition is met for this directive..." sounds correct to me.

Copy link
Member

Choose a reason for hiding this comment

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

Up to you

@kparzysz kparzysz merged commit 0ce801f into main Jun 29, 2024
7 checks passed
@kparzysz kparzysz deleted the users/kparzysz/spr/c01-is-exec branch June 29, 2024 14:37
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
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.
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.

3 participants