[llvm-branch-commits] [clang] [clang][OpenMP] Shorten directive classification in ParseOpenMP (PR #94691)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Jun 6 15:09:46 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Krzysztof Parzyszek (kparzysz)
<details>
<summary>Changes</summary>
Use directive categories to simplify long lists of `case` statements in the OpenMP parser. This is a step towards avoiding dependence on explicitly specified sets of directives that can be expressed more generically.
The upcoming OpenMP 6.0 will introduce many new combined directives, and the more generically we handle directives, the easier the introduction of the new standard will be.
---
Patch is 21.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/94691.diff
2 Files Affected:
- (modified) clang/include/clang/Parse/Parser.h (+13)
- (modified) clang/lib/Parse/ParseOpenMP.cpp (+217-309)
``````````diff
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index d054b8cf0d240..88571a4a46f2c 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3509,6 +3509,19 @@ class Parser : public CodeCompletionHandler {
/// metadirective and therefore ends on the closing paren.
StmtResult ParseOpenMPDeclarativeOrExecutableDirective(
ParsedStmtContext StmtCtx, bool ReadDirectiveWithinMetadirective = false);
+
+ /// Parses executable directive.
+ ///
+ /// \param StmtCtx The context in which we're parsing the directive.
+ /// \param DKind The kind of the executable directive.
+ /// \param Loc Source location of the beginning of the directive.
+ /// \param ReadDirectiveWithinMetadirective true if directive is within a
+ /// metadirective and therefore ends on the closing paren.
+ StmtResult
+ ParseOpenMPExecutableDirective(ParsedStmtContext StmtCtx,
+ OpenMPDirectiveKind DKind, SourceLocation Loc,
+ bool ReadDirectiveWithinMetadirective);
+
/// Parses clause of kind \a CKind for directive of a kind \a Kind.
///
/// \param DKind Kind of current directive.
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 50a872fedebf7..a10bb009ec835 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2374,86 +2374,209 @@ Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
case OMPD_unknown:
Diag(Tok, diag::err_omp_unknown_directive);
break;
- case OMPD_parallel:
- case OMPD_simd:
- case OMPD_tile:
- case OMPD_unroll:
- case OMPD_task:
- case OMPD_taskyield:
+ default:
+ switch (getDirectiveCategory(DKind)) {
+ case Category::Executable:
+ case Category::Meta:
+ case Category::Subsidiary:
+ case Category::Utility:
+ Diag(Tok, diag::err_omp_unexpected_directive)
+ << 1 << getOpenMPDirectiveName(DKind);
+ break;
+ default:
+ break;
+ }
+ }
+ while (Tok.isNot(tok::annot_pragma_openmp_end))
+ ConsumeAnyToken();
+ ConsumeAnyToken();
+ return nullptr;
+}
+
+StmtResult
+Parser::ParseOpenMPExecutableDirective(ParsedStmtContext StmtCtx,
+ OpenMPDirectiveKind DKind,
+ SourceLocation Loc,
+ bool ReadDirectiveWithinMetadirective) {
+ bool HasAssociatedStatement = true;
+
+ switch (DKind) {
case OMPD_barrier:
- case OMPD_taskwait:
- case OMPD_taskgroup:
- case OMPD_flush:
+ case OMPD_cancel:
+ case OMPD_cancellation_point:
case OMPD_depobj:
+ case OMPD_error:
+ case OMPD_flush:
+ case OMPD_interop:
case OMPD_scan:
- case OMPD_for:
- case OMPD_for_simd:
- case OMPD_sections:
- case OMPD_section:
- case OMPD_single:
- case OMPD_master:
- case OMPD_ordered:
- case OMPD_critical:
- case OMPD_parallel_for:
- case OMPD_parallel_for_simd:
- case OMPD_parallel_sections:
- case OMPD_parallel_master:
- case OMPD_parallel_masked:
- case OMPD_atomic:
- case OMPD_target:
- case OMPD_teams:
- case OMPD_cancellation_point:
- case OMPD_cancel:
- case OMPD_target_data:
case OMPD_target_enter_data:
case OMPD_target_exit_data:
- case OMPD_target_parallel:
- case OMPD_target_parallel_for:
- case OMPD_taskloop:
- case OMPD_taskloop_simd:
- case OMPD_master_taskloop:
- case OMPD_master_taskloop_simd:
- case OMPD_parallel_master_taskloop:
- case OMPD_parallel_master_taskloop_simd:
- case OMPD_masked_taskloop:
- case OMPD_masked_taskloop_simd:
- case OMPD_parallel_masked_taskloop:
- case OMPD_parallel_masked_taskloop_simd:
- case OMPD_distribute:
case OMPD_target_update:
- case OMPD_distribute_parallel_for:
- case OMPD_distribute_parallel_for_simd:
- case OMPD_distribute_simd:
- case OMPD_target_parallel_for_simd:
- case OMPD_target_simd:
- case OMPD_scope:
- case OMPD_teams_distribute:
- case OMPD_teams_distribute_simd:
- case OMPD_teams_distribute_parallel_for_simd:
- case OMPD_teams_distribute_parallel_for:
- case OMPD_target_teams:
- case OMPD_target_teams_distribute:
- case OMPD_target_teams_distribute_parallel_for:
- case OMPD_target_teams_distribute_parallel_for_simd:
- case OMPD_target_teams_distribute_simd:
- case OMPD_dispatch:
- case OMPD_masked:
- case OMPD_metadirective:
- case OMPD_loop:
- case OMPD_teams_loop:
- case OMPD_target_teams_loop:
- case OMPD_parallel_loop:
- case OMPD_target_parallel_loop:
- Diag(Tok, diag::err_omp_unexpected_directive)
- << 1 << getOpenMPDirectiveName(DKind);
- break;
- default:
+ case OMPD_taskwait:
+ case OMPD_taskyield:
+ if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) ==
+ ParsedStmtContext()) {
+ Diag(Tok, diag::err_omp_immediate_directive)
+ << getOpenMPDirectiveName(DKind) << 0;
+ if (DKind == OMPD_error) {
+ SkipUntil(tok::annot_pragma_openmp_end);
+ return StmtError();
+ }
+ }
+ HasAssociatedStatement = false;
break;
}
- while (Tok.isNot(tok::annot_pragma_openmp_end))
- ConsumeAnyToken();
- ConsumeAnyToken();
- return nullptr;
+
+ SourceLocation EndLoc;
+ SmallVector<OMPClause *, 5> Clauses;
+ std::bitset<llvm::omp::Clause_enumSize + 1> SeenClauses;
+ DeclarationNameInfo DirName;
+ OpenMPDirectiveKind CancelRegion = OMPD_unknown;
+ unsigned ScopeFlags = Scope::FnScope | Scope::DeclScope |
+ Scope::CompoundStmtScope | Scope::OpenMPDirectiveScope;
+
+ // Special processing for flush and depobj clauses.
+ Token ImplicitTok;
+ bool ImplicitClauseAllowed = false;
+ if (DKind == OMPD_flush || DKind == OMPD_depobj) {
+ ImplicitTok = Tok;
+ ImplicitClauseAllowed = true;
+ }
+ ConsumeToken();
+ // Parse directive name of the 'critical' directive if any.
+ if (DKind == OMPD_critical) {
+ BalancedDelimiterTracker T(*this, tok::l_paren,
+ tok::annot_pragma_openmp_end);
+ if (!T.consumeOpen()) {
+ if (Tok.isAnyIdentifier()) {
+ DirName =
+ DeclarationNameInfo(Tok.getIdentifierInfo(), Tok.getLocation());
+ ConsumeAnyToken();
+ } else {
+ Diag(Tok, diag::err_omp_expected_identifier_for_critical);
+ }
+ T.consumeClose();
+ }
+ } else if (DKind == OMPD_cancellation_point || DKind == OMPD_cancel) {
+ CancelRegion = parseOpenMPDirectiveKind(*this);
+ if (Tok.isNot(tok::annot_pragma_openmp_end))
+ ConsumeToken();
+ }
+
+ if (isOpenMPLoopDirective(DKind))
+ ScopeFlags |= Scope::OpenMPLoopDirectiveScope;
+ if (isOpenMPSimdDirective(DKind))
+ ScopeFlags |= Scope::OpenMPSimdDirectiveScope;
+ ParseScope OMPDirectiveScope(this, ScopeFlags);
+ Actions.OpenMP().StartOpenMPDSABlock(DKind, DirName, Actions.getCurScope(),
+ Loc);
+
+ while (Tok.isNot(tok::annot_pragma_openmp_end)) {
+ // If we are parsing for a directive within a metadirective, the directive
+ // ends with a ')'.
+ if (ReadDirectiveWithinMetadirective && Tok.is(tok::r_paren)) {
+ while (Tok.isNot(tok::annot_pragma_openmp_end))
+ ConsumeAnyToken();
+ break;
+ }
+ bool HasImplicitClause = false;
+ if (ImplicitClauseAllowed && Tok.is(tok::l_paren)) {
+ HasImplicitClause = true;
+ // Push copy of the current token back to stream to properly parse
+ // pseudo-clause OMPFlushClause or OMPDepobjClause.
+ PP.EnterToken(Tok, /*IsReinject*/ true);
+ PP.EnterToken(ImplicitTok, /*IsReinject*/ true);
+ ConsumeAnyToken();
+ }
+ OpenMPClauseKind CKind = Tok.isAnnotation()
+ ? OMPC_unknown
+ : getOpenMPClauseKind(PP.getSpelling(Tok));
+ if (HasImplicitClause) {
+ assert(CKind == OMPC_unknown && "Must be unknown implicit clause.");
+ if (DKind == OMPD_flush) {
+ CKind = OMPC_flush;
+ } else {
+ assert(DKind == OMPD_depobj && "Expected flush or depobj directives.");
+ CKind = OMPC_depobj;
+ }
+ }
+ // No more implicit clauses allowed.
+ ImplicitClauseAllowed = false;
+ Actions.OpenMP().StartOpenMPClause(CKind);
+ HasImplicitClause = false;
+ OMPClause *Clause =
+ ParseOpenMPClause(DKind, CKind, !SeenClauses[unsigned(CKind)]);
+ SeenClauses[unsigned(CKind)] = true;
+ if (Clause)
+ Clauses.push_back(Clause);
+
+ // Skip ',' if any.
+ if (Tok.is(tok::comma))
+ ConsumeToken();
+ Actions.OpenMP().EndOpenMPClause();
+ }
+ // End location of the directive.
+ EndLoc = Tok.getLocation();
+ // Consume final annot_pragma_openmp_end.
+ ConsumeAnnotationToken();
+
+ if (DKind == OMPD_ordered) {
+ // If the depend or doacross clause is specified, the ordered construct
+ // is a stand-alone directive.
+ for (auto CK : {OMPC_depend, OMPC_doacross}) {
+ if (SeenClauses[unsigned(CK)]) {
+ if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) ==
+ ParsedStmtContext()) {
+ Diag(Loc, diag::err_omp_immediate_directive)
+ << getOpenMPDirectiveName(DKind) << 1 << getOpenMPClauseName(CK);
+ }
+ HasAssociatedStatement = false;
+ }
+ }
+ }
+
+ if (DKind == OMPD_tile && !SeenClauses[unsigned(OMPC_sizes)]) {
+ Diag(Loc, diag::err_omp_required_clause)
+ << getOpenMPDirectiveName(OMPD_tile) << "sizes";
+ }
+
+ StmtResult AssociatedStmt;
+ if (HasAssociatedStatement) {
+ // The body is a block scope like in Lambdas and Blocks.
+ Actions.OpenMP().ActOnOpenMPRegionStart(DKind, getCurScope());
+ // FIXME: We create a bogus CompoundStmt scope to hold the contents of
+ // the captured region. Code elsewhere assumes that any FunctionScopeInfo
+ // should have at least one compound statement scope within it.
+ ParsingOpenMPDirectiveRAII NormalScope(*this, /*Value=*/false);
+ {
+ Sema::CompoundScopeRAII Scope(Actions);
+ AssociatedStmt = ParseStatement();
+
+ if (AssociatedStmt.isUsable() && isOpenMPLoopDirective(DKind) &&
+ getLangOpts().OpenMPIRBuilder)
+ AssociatedStmt =
+ Actions.OpenMP().ActOnOpenMPLoopnest(AssociatedStmt.get());
+ }
+ AssociatedStmt =
+ Actions.OpenMP().ActOnOpenMPRegionEnd(AssociatedStmt, Clauses);
+ } else if (DKind == OMPD_target_update || DKind == OMPD_target_enter_data ||
+ DKind == OMPD_target_exit_data) {
+ Actions.OpenMP().ActOnOpenMPRegionStart(DKind, getCurScope());
+ AssociatedStmt = (Sema::CompoundScopeRAII(Actions),
+ Actions.ActOnCompoundStmt(Loc, Loc, std::nullopt,
+ /*isStmtExpr=*/false));
+ AssociatedStmt =
+ Actions.OpenMP().ActOnOpenMPRegionEnd(AssociatedStmt, Clauses);
+ }
+
+ StmtResult Directive = Actions.OpenMP().ActOnOpenMPExecutableDirective(
+ DKind, DirName, CancelRegion, Clauses, AssociatedStmt.get(), Loc, EndLoc);
+
+ // Exit scope.
+ Actions.OpenMP().EndOpenMPDSABlock(Directive.get());
+ OMPDirectiveScope.Exit();
+
+ return Directive;
}
/// Parsing of declarative or executable OpenMP directives.
@@ -2503,24 +2626,36 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
"Not an OpenMP directive!");
ParsingOpenMPDirectiveRAII DirScope(*this);
ParenBraceBracketBalancer BalancerRAIIObj(*this);
- SmallVector<OMPClause *, 5> Clauses;
- std::bitset<llvm::omp::Clause_enumSize + 1> SeenClauses;
- unsigned ScopeFlags = Scope::FnScope | Scope::DeclScope |
- Scope::CompoundStmtScope | Scope::OpenMPDirectiveScope;
SourceLocation Loc = ReadDirectiveWithinMetadirective
? Tok.getLocation()
- : ConsumeAnnotationToken(),
- EndLoc;
+ : ConsumeAnnotationToken();
OpenMPDirectiveKind DKind = parseOpenMPDirectiveKind(*this);
if (ReadDirectiveWithinMetadirective && DKind == OMPD_unknown) {
Diag(Tok, diag::err_omp_unknown_directive);
return StmtError();
}
- OpenMPDirectiveKind CancelRegion = OMPD_unknown;
- // Name of critical directive.
- DeclarationNameInfo DirName;
+
StmtResult Directive = StmtError();
- bool HasAssociatedStatement = true;
+
+ bool IsExecutable = [&]() {
+ if (DKind == OMPD_error) // OMPD_error is handled as executable
+ return true;
+ switch (getDirectiveCategory(DKind)) {
+ case Category::Executable:
+ case Category::Subsidiary:
+ return true;
+ default:
+ break;
+ }
+ return false;
+ }();
+
+ if (IsExecutable) {
+ Directive = ParseOpenMPExecutableDirective(
+ StmtCtx, DKind, Loc, ReadDirectiveWithinMetadirective);
+ assert(!Directive.isUnset());
+ return Directive;
+ }
switch (DKind) {
case OMPD_nothing:
@@ -2763,233 +2898,6 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
}
break;
}
- case OMPD_flush:
- case OMPD_depobj:
- case OMPD_scan:
- case OMPD_taskyield:
- case OMPD_error:
- case OMPD_barrier:
- case OMPD_taskwait:
- case OMPD_cancellation_point:
- case OMPD_cancel:
- case OMPD_target_enter_data:
- case OMPD_target_exit_data:
- case OMPD_target_update:
- case OMPD_interop:
- if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) ==
- ParsedStmtContext()) {
- Diag(Tok, diag::err_omp_immediate_directive)
- << getOpenMPDirectiveName(DKind) << 0;
- if (DKind == OMPD_error) {
- SkipUntil(tok::annot_pragma_openmp_end);
- break;
- }
- }
- HasAssociatedStatement = false;
- // Fall through for further analysis.
- [[fallthrough]];
- case OMPD_parallel:
- case OMPD_simd:
- case OMPD_tile:
- case OMPD_unroll:
- case OMPD_for:
- case OMPD_for_simd:
- case OMPD_sections:
- case OMPD_single:
- case OMPD_section:
- case OMPD_master:
- case OMPD_critical:
- case OMPD_parallel_for:
- case OMPD_parallel_for_simd:
- case OMPD_parallel_sections:
- case OMPD_parallel_master:
- case OMPD_parallel_masked:
- case OMPD_task:
- case OMPD_ordered:
- case OMPD_atomic:
- case OMPD_target:
- case OMPD_teams:
- case OMPD_taskgroup:
- case OMPD_target_data:
- case OMPD_target_parallel:
- case OMPD_target_parallel_for:
- case OMPD_loop:
- case OMPD_teams_loop:
- case OMPD_target_teams_loop:
- case OMPD_parallel_loop:
- case OMPD_target_parallel_loop:
- case OMPD_scope:
- case OMPD_taskloop:
- case OMPD_taskloop_simd:
- case OMPD_master_taskloop:
- case OMPD_masked_taskloop:
- case OMPD_master_taskloop_simd:
- case OMPD_masked_taskloop_simd:
- case OMPD_parallel_master_taskloop:
- case OMPD_parallel_masked_taskloop:
- case OMPD_parallel_master_taskloop_simd:
- case OMPD_parallel_masked_taskloop_simd:
- case OMPD_distribute:
- case OMPD_distribute_parallel_for:
- case OMPD_distribute_parallel_for_simd:
- case OMPD_distribute_simd:
- case OMPD_target_parallel_for_simd:
- case OMPD_target_simd:
- case OMPD_teams_distribute:
- case OMPD_teams_distribute_simd:
- case OMPD_teams_distribute_parallel_for_simd:
- case OMPD_teams_distribute_parallel_for:
- case OMPD_target_teams:
- case OMPD_target_teams_distribute:
- case OMPD_target_teams_distribute_parallel_for:
- case OMPD_target_teams_distribute_parallel_for_simd:
- case OMPD_target_teams_distribute_simd:
- case OMPD_dispatch:
- case OMPD_masked: {
- // Special processing for flush and depobj clauses.
- Token ImplicitTok;
- bool ImplicitClauseAllowed = false;
- if (DKind == OMPD_flush || DKind == OMPD_depobj) {
- ImplicitTok = Tok;
- ImplicitClauseAllowed = true;
- }
- ConsumeToken();
- // Parse directive name of the 'critical' directive if any.
- if (DKind == OMPD_critical) {
- BalancedDelimiterTracker T(*this, tok::l_paren,
- tok::annot_pragma_openmp_end);
- if (!T.consumeOpen()) {
- if (Tok.isAnyIdentifier()) {
- DirName =
- DeclarationNameInfo(Tok.getIdentifierInfo(), Tok.getLocation());
- ConsumeAnyToken();
- } else {
- Diag(Tok, diag::err_omp_expected_identifier_for_critical);
- }
- T.consumeClose();
- }
- } else if (DKind == OMPD_cancellation_point || DKind == OMPD_cancel) {
- CancelRegion = parseOpenMPDirectiveKind(*this);
- if (Tok.isNot(tok::annot_pragma_openmp_end))
- ConsumeToken();
- }
-
- if (isOpenMPLoopDirective(DKind))
- ScopeFlags |= Scope::OpenMPLoopDirectiveScope;
- if (isOpenMPSimdDirective(DKind))
- ScopeFlags |= Scope::OpenMPSimdDirectiveScope;
- ParseScope OMPDirectiveScope(this, ScopeFlags);
- Actions.OpenMP().StartOpenMPDSABlock(DKind, DirName, Actions.getCurScope(),
- Loc);
-
- while (Tok.isNot(tok::annot_pragma_openmp_end)) {
- // If we are parsing for a directive within a metadirective, the directive
- // ends with a ')'.
- if (ReadDirectiveWithinMetadirective && Tok.is(tok::r_paren)) {
- while (Tok.isNot(tok::annot_pragma_openmp_end))
- ConsumeAnyToken();
- break;
- }
- bool HasImplicitClause = false;
- if (ImplicitClauseAllowed && Tok.is(tok::l_paren)) {
- HasImplicitClause = true;
- // Push copy of the current token back to stream to properly parse
- // pseudo-clause OMPFlushClause or OMPDepobjClause.
- PP.EnterToken(Tok, /*IsReinject*/ true);
- PP.EnterToken(ImplicitTok, /*IsReinject*/ true);
- ConsumeAnyToken();
- }
- OpenMPClauseKind CKind = Tok.isAnnotation()
- ? OMPC_unknown
- : getOpenMPClauseKind(PP.getSpelling(Tok));
- if (HasImplicitClause) {
- assert(CKind == OMPC_unknown && "Must be unknown implicit clause.");
- if (DKind == OMPD_flush) {
- CKind = OMPC_flush;
- } else {
- assert(DKind == OMPD_depobj &&
- "Expected flush or depobj directives.");
- CKind = OMPC_depobj;
- }
- }
- // No more implicit clauses allowed.
- ImplicitClauseAllowed = false;
- Actions.OpenMP().StartOpenMPClause(CKind);
- HasImplicitClause = false;
- OMPClause *Clause =
- ParseOpenMPClause(DKind, CKind, !SeenClauses[unsigned(CKind)]);
- SeenClauses[unsigned(CKind)] = true;
- if (Clause)
- Clauses.push_back(Clause);
-
- // Skip ',' if any.
- if (Tok.is(tok::comma))
- ConsumeToken();
- Actions.OpenMP().EndOpenMPClause();
- }
- // End location of the directive.
- EndLoc = Tok.getLocation();
- // Consume final annot_pragma_openmp_end.
- ConsumeAnnotationToken();
-
- if (DKind == OMPD_ordered) {
- // If the depend or doacross clause is specified, the ordered construct
- // is a stand-alone directive.
- for (auto CK : {OMPC_depend, OMPC_doacross}) {
- if (SeenClauses[unsigned(CK)]) {
- if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) ==
- ParsedStmtContext()) {
- Diag(Loc, diag::err_omp_immediate_directive)
- << getOpenMPDirectiveName(DKind) << 1
- << getOpenMPClauseName(CK);
- }
- HasAssociatedStatement = false;
- }
- }
- }
-
- if (DKind == OMPD_tile && !SeenClauses[unsigned(OMPC_sizes)]) {
- Diag(Loc, diag::err_omp_required_clause)
- << getOpenMPDirectiveName(OMPD_tile) << "sizes";
- }
-
- StmtResult AssociatedStmt;
- if (HasAssociatedStatement) {
- // The body is a block scope like in Lambdas and Blocks.
- Actions.OpenMP().ActOnOpenMPRegionStart(DKind, getCurScope());
- // FIXME: We create a bogus CompoundStmt scope to hold the contents of
- // the captured region. Code elsewhere assumes that any FunctionScopeInfo
- // should have at least one compound statement scope within it.
- ParsingOpenMPDirectiveRAII NormalScope(*this, /*Value=*/false);
- {
- Sema::CompoundScopeRAII Scope(Actions);
- AssociatedStmt = ParseStatement();
-
- if (AssociatedStmt.isUsable() && isOpenMPLoopDirective(DKind) &&
- getLangOpts().OpenMPIRBuilder)
- AssociatedStmt =
- Actions.OpenMP().ActOnOpenMPLoopnest(Assoc...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/94691
More information about the llvm-branch-commits
mailing list