Skip to content

[Flang OpenMP] Add semantics checks for cray pointer usage in DSA list #121028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions flang/lib/Semantics/check-omp-structure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3340,13 +3340,15 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Ordered &x) {
void OmpStructureChecker::Enter(const parser::OmpClause::Shared &x) {
CheckAllowedClause(llvm::omp::Clause::OMPC_shared);
CheckIsVarPartOfAnotherVar(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);
CheckIsVarPartOfAnotherVar(GetContext().clauseSource, x.v, "PRIVATE");
CheckIntentInPointer(symbols, llvm::omp::Clause::OMPC_private);
CheckCrayPointee(x.v, "PRIVATE");
}

void OmpStructureChecker::Enter(const parser::OmpClause::Nowait &x) {
Expand Down Expand Up @@ -3426,6 +3428,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Firstprivate &x) {
CheckAllowedClause(llvm::omp::Clause::OMPC_firstprivate);

CheckIsVarPartOfAnotherVar(GetContext().clauseSource, x.v, "FIRSTPRIVATE");
CheckCrayPointee(x.v, "FIRSTPRIVATE");
CheckIsLoopIvPartOfClause(llvmOmpClause::OMPC_firstprivate, x.v);

SymbolSourceMap currSymbols;
Expand Down Expand Up @@ -4522,6 +4525,22 @@ void OmpStructureChecker::CheckProcedurePointer(
}
}

void OmpStructureChecker::CheckCrayPointee(
const parser::OmpObjectList &objectList, llvm::StringRef clause) {
SymbolSourceMap symbols;
GetSymbolsInObjectList(objectList, symbols);
for (auto it{symbols.begin()}; it != symbols.end(); ++it) {
const auto *symbol{it->first};
const auto source{it->second};
if (symbol->test(Symbol::Flag::CrayPointee)) {
context_.Say(source,
"Cray Pointee '%s' may not appear in %s clause, use Cray Pointer '%s' instead"_err_en_US,
symbol->name(), clause.str(),
semantics::GetCrayPointer(*symbol).name());
Comment on lines +4537 to +4539
Copy link
Contributor

Choose a reason for hiding this comment

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

@mjklemm is this a correct interpretation of the standard? It isn't clear to me if naming cray pointees is actually prohibited, it only seems to require that the data-sharing attribute is the same as the pointer.

Of course, they are deprecated so we could disallow on those grounds (as we did with - reductions).

Copy link
Contributor

Choose a reason for hiding this comment

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

The OpenMP specification is ambiguous on this. Let's do it this way and see if application users will complain about this. :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

is the same as the pointer.

is the same as the target of the pointer.

}
}
}

void OmpStructureChecker::GetSymbolsInObjectList(
const parser::OmpObjectList &objectList, SymbolSourceMap &symbols) {
for (const auto &ompObject : objectList.v) {
Expand Down
2 changes: 2 additions & 0 deletions flang/lib/Semantics/check-omp-structure.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ class OmpStructureChecker
const parser::CharBlock &source, const parser::OmpObjectList &objList);
void CheckIntentInPointer(SymbolSourceMap &, const llvm::omp::Clause);
void CheckProcedurePointer(SymbolSourceMap &, const llvm::omp::Clause);
void CheckCrayPointee(
const parser::OmpObjectList &objectList, llvm::StringRef clause);
void GetSymbolsInObjectList(const parser::OmpObjectList &, SymbolSourceMap &);
void CheckDefinableObjects(SymbolSourceMap &, const llvm::omp::Clause);
void CheckCopyingPolymorphicAllocatable(
Expand Down
24 changes: 18 additions & 6 deletions flang/lib/Semantics/resolve-directives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2115,8 +2115,12 @@ void OmpAttributeVisitor::Post(const parser::OpenMPAllocatorsConstruct &x) {
static bool IsPrivatizable(const Symbol *sym) {
auto *misc{sym->detailsIf<MiscDetails>()};
return IsVariableName(*sym) && !IsProcedure(*sym) && !IsNamedConstant(*sym) &&
!semantics::IsAssumedSizeArray(
*sym) && /* OpenMP 5.2, 5.1.1: Assumed-size arrays are shared*/
(!semantics::IsAssumedSizeArray(
*sym) || /* OpenMP 5.2, 5.1.1: Assumed-size arrays are shared*/
(sym->test(Symbol::Flag::CrayPointee) &&
// If CrayPointer is among the DSA list then the
// CrayPointee is Privatizable
&semantics::GetCrayPointer(*sym))) &&
!sym->owner().IsDerivedType() &&
sym->owner().kind() != Scope::Kind::ImpliedDos &&
!sym->detailsIf<semantics::AssocEntityDetails>() &&
Expand Down Expand Up @@ -2282,10 +2286,18 @@ void OmpAttributeVisitor::Post(const parser::Name &name) {
// the scope of the parallel region, and not in this scope.
// TODO: check whether this should be caught in IsObjectWithDSA
!symbol->test(Symbol::Flag::OmpPrivate)) {
context_.Say(name.source,
"The DEFAULT(NONE) clause requires that '%s' must be listed in "
"a data-sharing attribute clause"_err_en_US,
symbol->name());
if (symbol->test(Symbol::Flag::CrayPointee)) {
std::string crayPtrName{
semantics::GetCrayPointer(*symbol).name().ToString()};
if (!IsObjectWithDSA(*currScope().FindSymbol(crayPtrName)))
context_.Say(name.source,
"The DEFAULT(NONE) clause requires that the Cray Pointer '%s' must be listed in a data-sharing attribute clause"_err_en_US,
crayPtrName);
} else {
context_.Say(name.source,
"The DEFAULT(NONE) clause requires that '%s' must be listed in a data-sharing attribute clause"_err_en_US,
symbol->name());
}
}
}
}
Expand Down
27 changes: 27 additions & 0 deletions flang/test/Semantics/OpenMP/cray-pointer-usage.f90
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
!RUN: %python %S/../test_errors.py %s %flang -fopenmp
subroutine test_cray_pointer_usage
implicit none
real(8) :: var(*), pointee(2)
pointer(ivar, var)

pointee = 42.0
ivar = loc(pointee)

!$omp parallel num_threads(2) default(none)
! ERROR: The DEFAULT(NONE) clause requires that the Cray Pointer 'ivar' must be listed in a data-sharing attribute clause
print *, var(1)
!$omp end parallel

! ERROR: Cray Pointee 'var' may not appear in PRIVATE clause, use Cray Pointer 'ivar' instead
!$omp parallel num_threads(2) default(none) private(var)
print *, var(1)
!$omp end parallel

!$omp parallel num_threads(2) default(none) firstprivate(ivar)
print *, var(1)
!$omp end parallel

!$omp parallel num_threads(2) default(private) shared(ivar)
print *, var(1)
!$omp end parallel
end subroutine test_cray_pointer_usage
Loading