[clang] 89167e3 - [OpenMP][NFC] Refactor code for interop parts of 'init' and 'append_args' clauses
Mike Rice via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 18 17:14:00 PDT 2022
Author: Mike Rice
Date: 2022-08-18T17:13:30-07:00
New Revision: 89167e3c5b008b44f1fa8a222652e7bdc62cfa8a
URL: https://github.com/llvm/llvm-project/commit/89167e3c5b008b44f1fa8a222652e7bdc62cfa8a
DIFF: https://github.com/llvm/llvm-project/commit/89167e3c5b008b44f1fa8a222652e7bdc62cfa8a.diff
LOG: [OpenMP][NFC] Refactor code for interop parts of 'init' and 'append_args' clauses
The 'init' clause allows an interop-modifier of prefer_type(list) and
and interop-types 'target' and 'targetsync'.
The 'append_args' clause uses an append-op that also includes
interop-types ('target' and 'targetsync') and will allow
a prefer_type list in the next OpenMP version.
This change adds a helper struct OMPInteropInfo and uses it in the parsing
of both the 'init' and 'append_args' clauses.
One OMPInteropInfo object represents the info in a single 'init' clause.
Since 'append_args' allows a variable number of interop items it will
require an array of OMPInteropInfo objects once that is supported.
Differential Revision: https://reviews.llvm.org/D132171
Added:
Modified:
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/OpenMPClause.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/TreeTransform.h
Removed:
################################################################################
diff --git a/clang/include/clang/AST/OpenMPClause.h b/clang/include/clang/AST/OpenMPClause.h
index 072519462b9f..a3982d7ba0ae 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -7709,6 +7709,13 @@ class OMPOrderClause final : public OMPClause {
}
};
+/// Contains 'interop' data for 'append_args' and 'init' clauses.
+struct OMPInteropInfo final {
+ bool IsTarget = false;
+ bool IsTargetSync = false;
+ llvm::SmallVector<Expr *, 4> PreferTypes;
+};
+
/// This represents the 'init' clause in '#pragma omp ...' directives.
///
/// \code
@@ -7763,16 +7770,14 @@ class OMPInitClause final
///
/// \param C AST context.
/// \param InteropVar The interop variable.
- /// \param PrefExprs The list of preference expressions.
- /// \param IsTarget Uses the 'target' interop-type.
- /// \param IsTargetSync Uses the 'targetsync' interop-type.
+ /// \param InteropInfo The interop-type and prefer_type list.
/// \param StartLoc Starting location of the clause.
/// \param LParenLoc Location of '('.
/// \param VarLoc Location of the interop variable.
/// \param EndLoc Ending location of the clause.
static OMPInitClause *Create(const ASTContext &C, Expr *InteropVar,
- ArrayRef<Expr *> PrefExprs, bool IsTarget,
- bool IsTargetSync, SourceLocation StartLoc,
+ OMPInteropInfo &InteropInfo,
+ SourceLocation StartLoc,
SourceLocation LParenLoc, SourceLocation VarLoc,
SourceLocation EndLoc);
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 41bfc9f48ecc..567433f2a889 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3332,6 +3332,9 @@ class Parser : public CodeCompletionHandler {
/// '(' { <allocator> [ '(' <allocator_traits> ')' ] }+ ')'
OMPClause *ParseOpenMPUsesAllocatorClause(OpenMPDirectiveKind DKind);
+ /// Parses the 'interop' parts of the 'append_args' and 'init' clauses.
+ bool ParseOMPInteropInfo(OMPInteropInfo &InteropInfo, OpenMPClauseKind Kind);
+
/// Parses clause with an interop variable of kind \a Kind.
///
/// \param Kind Kind of current clause.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 85641f595d41..51e526a62274 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11621,12 +11621,10 @@ class Sema final {
SourceLocation EndLoc);
/// Called on well-formed 'init' clause.
- OMPClause *ActOnOpenMPInitClause(Expr *InteropVar, ArrayRef<Expr *> PrefExprs,
- bool IsTarget, bool IsTargetSync,
- SourceLocation StartLoc,
- SourceLocation LParenLoc,
- SourceLocation VarLoc,
- SourceLocation EndLoc);
+ OMPClause *
+ ActOnOpenMPInitClause(Expr *InteropVar, OMPInteropInfo &InteropInfo,
+ SourceLocation StartLoc, SourceLocation LParenLoc,
+ SourceLocation VarLoc, SourceLocation EndLoc);
/// Called on well-formed 'use' clause.
OMPClause *ActOnOpenMPUseClause(Expr *InteropVar, SourceLocation StartLoc,
diff --git a/clang/lib/AST/OpenMPClause.cpp b/clang/lib/AST/OpenMPClause.cpp
index dc2d90e366bc..214ae5bd6be4 100644
--- a/clang/lib/AST/OpenMPClause.cpp
+++ b/clang/lib/AST/OpenMPClause.cpp
@@ -1626,18 +1626,19 @@ OMPAffinityClause *OMPAffinityClause::CreateEmpty(const ASTContext &C,
}
OMPInitClause *OMPInitClause::Create(const ASTContext &C, Expr *InteropVar,
- ArrayRef<Expr *> PrefExprs, bool IsTarget,
- bool IsTargetSync, SourceLocation StartLoc,
+ OMPInteropInfo &InteropInfo,
+ SourceLocation StartLoc,
SourceLocation LParenLoc,
SourceLocation VarLoc,
SourceLocation EndLoc) {
- void *Mem = C.Allocate(totalSizeToAlloc<Expr *>(PrefExprs.size() + 1));
- auto *Clause =
- new (Mem) OMPInitClause(IsTarget, IsTargetSync, StartLoc, LParenLoc,
- VarLoc, EndLoc, PrefExprs.size() + 1);
+ void *Mem =
+ C.Allocate(totalSizeToAlloc<Expr *>(InteropInfo.PreferTypes.size() + 1));
+ auto *Clause = new (Mem) OMPInitClause(
+ InteropInfo.IsTarget, InteropInfo.IsTargetSync, StartLoc, LParenLoc,
+ VarLoc, EndLoc, InteropInfo.PreferTypes.size() + 1);
Clause->setInteropVar(InteropVar);
- llvm::copy(PrefExprs, Clause->getTrailingObjects<Expr *>() + 1);
+ llvm::copy(InteropInfo.PreferTypes, Clause->getTrailingObjects<Expr *>() + 1);
return Clause;
}
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index c0261e1e868c..2421f8d5beb5 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -1502,54 +1502,6 @@ void Parser::ParseOMPDeclareVariantClauses(Parser::DeclGroupPtrTy Ptr,
(void)ConsumeAnnotationToken();
}
-/// Parse a list of interop-types. These are 'target' and 'targetsync'. Both
-/// are allowed but duplication of either is not meaningful.
-static Optional<OMPDeclareVariantAttr::InteropType>
-parseInteropTypeList(Parser &P) {
- const Token &Tok = P.getCurToken();
- bool HasError = false;
- bool IsTarget = false;
- bool IsTargetSync = false;
-
- while (Tok.is(tok::identifier)) {
- if (Tok.getIdentifierInfo()->isStr("target")) {
- // OpenMP 5.1 [2.15.1, interop Construct, Restrictions]
- // Each interop-type may be specified on an action-clause at most
- // once.
- if (IsTarget)
- P.Diag(Tok, diag::warn_omp_more_one_interop_type) << "target";
- IsTarget = true;
- } else if (Tok.getIdentifierInfo()->isStr("targetsync")) {
- if (IsTargetSync)
- P.Diag(Tok, diag::warn_omp_more_one_interop_type) << "targetsync";
- IsTargetSync = true;
- } else {
- HasError = true;
- P.Diag(Tok, diag::err_omp_expected_interop_type);
- }
- P.ConsumeToken();
-
- if (!Tok.is(tok::comma))
- break;
- P.ConsumeToken();
- }
- if (HasError)
- return None;
-
- if (!IsTarget && !IsTargetSync) {
- P.Diag(Tok, diag::err_omp_expected_interop_type);
- return None;
- }
-
- // As of OpenMP 5.1,there are two interop-types, "target" and
- // "targetsync". Either or both are allowed for a single interop.
- if (IsTarget && IsTargetSync)
- return OMPDeclareVariantAttr::Target_TargetSync;
- if (IsTarget)
- return OMPDeclareVariantAttr::Target;
- return OMPDeclareVariantAttr::TargetSync;
-}
-
bool Parser::parseOpenMPAppendArgs(
SmallVectorImpl<OMPDeclareVariantAttr::InteropType> &InterOpTypes) {
bool HasError = false;
@@ -1568,12 +1520,21 @@ bool Parser::parseOpenMPAppendArgs(
if (IT.expectAndConsume(diag::err_expected_lparen_after, "interop"))
return true;
- // Parse the interop-types.
- if (Optional<OMPDeclareVariantAttr::InteropType> IType =
- parseInteropTypeList(*this))
- InterOpTypes.push_back(*IType);
- else
+ OMPInteropInfo InteropInfo;
+ if (ParseOMPInteropInfo(InteropInfo, OMPC_append_args)) {
HasError = true;
+ } else {
+ OMPDeclareVariantAttr::InteropType IT;
+ // As of OpenMP 5.1, there are two interop-types, "target" and
+ // "targetsync". Either or both are allowed for a single interop.
+ if (InteropInfo.IsTarget && InteropInfo.IsTargetSync)
+ IT = OMPDeclareVariantAttr::Target_TargetSync;
+ else if (InteropInfo.IsTarget)
+ IT = OMPDeclareVariantAttr::Target;
+ else
+ IT = OMPDeclareVariantAttr::TargetSync;
+ InterOpTypes.push_back(IT);
+ }
IT.consumeClose();
if (Tok.is(tok::comma))
@@ -3496,6 +3457,90 @@ bool Parser::ParseOpenMPIndirectClause(Sema::DeclareTargetContextInfo &DTCI,
return false;
}
+/// Parses a comma-separated list of interop-types and a prefer_type list.
+///
+bool Parser::ParseOMPInteropInfo(OMPInteropInfo &InteropInfo,
+ OpenMPClauseKind Kind) {
+ const Token &Tok = getCurToken();
+ bool HasError = false;
+ bool IsTarget = false;
+ bool IsTargetSync = false;
+
+ while (Tok.is(tok::identifier)) {
+ // Currently prefer_type is only allowed with 'init' and it must be first.
+ bool PreferTypeAllowed = Kind == OMPC_init &&
+ InteropInfo.PreferTypes.empty() && !IsTarget &&
+ !IsTargetSync;
+ if (Tok.getIdentifierInfo()->isStr("target")) {
+ // OpenMP 5.1 [2.15.1, interop Construct, Restrictions]
+ // Each interop-type may be specified on an action-clause at most
+ // once.
+ if (IsTarget)
+ Diag(Tok, diag::warn_omp_more_one_interop_type) << "target";
+ IsTarget = true;
+ ConsumeToken();
+ } else if (Tok.getIdentifierInfo()->isStr("targetsync")) {
+ if (IsTargetSync)
+ Diag(Tok, diag::warn_omp_more_one_interop_type) << "targetsync";
+ IsTargetSync = true;
+ ConsumeToken();
+ } else if (Tok.getIdentifierInfo()->isStr("prefer_type") &&
+ PreferTypeAllowed) {
+ ConsumeToken();
+ BalancedDelimiterTracker PT(*this, tok::l_paren,
+ tok::annot_pragma_openmp_end);
+ if (PT.expectAndConsume(diag::err_expected_lparen_after, "prefer_type"))
+ HasError = true;
+
+ while (Tok.isNot(tok::r_paren)) {
+ SourceLocation Loc = Tok.getLocation();
+ ExprResult LHS = ParseCastExpression(AnyCastExpr);
+ ExprResult PTExpr = Actions.CorrectDelayedTyposInExpr(
+ ParseRHSOfBinaryExpression(LHS, prec::Conditional));
+ PTExpr = Actions.ActOnFinishFullExpr(PTExpr.get(), Loc,
+ /*DiscardedValue=*/false);
+ if (PTExpr.isUsable()) {
+ InteropInfo.PreferTypes.push_back(PTExpr.get());
+ } else {
+ HasError = true;
+ SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,
+ StopBeforeMatch);
+ }
+
+ if (Tok.is(tok::comma))
+ ConsumeToken();
+ }
+ PT.consumeClose();
+ } else {
+ HasError = true;
+ Diag(Tok, diag::err_omp_expected_interop_type);
+ ConsumeToken();
+ }
+ if (!Tok.is(tok::comma))
+ break;
+ ConsumeToken();
+ }
+
+ if (!HasError && !IsTarget && !IsTargetSync) {
+ Diag(Tok, diag::err_omp_expected_interop_type);
+ HasError = true;
+ }
+
+ if (Kind == OMPC_init) {
+ if (Tok.isNot(tok::colon) && (IsTarget || IsTargetSync))
+ Diag(Tok, diag::warn_pragma_expected_colon) << "interop types";
+ if (Tok.is(tok::colon))
+ ConsumeToken();
+ }
+
+ // As of OpenMP 5.1,there are two interop-types, "target" and
+ // "targetsync". Either or both are allowed for a single interop.
+ InteropInfo.IsTarget = IsTarget;
+ InteropInfo.IsTargetSync = IsTargetSync;
+
+ return HasError;
+}
+
/// Parsing of OpenMP clauses that use an interop-var.
///
/// init-clause:
@@ -3528,57 +3573,10 @@ OMPClause *Parser::ParseOpenMPInteropClause(OpenMPClauseKind Kind,
getOpenMPClauseName(Kind).data()))
return nullptr;
- bool IsTarget = false;
- bool IsTargetSync = false;
- SmallVector<Expr *, 4> Prefs;
-
- if (Kind == OMPC_init) {
-
- // Parse optional interop-modifier.
- if (Tok.is(tok::identifier) && PP.getSpelling(Tok) == "prefer_type") {
- ConsumeToken();
- BalancedDelimiterTracker PT(*this, tok::l_paren,
- tok::annot_pragma_openmp_end);
- if (PT.expectAndConsume(diag::err_expected_lparen_after, "prefer_type"))
- return nullptr;
-
- while (Tok.isNot(tok::r_paren)) {
- SourceLocation Loc = Tok.getLocation();
- ExprResult LHS = ParseCastExpression(AnyCastExpr);
- ExprResult PTExpr = Actions.CorrectDelayedTyposInExpr(
- ParseRHSOfBinaryExpression(LHS, prec::Conditional));
- PTExpr = Actions.ActOnFinishFullExpr(PTExpr.get(), Loc,
- /*DiscardedValue=*/false);
- if (PTExpr.isUsable())
- Prefs.push_back(PTExpr.get());
- else
- SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,
- StopBeforeMatch);
-
- if (Tok.is(tok::comma))
- ConsumeToken();
- }
- PT.consumeClose();
- }
-
- if (!Prefs.empty()) {
- if (Tok.is(tok::comma))
- ConsumeToken();
- else
- Diag(Tok, diag::err_omp_expected_punc_after_interop_mod);
- }
-
- // Parse the interop-types.
- if (Optional<OMPDeclareVariantAttr::InteropType> IType =
- parseInteropTypeList(*this)) {
- IsTarget = IType != OMPDeclareVariantAttr::TargetSync;
- IsTargetSync = IType != OMPDeclareVariantAttr::Target;
- if (Tok.isNot(tok::colon))
- Diag(Tok, diag::warn_pragma_expected_colon) << "interop types";
- }
- if (Tok.is(tok::colon))
- ConsumeToken();
- }
+ bool InteropError = false;
+ OMPInteropInfo InteropInfo;
+ if (Kind == OMPC_init)
+ InteropError = ParseOMPInteropInfo(InteropInfo, OMPC_init);
// Parse the variable.
SourceLocation VarLoc = Tok.getLocation();
@@ -3594,14 +3592,12 @@ OMPClause *Parser::ParseOpenMPInteropClause(OpenMPClauseKind Kind,
if (!T.consumeClose())
RLoc = T.getCloseLocation();
- if (ParseOnly || !InteropVarExpr.isUsable() ||
- (Kind == OMPC_init && !IsTarget && !IsTargetSync))
+ if (ParseOnly || !InteropVarExpr.isUsable() || InteropError)
return nullptr;
if (Kind == OMPC_init)
- return Actions.ActOnOpenMPInitClause(InteropVarExpr.get(), Prefs, IsTarget,
- IsTargetSync, Loc, T.getOpenLocation(),
- VarLoc, RLoc);
+ return Actions.ActOnOpenMPInitClause(InteropVarExpr.get(), InteropInfo, Loc,
+ T.getOpenLocation(), VarLoc, RLoc);
if (Kind == OMPC_use)
return Actions.ActOnOpenMPUseClause(InteropVarExpr.get(), Loc,
T.getOpenLocation(), VarLoc, RLoc);
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index c703d7670abe..4f1f69009327 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -17344,8 +17344,7 @@ static bool isValidInteropVariable(Sema &SemaRef, Expr *InteropVarExpr,
}
OMPClause *
-Sema::ActOnOpenMPInitClause(Expr *InteropVar, ArrayRef<Expr *> PrefExprs,
- bool IsTarget, bool IsTargetSync,
+Sema::ActOnOpenMPInitClause(Expr *InteropVar, OMPInteropInfo &InteropInfo,
SourceLocation StartLoc, SourceLocation LParenLoc,
SourceLocation VarLoc, SourceLocation EndLoc) {
@@ -17354,7 +17353,7 @@ Sema::ActOnOpenMPInitClause(Expr *InteropVar, ArrayRef<Expr *> PrefExprs,
// Check prefer_type values. These foreign-runtime-id values are either
// string literals or constant integral expressions.
- for (const Expr *E : PrefExprs) {
+ for (const Expr *E : InteropInfo.PreferTypes) {
if (E->isValueDependent() || E->isTypeDependent() ||
E->isInstantiationDependent() || E->containsUnexpandedParameterPack())
continue;
@@ -17366,9 +17365,8 @@ Sema::ActOnOpenMPInitClause(Expr *InteropVar, ArrayRef<Expr *> PrefExprs,
return nullptr;
}
- return OMPInitClause::Create(Context, InteropVar, PrefExprs, IsTarget,
- IsTargetSync, StartLoc, LParenLoc, VarLoc,
- EndLoc);
+ return OMPInitClause::Create(Context, InteropVar, InteropInfo, StartLoc,
+ LParenLoc, VarLoc, EndLoc);
}
OMPClause *Sema::ActOnOpenMPUseClause(Expr *InteropVar, SourceLocation StartLoc,
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index fa7f34c415dc..db9fa77d3937 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -2216,15 +2216,13 @@ class TreeTransform {
///
/// By default, performs semantic analysis to build the new OpenMP clause.
/// Subclasses may override this routine to provide
diff erent behavior.
- OMPClause *RebuildOMPInitClause(Expr *InteropVar, ArrayRef<Expr *> PrefExprs,
- bool IsTarget, bool IsTargetSync,
+ OMPClause *RebuildOMPInitClause(Expr *InteropVar, OMPInteropInfo &InteropInfo,
SourceLocation StartLoc,
SourceLocation LParenLoc,
SourceLocation VarLoc,
SourceLocation EndLoc) {
- return getSema().ActOnOpenMPInitClause(InteropVar, PrefExprs, IsTarget,
- IsTargetSync, StartLoc, LParenLoc,
- VarLoc, EndLoc);
+ return getSema().ActOnOpenMPInitClause(InteropVar, InteropInfo, StartLoc,
+ LParenLoc, VarLoc, EndLoc);
}
/// Build a new OpenMP 'use' clause.
@@ -9673,17 +9671,19 @@ OMPClause *TreeTransform<Derived>::TransformOMPInitClause(OMPInitClause *C) {
if (IVR.isInvalid())
return nullptr;
- llvm::SmallVector<Expr *, 8> PrefExprs;
- PrefExprs.reserve(C->varlist_size() - 1);
+ OMPInteropInfo InteropInfo;
+ InteropInfo.IsTarget = C->getIsTarget();
+ InteropInfo.IsTargetSync = C->getIsTargetSync();
+ InteropInfo.PreferTypes.reserve(C->varlist_size() - 1);
for (Expr *E : llvm::drop_begin(C->varlists())) {
ExprResult ER = getDerived().TransformExpr(cast<Expr>(E));
if (ER.isInvalid())
return nullptr;
- PrefExprs.push_back(ER.get());
+ InteropInfo.PreferTypes.push_back(ER.get());
}
- return getDerived().RebuildOMPInitClause(
- IVR.get(), PrefExprs, C->getIsTarget(), C->getIsTargetSync(),
- C->getBeginLoc(), C->getLParenLoc(), C->getVarLoc(), C->getEndLoc());
+ return getDerived().RebuildOMPInitClause(IVR.get(), InteropInfo,
+ C->getBeginLoc(), C->getLParenLoc(),
+ C->getVarLoc(), C->getEndLoc());
}
template <typename Derived>
More information about the cfe-commits
mailing list