llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-flang-openmp Author: Krzysztof Parzyszek (kparzysz) <details> <summary>Changes</summary> For clauses that take list of variable, locator, and extended list items, perform checks that the actual arguments meet the corresponding requirements. This is version-based, since clause requirements have changed over time. --- Patch is 33.65 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/201334.diff 18 Files Affected: - (modified) flang/include/flang/Semantics/openmp-utils.h (+25) - (modified) flang/lib/Semantics/check-omp-structure.cpp (+123-89) - (modified) flang/lib/Semantics/check-omp-structure.h (+1-1) - (modified) flang/lib/Semantics/openmp-utils.cpp (+224-1) - (modified) flang/test/Semantics/OpenMP/copyprivate04.f90 (+1-1) - (modified) flang/test/Semantics/OpenMP/copyprivate05.f90 (+1-1) - (modified) flang/test/Semantics/OpenMP/declare-target01.f90 (+15-15) - (modified) flang/test/Semantics/OpenMP/from-clause-v45.f90 (+1-1) - (modified) flang/test/Semantics/OpenMP/from-clause-v51.f90 (+1-1) - (modified) flang/test/Semantics/OpenMP/in-reduction.f90 (+1-1) - (modified) flang/test/Semantics/OpenMP/lastprivate01.f90 (+1-2) - (modified) flang/test/Semantics/OpenMP/name-conflict.f90 (+2-2) - (modified) flang/test/Semantics/OpenMP/named-constants.f90 (+4-4) - (modified) flang/test/Semantics/OpenMP/reduction04.f90 (+1-2) - (modified) flang/test/Semantics/OpenMP/reduction16.f90 (+1-1) - (modified) flang/test/Semantics/OpenMP/task-reduction.f90 (+1-1) - (modified) flang/test/Semantics/OpenMP/to-clause-v45.f90 (+1-1) - (modified) flang/test/Semantics/OpenMP/to-clause-v51.f90 (+1-1) ``````````diff diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h index 4001b337193a1..0b6bd40ad84c2 100644 --- a/flang/include/flang/Semantics/openmp-utils.h +++ b/flang/include/flang/Semantics/openmp-utils.h @@ -95,12 +95,20 @@ bool IsCommonBlock(const Symbol &sym); bool IsExtendedListItem(const Symbol &sym); bool IsVariableListItem(const Symbol &sym); bool IsTypeParamInquiry(const Symbol &sym); +bool IsComplexPart(const Symbol &sym); bool IsStructureComponent(const Symbol &sym); bool IsPrivatizable(const Symbol &sym); bool IsVarOrFunctionRef(const MaybeExpr &expr); bool IsWholeAssumedSizeArray(const parser::OmpObject &object); +bool IsExtendedListItem( + const parser::OmpObject &object, SemanticsContext *semaCtx); +bool IsLocatorListItem( + const parser::OmpObject &object, SemanticsContext *semaCtx); +bool IsVariableListItem( + const parser::OmpObject &object, SemanticsContext *semaCtx); + const Symbol *GetHostSymbol(const Symbol &sym); bool IsMapEnteringType(parser::OmpMapType::Value type); @@ -139,6 +147,23 @@ bool IsPointerAssignment(const evaluate::Assignment &x); MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp); +enum struct ListItemKind : uint32_t { + Depend, + DirectiveName, + DirectiveSpecification, + Extended, + IntegerExpression, + Interop, + Locator, + Operation, + Parameter, + ProcedureArgument, + Variable, +}; + +std::optional<ListItemKind> GetArgumentListItemKind( + llvm::omp::Clause clause, unsigned version); + bool IsLoopTransforming(llvm::omp::Directive dir); bool HasDataEnvironment(llvm::omp::Directive dir); diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 055fa2325f5a1..94187aff125ee 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -424,12 +424,6 @@ void OmpStructureChecker::AnalyzeObject(const parser::OmpObject &object) { object.u); } -void OmpStructureChecker::AnalyzeObjects(const parser::OmpObjectList &objects) { - for (const parser::OmpObject &object : objects.v) { - AnalyzeObject(object); - } -} - const parser::OpenMPConstruct * OmpStructureChecker::GetCurrentConstruct() const { for (const LoopOrConstruct &c : llvm::reverse(constructStack_)) { @@ -624,12 +618,120 @@ bool OmpStructureChecker::HasRequires(llvm::omp::Clause req) { DEREF(unit.symbol()).details()); } -void OmpStructureChecker::CheckVariableListItem( - const SymbolSourceMap &symbols) { - for (auto &[symbol, source] : symbols) { - if (!IsVariableListItem(*symbol)) { - context_.SayWithDecl( - *symbol, source, "'%s' must be a variable"_err_en_US, symbol->name()); +void OmpStructureChecker::CheckArgumentObjectKind(const parser::OmpClause &x) { + unsigned version{context_.langOptions().OpenMPVersion}; + llvm::omp::Directive dirId{GetContext().directive}; + llvm::omp::Clause clauseId{x.Id()}; + + // Filter out clauses that don't take OmpObjectList. + version = std::max(version, 45u); + auto argType{GetArgumentListItemKind(clauseId, version)}; + if (!argType) { + return; + } + switch (*argType) { + case ListItemKind::Extended: + case ListItemKind::Locator: + case ListItemKind::Variable: + break; + default: + return; + } + + // Corner cases: + if (clauseId == llvm::omp::Clause::OMPC_to) { + // 4.5 5+ + // TO (declare target) extended extended + // TO (target update) variable locator + if (dirId == llvm::omp::Directive::OMPD_declare_target) { + argType = ListItemKind::Extended; + } else { + assert(dirId == llvm::omp::Directive::OMPD_target_update && + "Unexpected directive"); + argType = version < 50 ? ListItemKind::Variable : ListItemKind::Locator; + } + } else if (clauseId == llvm::omp::Clause::OMPC_uniform) { + // 4.5 5+ + // UNIFORM variable parameter + // The uniform clause takes std::list<Name> at the moment and cannot + // be verified here. + return; + } else if (clauseId == llvm::omp::Clause::OMPC_depend) { + // 6.0- 6.1+ + // DEPEND locator/doacross depend + if (version >= 61) { + return; + } + // The DEPEND clause with SINK/SOURCE will not have an object list. + if (auto *depend{parser::Unwrap<parser::OmpDependClause>(x)}) { + if (parser::Unwrap<parser::OmpDoacross>(*depend)) { + return; + } + } + } + + // Named constants are OK to be used within 'shared' and 'firstprivate' + // clauses. The check for this happens a few lines below. + bool NamedConstantAllowed{false}; + switch (clauseId) { + case llvm::omp::Clause::OMPC_shared: + case llvm::omp::Clause::OMPC_firstprivate: + NamedConstantAllowed = true; + break; + default: + break; + } + + for (auto &object : DEREF(GetOmpObjectList(x)).v) { + AnalyzeObject(object); + // substring + const Symbol *symbol{GetObjectSymbol(object, /*ultimate=*/true)}; + if (symbol == nullptr) { + // This may happen with a blank common block. Skip these cases. + continue; + } + auto source{*parser::omp::GetObjectSource(object)}; + if (NamedConstantAllowed && IsNamedConstant(*symbol)) { + continue; + } + // Emit a more user-friendly diagnostic than "'kind' must be a variable + // list item" for x%kind, or for cplx%re. + if (IsTypeParamInquiry(*symbol)) { + context_.Say(source, + "Type parameter inquiry is not allowed as a list item on %s clause"_err_en_US, + parser::omp::GetUpperName(clauseId, version)); + continue; + } + if (IsComplexPart(*symbol)) { + // We have been tolerating complex part designators. + continue; + } + + const char *neededType{nullptr}; + + switch (*argType) { + case ListItemKind::Extended: + if (!IsExtendedListItem(object, &context_)) { + neededType = "an extended"; + } + break; + case ListItemKind::Locator: + if (!IsLocatorListItem(object, &context_)) { + neededType = "a locator"; + } + break; + case ListItemKind::Variable: + if (!IsVariableListItem(object, &context_)) { + neededType = "a variable"; + } + break; + default: + break; + } + + if (neededType) { + context_.SayWithDecl(*symbol, source, + "'%s' must be %s list item"_err_en_US, symbol->name(), neededType); } } } @@ -2385,7 +2487,6 @@ void OmpStructureChecker::Leave(const parser::OmpDeclareTargetDirective &x) { std::is_same_v<TypeC, parser::OmpClause::To>) { auto &objList{*GetOmpObjectList(c)}; CheckSymbolNames(dirName.source, objList); - CheckTypeParamInquiry(dirName.source, objList); CheckVarIsNotPartOfAnotherVar(dirName.source, objList); CheckThreadprivateOrDeclareTargetVar(objList); } @@ -3627,44 +3728,7 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &x) { void OmpStructureChecker::Enter(const parser::OmpClause &x) { SetContextClause(x); - - llvm::omp::Clause id{x.Id()}; - // The visitors for these clauses do their own checks. - switch (id) { - case llvm::omp::Clause::OMPC_copyprivate: - case llvm::omp::Clause::OMPC_enter: - case llvm::omp::Clause::OMPC_lastprivate: - case llvm::omp::Clause::OMPC_reduction: - case llvm::omp::Clause::OMPC_to: - return; - default: - break; - } - - // Named constants are OK to be used within 'shared' and 'firstprivate' - // clauses. The check for this happens a few lines below. - bool SharedOrFirstprivate = false; - switch (id) { - case llvm::omp::Clause::OMPC_shared: - case llvm::omp::Clause::OMPC_firstprivate: - SharedOrFirstprivate = true; - break; - default: - break; - } - - if (const parser::OmpObjectList *objList{GetOmpObjectList(x)}) { - AnalyzeObjects(*objList); - SymbolSourceMap symbols; - GetSymbolsInObjectList(*objList, symbols); - for (const auto &[symbol, source] : symbols) { - if (!IsVariableListItem(*symbol) && - !(IsNamedConstant(*symbol) && SharedOrFirstprivate)) { - context_.SayWithDecl(*symbol, source, - "'%s' must be a variable"_err_en_US, symbol->name()); - } - } - } + CheckArgumentObjectKind(x); } // Restrictions specific to each clause are implemented apart from the @@ -3884,17 +3948,6 @@ void OmpStructureChecker::CheckReductionObjects( } } } - // Type parameter inquiries are not allowed. - for (const parser::OmpObject &object : objects.v) { - if (auto *symbol{GetObjectSymbol(object)}) { - if (IsTypeParamInquiry(*symbol)) { - auto source{GetObjectSource(object)}; - context_.Say(source ? *source : GetContext().clauseSource, - "Type parameter inquiry is not permitted in %s clause"_err_en_US, - parser::omp::GetUpperName(clauseId, version)); - } - } - } } } @@ -4176,15 +4229,14 @@ void OmpStructureChecker::CheckSharedBindingInOuterContext( void OmpStructureChecker::Enter(const parser::OmpClause::Shared &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_shared); - CheckTypeParamInquiry(GetContext().clauseSource, x.v); CheckVarIsNotPartOfAnotherVar(GetContext().clauseSource, x.v, "SHARED"); CheckCrayPointee(x.v, "SHARED"); } + void OmpStructureChecker::Enter(const parser::OmpClause::Private &x) { SymbolSourceMap symbols; GetSymbolsInObjectList(x.v, symbols); CheckAllowedClause(llvm::omp::Clause::OMPC_private); - CheckTypeParamInquiry(GetContext().clauseSource, x.v); CheckVarIsNotPartOfAnotherVar(GetContext().clauseSource, x.v, "PRIVATE"); CheckIntentInPointer(symbols, llvm::omp::Clause::OMPC_private); CheckCrayPointee(x.v, "PRIVATE"); @@ -4233,7 +4285,8 @@ void OmpStructureChecker::CheckVarIsNotPartOfAnotherVar( } if (report || parser::Unwrap<parser::ArrayElement>(object)) { - if (llvm::omp::nonPartialVarSet.test(GetContext().directive)) { + if (clause.empty() && + llvm::omp::nonPartialVarSet.test(GetContext().directive)) { context_.Say(source, "A variable that is part of another variable (as an array or structure element) cannot appear on the %s directive"_err_en_US, ContextDirectiveAsFortran()); @@ -4248,7 +4301,6 @@ void OmpStructureChecker::CheckVarIsNotPartOfAnotherVar( void OmpStructureChecker::Enter(const parser::OmpClause::Firstprivate &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_firstprivate); - CheckTypeParamInquiry(GetContext().clauseSource, x.v); CheckVarIsNotPartOfAnotherVar(GetContext().clauseSource, x.v, "FIRSTPRIVATE"); CheckCrayPointee(x.v, "FIRSTPRIVATE"); @@ -4453,7 +4505,6 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Detach &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_detach); } - CheckTypeParamInquiry(GetContext().clauseSource, x.v.v); // OpenMP 5.2: 12.5.2 Detach clause restrictions if (version >= 52) { CheckVarIsNotPartOfAnotherVar(GetContext().clauseSource, x.v.v, "DETACH"); @@ -4909,7 +4960,6 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Copyprivate &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_copyprivate); SymbolSourceMap symbols; GetSymbolsInObjectList(x.v, symbols); - CheckVariableListItem(symbols); CheckIntentInPointer(symbols, llvm::omp::Clause::OMPC_copyprivate); CheckCopyingPolymorphicAllocatable( symbols, llvm::omp::Clause::OMPC_copyprivate); @@ -4919,7 +4969,6 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_lastprivate); const auto &objectList{*GetOmpObjectList(x)}; - CheckTypeParamInquiry(GetContext().clauseSource, objectList); CheckVarIsNotPartOfAnotherVar( GetContext().clauseSource, objectList, "LASTPRIVATE"); CheckCrayPointee(objectList, "LASTPRIVATE"); @@ -5161,18 +5210,8 @@ void OmpStructureChecker::Enter(const parser::OmpClause::HasDeviceAddr &x) { void OmpStructureChecker::Enter(const parser::OmpClause::Enter &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_enter); - if (!OmpVerifyModifiers( - x.v, llvm::omp::OMPC_enter, GetContext().clauseSource, context_)) { - return; - } - SymbolSourceMap symbols; - GetSymbolsInObjectList(*GetOmpObjectList(x), symbols); - for (const auto &[symbol, source] : symbols) { - if (!IsExtendedListItem(*symbol)) { - context_.SayWithDecl(*symbol, source, - "'%s' must be a variable or a procedure"_err_en_US, symbol->name()); - } - } + OmpVerifyModifiers( + x.v, llvm::omp::OMPC_enter, GetContext().clauseSource, context_); } void OmpStructureChecker::Enter(const parser::OmpClause::From &x) { @@ -5190,10 +5229,6 @@ void OmpStructureChecker::Enter(const parser::OmpClause::From &x) { } const auto &objList{*GetOmpObjectList(x)}; - SymbolSourceMap symbols; - GetSymbolsInObjectList(objList, symbols); - CheckVariableListItem(symbols); - // Ref: [4.5:109:19] // If a list item is an array section it must specify contiguous storage. if (version <= 45) { @@ -5230,10 +5265,6 @@ void OmpStructureChecker::Enter(const parser::OmpClause::To &x) { } const auto &objList{*GetOmpObjectList(x)}; - SymbolSourceMap symbols; - GetSymbolsInObjectList(objList, symbols); - CheckVariableListItem(symbols); - // Ref: [4.5:109:19] // If a list item is an array section it must specify contiguous storage. if (version <= 45) { @@ -5467,6 +5498,9 @@ void OmpStructureChecker::CheckDefinableObjects( SymbolSourceMap &symbols, const llvm::omp::Clause clause) { unsigned version{context_.langOptions().OpenMPVersion}; for (auto &[symbol, source] : symbols) { + if (!IsVariableListItem(*symbol)) { + continue; + } if (auto msg{WhyNotDefinable(source, context_.FindScope(source), DefinabilityFlags{}, *symbol)}) { context_ diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h index 8a85f489eeb4c..f1f210e047a38 100644 --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -289,7 +289,7 @@ class OmpStructureChecker : public OmpStructureCheckerBase { // check-omp-structure.cpp bool IsAllowedClause(llvm::omp::Clause clauseId); bool CheckAllowedClause(llvm::omp::Clause clause); - void CheckVariableListItem(const SymbolSourceMap &symbols); + void CheckArgumentObjectKind(const parser::OmpClause &x); void CheckDirectiveSpelling( parser::CharBlock spelling, llvm::omp::Directive id); void CheckDirectiveDeprecation(const parser::OpenMPConstruct &x); diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp index 51dd08d0924b1..d2f26b70c60ae 100644 --- a/flang/lib/Semantics/openmp-utils.cpp +++ b/flang/lib/Semantics/openmp-utils.cpp @@ -154,7 +154,8 @@ bool IsCommonBlock(const Symbol &sym) { } bool IsVariableListItem(const Symbol &sym) { - return evaluate::IsVariable(sym) || sym.attrs().test(Attr::POINTER); + return evaluate::IsVariable(sym) || IsCommonBlock(sym) || + sym.attrs().test(Attr::POINTER); } bool IsExtendedListItem(const Symbol &sym) { @@ -174,6 +175,14 @@ bool IsTypeParamInquiry(const Symbol &sym) { sym.details()); } +bool IsComplexPart(const Symbol &sym) { + if (auto *misc{sym.detailsIf<MiscDetails>()}) { + return misc->kind() == MiscDetails::Kind::ComplexPartRe || + misc->kind() == MiscDetails::Kind::ComplexPartIm; + } + return false; +} + bool IsStructureComponent(const Symbol &sym) { return sym.owner().kind() == Scope::Kind::DerivedType; } @@ -217,6 +226,38 @@ bool IsWholeAssumedSizeArray(const parser::OmpObject &object) { return false; } +bool IsExtendedListItem( + const parser::OmpObject &object, SemanticsContext *semaCtx) { + if (IsVariableListItem(object, semaCtx)) { + return true; + } + if (auto *sym{GetObjectSymbol(object, /*ultimate=*/true)}) { + return IsProcedure(*sym); + } + return false; +} + +bool IsLocatorListItem( + const parser::OmpObject &object, SemanticsContext *semaCtx) { + if (IsVariableListItem(object, semaCtx)) { + return true; + } + if (auto *desg{parser::Unwrap<parser::Designator>(object)}) { + evaluate::ExpressionAnalyzer ea(*semaCtx); + auto restorer{ea.GetContextualMessages().DiscardMessages()}; + return IsVarOrFunctionRef(ea.Analyze(*desg)); + } + return false; +} + +bool IsVariableListItem( + const parser::OmpObject &object, SemanticsContext *semaCtx) { + if (auto *sym{GetObjectSymbol(object, /*ultimate=*/true)}) { + return IsVariableListItem(*sym); + } + return false; +} + const Symbol *GetHostSymbol(const Symbol &sym) { if (auto *details{sym.detailsIf<HostAssocDetails>()}) { return &details->symbol(); @@ -554,6 +595,188 @@ MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp) { instance.u); } +/// For clauses that take argument lists, return the type of the argument +/// list item. For other clauses return std::nullopt. +std::optional<ListItemKind> GetArgumentListItemKind( + llvm::omp::Clause clause, unsigned version) { + switch (clause) { + case llvm::omp::Clause::OMPC_absent: + if (version >= 51) { + return ListItemKind::DirectiveName; + } + break; + case llvm::omp::Clause::OMPC_adjust_args: + if (version >= 61) { + return ListItemKind::ProcedureArgument; + } + if (version >= 51) { + return ListItemKind::Parameter; + } + break; + case llvm::omp::Clause::OMPC_affinity: + if (version >= 50) { + return ListItemKind::Locator; + } + break; + case llvm::omp::Clause::OMPC_aligned: + return ListItemKind::Variable; + case llvm::omp::Clause::OMPC_allocate: + if (version >= 50) { + return ListItemKind::Variable; + } + break; + case llvm::omp::Clause::OMPC_append_args: + if (version >= 51) { + return ListItemKind::Operation; + } + break; + case llvm::omp::Clause::OMPC_apply: + if (version >= 60) { + return ListItemKind::DirectiveSpecification; + } + break; + case llvm::omp::Clause::OMPC_contains: + if (version >= 51) { + return ListItemKind::DirectiveName; + } + break; + case llvm::omp::Clause::OMPC_copyin: + return ListItemKind::Variable; + case llvm::omp::Clause::OMPC_copyprivate: + return ListItemKind::Variable; + case llvm::omp::Clause::OMPC_counts: + if (version >= 60) { + return ListItemKind::IntegerExpression; + } + break; + case llvm::omp::Clause::OMPC_depend: + if (version >= 61) { + return ListItemKind::Depend; + } + if (version >= 50) { + return ListItemKind::Locator; + } + return ListItemKind::Variable; + case llvm::omp::Clause::OMPC_enter: + if (version >= 52) { + return ListItemKind::Extended; + } + break; + case llvm::omp::Clause::OMPC_exclusive: + if (version >= 50) { + return ListItemKind::Variable; + } + break; + case llvm::omp::Clause::OMPC_firstprivate: + return ListItemKind::Variable; + case llvm::omp::Clause::OMPC_from: + if (version >= 50) { + return ListItemKind::Locator; + } + return ListItemKind::Variable; + case llvm::omp::Clause::OMPC_has_device_addr: + if (version >= 51) { + return ListItemKind::Variable; + } + break; + case llvm::omp::Clause::OMPC_in_reduction: + if (version >= 50) { + return ListItemKind::Variable; + } + break; + case llvm::omp::Clause::OMPC_inclusive: + if (version >= 50) { + return ListItemKind::Variable; + } + break; + case llvm::omp::Clause::OMPC_induction: + if (version >= 60) { + return ListItemKind::Variable; + } + break; + case llvm::omp::Clause::OMPC_interop: + if (version >= 60) { + return ListItemKind::Interop; + } + break; + case llvm::omp::Clause::OMPC_is_device_ptr: + return ListItemKind::Variable; + case llvm::omp::Clause::OMPC_lastprivate: + return ListItemKind::Variable; + case llvm::omp::Clause::OMPC_linear: + return ListItemKind::Variable; + case llvm::omp::Clause::OMPC_link: + return ListItemKind::Variable; + case llvm::omp::Clause::OMPC_local: + if (version >= 60) { + return ListItemKind::Variable; + } + brea... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/201334 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
