Skip to content

[Flang][OpenMP] Add semantic checks for Worshare construct #111358

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
Thirumalai-Shaktivel merged 3 commits into from
Oct 18, 2024
Merged

[Flang][OpenMP] Add semantic checks for Worshare construct #111358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
554361b
[LLVM-Flang][OpenMP] Add a semantic check for Worshare construct
Thirumalai-Shaktivel Oct 7, 2024
b1400b1
[Flang][OpenMP] Allow Nowait clause only once in the workshare construct
Thirumalai-Shaktivel Oct 7, 2024
82b77c3
[Test] Add tests
Thirumalai-Shaktivel Oct 7, 2024
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
22 changes: 17 additions & 5 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 @@ -68,11 +68,23 @@ class OmpWorkshareBlockChecker {
if (const auto *e{GetExpr(context_, expr)}) {
for (const Symbol &symbol : evaluate::CollectSymbols(*e)) {
const Symbol &root{GetAssociationRoot(symbol)};
if (IsFunction(root) && !IsElementalProcedure(root)) {
context_.Say(expr.source,
"User defined non-ELEMENTAL function "
"'%s' is not allowed in a WORKSHARE construct"_err_en_US,
root.name());
if (IsFunction(root)) {
std::string attrs{""};
if (!IsElementalProcedure(root)) {
attrs = " non-ELEMENTAL";
}
if (root.attrs().test(Attr::IMPURE)) {
if (attrs != "") {
attrs = "," + attrs;
}
attrs = " IMPURE" + attrs;
}
if (attrs != "") {
context_.Say(expr.source,
"User defined%s function '%s' is not allowed in a "
"WORKSHARE construct"_err_en_US,
attrs, root.name());
}
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions flang/test/Semantics/OpenMP/workshare02.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ module my_mod
integer function my_func()
my_func = 10
end function my_func

impure integer function impure_my_func()
impure_my_func = 20
end function impure_my_func

impure elemental integer function impure_ele_my_func()
impure_ele_my_func = 20
end function impure_ele_my_func
end module my_mod

subroutine workshare(aa, bb, cc, dd, ee, ff, n)
Expand Down Expand Up @@ -61,6 +69,16 @@ subroutine workshare(aa, bb, cc, dd, ee, ff, n)
j = j - my_func()
!$omp end atomic

!ERROR: User defined IMPURE, non-ELEMENTAL function 'impure_my_func' is not allowed in a WORKSHARE construct
cc = impure_my_func()
!ERROR: User defined IMPURE function 'impure_ele_my_func' is not allowed in a WORKSHARE construct
aa(1) = impure_ele_my_func()

!$omp end workshare

!$omp workshare
j = j + 1
!ERROR: At most one NOWAIT clause can appear on the END WORKSHARE directive
!$omp end workshare nowait nowait

end subroutine workshare
2 changes: 1 addition & 1 deletion llvm/include/llvm/Frontend/OpenMP/OMP.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1170,7 +1170,7 @@ def OMP_Workshare : Directive<"workshare"> {
let category = CA_Executable;
}
def OMP_EndWorkshare : Directive<"end workshare"> {
let allowedClauses = [
let allowedOnceClauses = [
Copy link
Contributor

@tblah tblah Oct 9, 2024

Choose a reason for hiding this comment

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

nit: please could you add a test for this check as well. Something in flang/test/ is okay

Otherwise this LGTM

Copy link
Member Author

@Thirumalai-Shaktivel Thirumalai-Shaktivel Oct 9, 2024

Choose a reason for hiding this comment

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

I have added a test here: https://github.com/llvm/llvm-project/pull/111358/files#diff-3b5218aa589f253f6fe3c0637625dfe6c38d9dc425d5290a626b2b6796f06db5R79-R82. Is it fine or should I move it to flang/test

Copy link
Contributor

@tblah tblah Oct 9, 2024

Choose a reason for hiding this comment

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

Ahh my mistake

Copy link
Member Author

@Thirumalai-Shaktivel Thirumalai-Shaktivel Oct 9, 2024

Choose a reason for hiding this comment

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

No issues, @tblah. Thanks for the review!

VersionedClause<OMPC_NoWait>,
];
let leafConstructs = OMP_Workshare.leafConstructs;
Expand Down