Skip to content

[Flang][OpenMP][Semantics] Modify errors to warnings for semantic checks in IS_DEVICE_PTR related to list-items being dummy arguments. #74370

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 1 commit into from
Dec 8, 2023
Merged

[Flang][OpenMP][Semantics] Modify errors to warnings for semantic checks in IS_DEVICE_PTR related to list-items being dummy arguments. #74370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 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 @@ -2996,11 +2996,14 @@ void OmpStructureChecker::Enter(const parser::OmpClause::IsDevicePtr &x) {
source.ToString());
} else if (!(IsDummy(*symbol))) {
context_.Say(itr->second->source,
"Variable '%s' in IS_DEVICE_PTR clause must be a dummy argument"_err_en_US,
"Variable '%s' in IS_DEVICE_PTR clause must be a dummy argument. "
"This semantic check is deprecated from OpenMP 5.2 and later."_warn_en_US,
source.ToString());
} else if (IsAllocatableOrPointer(*symbol) || IsValue(*symbol)) {
context_.Say(itr->second->source,
"Variable '%s' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute."_err_en_US,
"Variable '%s' in IS_DEVICE_PTR clause must be a dummy argument "
"that does not have the ALLOCATABLE, POINTER or VALUE attribute. "
"This semantic check is deprecated from OpenMP 5.2 and later."_warn_en_US,
source.ToString());
}
}
Expand Down
8 changes: 4 additions & 4 deletions flang/test/Semantics/OpenMP/target01.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,19 @@ subroutine bar(b1, b2, b3)
type(c_ptr), pointer :: b2
type(c_ptr), value :: b3

!ERROR: Variable 'c' in IS_DEVICE_PTR clause must be a dummy argument
Copy link
Member

Choose a reason for hiding this comment

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

According to the restriction, this error should be reported right? It says argument must be a dummy argument. Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this restriction is something on the lines of "If the list-item in an is_device_ptr clause is a dummy argument it should not have ALLOCATABLE, POINTER or VALUE attributes."

I think this following text might not have completely meant that all list-items must be dummy arguments only I think the conjunction of "does not have the ALLOCATABLE, POINTER or VALUE attribute" might be the reason for the "must".
A list item that appears in an is_device_ptr clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute.

Also, this restriction is not present from OpenMP 5.1 onwards through which I assumed it might not be originally intended for OpenMP spec committee might not have intended to restricted list-item of is_device_ptr clause to be dummy argument only.

Please correct me if I am interpreting it incorrectly. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

So it seems like the type of list item became more restrictive as the standard progressed.

OpenMP 4.5 standard (2.10.4): A list item that appears in an is_device_ptr clause must be a dummy argument

OpenMP 5.0 standard (2.12.5): A list item that appears in an is_device_ptr clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute.

However, my understanding of both these statements is leaning similar to @shraiysh's- any list item in this clause has to be a dummy argument, but without any additional attributes from ALLOCATABLE, POINTER, VALUE. In other words, I don't think non-dummy arguments are allowed in this clause. This becomes clear when we look at the OpenMP 4.5 standard wording: it is, without a doubt, restricting all list items within the clause to be dummy arguments.

Copy link
Member

@shraiysh shraiysh Dec 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Like Raghu mentioned, I cannot find this check, or anything related to the c_ptr being a dummy argument in the OpenMP 5.2 spec. If this check is not required in 5.2, we probably should remove this check completely - please let me know if there is anything related to this in 5.2. As discussed in the meeting, OpenMP 5.2 is essentially a fix on OpenMP 5.1 and OpenMP 5.0, so we should stick to OpenMP 5.2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I asked @mjklemm who is CEO of OpenMP ARB

Here are his comments:

@mjklemm : 5.0 is broken. 5.1/5.2 has fixed it. OpenMP 6.0 will have a different behavior. I wonder if it would make sense to directly implement the 5.2 and 6.0 behavior.

This aligns with @shraiysh 's comment. Even I could not find anything related to dummy argument list item of IS_DEVICE_PTR clause. So, I will remove the dummy argument check.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think in 5.2 we deprecate all the other use of the is_device_ptr clause in Fortran except type(c_ptr) variables in order to avoid any confusion between pointer and nonpointer list item. The has_device_address clause should be the choice for Fortran unless it is a type(c_ptr) variable.

In Section 5.4.7, we have:

If the is_device_ptr clause is specified on a target construct, if any list item is not of type C_PTR, the behavior is as if the list item appeared in a has_device_addr clause. Support for such list items in an is_device_ptr clause is deprecated.

In my opinion, issuing a warning message about the use being deprecated is good for the users. Furthermore, I don't think the spec wants to add more Fortran stuff to the is_device_ptr clause as it creates so much confusion in the past.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed all your review comments and updated my patch. Can you please review? Thanks!

!WARNING: Variable 'c' in IS_DEVICE_PTR clause must be a dummy argument. This semantic check is deprecated from OpenMP 5.2 and later.
!$omp target is_device_ptr(c)
y = y + 1
!$omp end target
!ERROR: Variable 'b1' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute.
!WARNING: Variable 'b1' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute. This semantic check is deprecated from OpenMP 5.2 and later.
!$omp target is_device_ptr(b1)
y = y + 1
!$omp end target
!ERROR: Variable 'b2' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute.
!WARNING: Variable 'b2' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute. This semantic check is deprecated from OpenMP 5.2 and later.
!$omp target is_device_ptr(b2)
y = y + 1
!$omp end target
!ERROR: Variable 'b3' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute.
!WARNING: Variable 'b3' in IS_DEVICE_PTR clause must be a dummy argument that does not have the ALLOCATABLE, POINTER or VALUE attribute. This semantic check is deprecated from OpenMP 5.2 and later.
!$omp target is_device_ptr(b3)
y = y + 1
!$omp end target
Expand Down