[flang][OpenMP] More detailed checks for argument list items in clauses#201334
[flang][OpenMP] More detailed checks for argument list items in clauses#201334kparzysz wants to merge 7 commits into
Conversation
This will make it possible to diagnose these situations independently. This isn't perfect, but will be improved gradually in the future.
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.
|
@llvm/pr-subscribers-flang-openmp Author: Krzysztof Parzyszek (kparzysz) ChangesFor 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:
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]
|
|
@llvm/pr-subscribers-flang-semantics Author: Krzysztof Parzyszek (kparzysz) ChangesFor 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:
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]
|
|
FYI, I'll review this tomorrow morning, but you'll still probably want an area expert to review. |
There was a problem hiding this comment.
AnalyzeObjects is still declared here but the definition was deleted.
| switch (*argType) { | ||
| case ListItemKind::Extended: | ||
| if (!IsExtendedListItem(object, &context_)) { | ||
| neededType = "an extended"; |
There was a problem hiding this comment.
There is no test for this error. I recommend adding a test case.
There was a problem hiding this comment.
Currently we can't have a function reference in an OmpObject, which would be necessary for making an object that is not an extended list item. I'm working on that now, but as it stands now we can't test this particular case.
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.