Skip to content

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

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.

Sign up for GitHub

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 4 commits into from
Jan 28, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9425e5d
[Flang OpenMP] Add semantics checks for cray pointer usage in DSA list
Thirumalai-Shaktivel Dec 24, 2024
538a41c
[Flang] Remove redundant check for CrayPointer
Thirumalai-Shaktivel Jan 16, 2025
1142fdc
[Flang] Throw errors for other DSA clauses based on OpenMP 5.0
Thirumalai-Shaktivel Jan 27, 2025
e9e799a
Merge branch 'main' into llvm/cray_ptr_02
Thirumalai-Shaktivel Jan 27, 2025
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
25 changes: 25 additions & 0 deletions flang/lib/Semantics/check-omp-structure.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3438,13 +3438,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 @@ -3524,6 +3526,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 @@ -3758,6 +3761,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Linear &x) {

SymbolSourceMap symbols;
auto &objects{std::get<parser::OmpObjectList>(x.v.t)};
CheckCrayPointee(objects, "LINEAR", false);
GetSymbolsInObjectList(objects, symbols);

auto CheckIntegerNoRef{[&](const Symbol *symbol, parser::CharBlock source) {
Expand Down Expand Up @@ -4203,6 +4207,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) {
const auto &objectList{std::get<parser::OmpObjectList>(x.v.t)};
CheckIsVarPartOfAnotherVar(
GetContext().clauseSource, objectList, "LASTPRIVATE");
CheckCrayPointee(objectList, "LASTPRIVATE");

DirectivesClauseTriple dirClauseTriple;
SymbolSourceMap currSymbols;
Expand Down Expand Up @@ -4620,6 +4625,26 @@ void OmpStructureChecker::CheckProcedurePointer(
}
}

void OmpStructureChecker::CheckCrayPointee(
const parser::OmpObjectList &objectList, llvm::StringRef clause,
bool suggestToUseCrayPointer) {
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)) {
std::string suggestionMsg = "";
if (suggestToUseCrayPointer)
suggestionMsg = ", use Cray Pointer '" +
semantics::GetCrayPointer(*symbol).name().ToString() + "' instead";
context_.Say(source,
"Cray Pointee '%s' may not appear in %s clause%s"_err_en_US,
symbol->name(), clause.str(), suggestionMsg);
}
}
}

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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,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, bool suggestToUseCrayPointer = true);
void GetSymbolsInObjectList(const parser::OmpObjectList &, SymbolSourceMap &);
void CheckDefinableObjects(SymbolSourceMap &, const llvm::omp::Clause);
void CheckCopyingPolymorphicAllocatable(
Expand Down
23 changes: 17 additions & 6 deletions flang/lib/Semantics/resolve-directives.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2105,8 +2105,11 @@ 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*/
( // OpenMP 5.2, 5.1.1: Assumed-size arrays are shared
!semantics::IsAssumedSizeArray(*sym) ||
// If CrayPointer is among the DSA list then the
// CrayPointee is Privatizable
sym->test(Symbol::Flag::CrayPointee)) &&
Copy link
Member Author

Choose a reason for hiding this comment

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

Here, would checking only for CrayPointee be right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Was the compilation failure due to the use of & in &semantics::GetCrayPointer(*sym))?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. It fails in flang-aarch64-libcxx and It says the following:

[...]
FAILED: tools/flang/lib/Semantics/CMakeFiles/FortranSemantics.dir/resolve-directives.cpp.o 
/usr/local/bin/c++ -DFLANG_INCLUDE_TESTS=1 -DFLANG_LITTLE_ENDIAN=1 -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/flang/lib/Semantics -I/home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/lib/Semantics -I/home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/include -I/home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/flang/include -I/home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/include -I/home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/llvm/include -isystem /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/llvm/../mlir/include -isystem /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/mlir/include -isystem /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/clang/include -isystem /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/llvm/../clang/include -stdlib=libc++ -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Werror -Wno-deprecated-copy -Wno-string-conversion -Wno-ctad-maybe-unsupported -Wno-unused-command-line-argument -Wstring-conversion           -Wcovered-switch-default -Wno-nested-anon-types -O3 -DNDEBUG -std=c++17 -fPIC  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT tools/flang/lib/Semantics/CMakeFiles/FortranSemantics.dir/resolve-directives.cpp.o -MF tools/flang/lib/Semantics/CMakeFiles/FortranSemantics.dir/resolve-directives.cpp.o.d -o tools/flang/lib/Semantics/CMakeFiles/FortranSemantics.dir/resolve-directives.cpp.o -c /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/lib/Semantics/resolve-directives.cpp
../llvm-project/flang/lib/Semantics/resolve-directives.cpp:2123:16: error: reference cannot be bound to dereferenced null pointer in well-defined C++ code; pointer may be assumed to always convert to true [-Werror,-Wundefined-bool-conversion]
 2120 |           (sym->test(Symbol::Flag::CrayPointee) &&
      |                                                 ~~
 2121 |               // If CrayPointer is among the DSA list then the
 2122 |               // CrayPointee is Privatizable
 2123 |               &semantics::GetCrayPointer(*sym))) &&
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../llvm-project/flang/include/flang/Semantics/tools.h:315:15: note: 'GetCrayPointer' returns a reference
  315 | const Symbol &GetCrayPointer(const Symbol &crayPointee);
      |               ^
1 error generated.

!sym->owner().IsDerivedType() &&
sym->owner().kind() != Scope::Kind::ImpliedDos &&
sym->owner().kind() != Scope::Kind::Forall &&
Expand Down Expand Up @@ -2273,10 +2276,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
44 changes: 44 additions & 0 deletions flang/test/Semantics/OpenMP/cray-pointer-usage.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
!RUN: %python %S/../test_errors.py %s %flang -fopenmp
subroutine test_cray_pointer_usage
implicit none
integer :: i
real(8) :: var(*), pointee(2)
pointer(ivar, var)
! ERROR: Cray Pointee 'var' may not appear in LINEAR clause
! ERROR: The list item 'var' specified without the REF 'linear-modifier' must be of INTEGER type
! ERROR: The list item `var` must be a dummy argument
!$omp declare simd linear(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

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

! ERROR: Cray Pointee 'var' may not appear in LASTPRIVATE clause, use Cray Pointer 'ivar' instead
!$omp do lastprivate(var)
do i = 1, 10
print *, var(1)
end do
!$omp end do

!$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