[flang][OpenMP] Skip implicit typing for OpenMPDeclarativeConstruct

[mrkajetanp]

DeclareSimdConstruct (and other declarative constructs) can currently implicitly declare variables regardless of whether the source code contains "implicit none" or not. This causes semantic analysis issues if the implicit type does not match the declared type. To solve it, skip implicit typing for OpenMPDeclarativeConstruct. Fixes issue #140754.

[llvmbot]

@llvm/pr-subscribers-flang-semantics

Author: Kajetan Puchalski (mrkajetanp)

Changes

DeclareSimdConstruct currently can implicitly declare variables regardless of whether the source code contains "implicit none" or not. This causes semantic analysis issues if the implicit type does not match the declared type. To solve it, skip implicit typing for declare simd. Fixes issue #140754.

---

Full diff: https://github.com/llvm/llvm-project/pull/142415.diff

2 Files Affected:

• (modified) flang/lib/Semantics/resolve-names.cpp (+1)
• (added) flang/test/Semantics/OpenMP/declare-simd-linear.f90 (+18)

diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 7bea6fdb00e55..93096442329e9 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1513,6 +1513,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
 
   bool Pre(const parser::OpenMPDeclareSimdConstruct &x) {
     AddOmpSourceRange(x.source);
+    SkipImplicitTyping(true);
     return true;
   }
 
diff --git a/flang/test/Semantics/OpenMP/declare-simd-linear.f90 b/flang/test/Semantics/OpenMP/declare-simd-linear.f90
new file mode 100644
index 0000000000000..681cac7f02e0f
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-simd-linear.f90
@@ -0,0 +1,18 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! Test declare simd with linear clause
+
+module mod
+contains
+subroutine sub(m,i)
+!$omp declare simd linear(i:1)
+  implicit none
+  integer*8 i,n
+  value i
+  parameter(n=10000)
+  real*4 a,b,m
+  common/com1/a(n)
+  common/com2/b(n)
+  a(i) = b(i) + m
+  i=i+2
+end subroutine
+end module

[llvmbot]

@llvm/pr-subscribers-flang-openmp

Author: Kajetan Puchalski (mrkajetanp)

Changes

DeclareSimdConstruct currently can implicitly declare variables regardless of whether the source code contains "implicit none" or not. This causes semantic analysis issues if the implicit type does not match the declared type. To solve it, skip implicit typing for declare simd. Fixes issue #140754.

---

Full diff: https://github.com/llvm/llvm-project/pull/142415.diff

2 Files Affected:

• (modified) flang/lib/Semantics/resolve-names.cpp (+1)
• (added) flang/test/Semantics/OpenMP/declare-simd-linear.f90 (+18)

diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 7bea6fdb00e55..93096442329e9 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1513,6 +1513,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
 
   bool Pre(const parser::OpenMPDeclareSimdConstruct &x) {
     AddOmpSourceRange(x.source);
+    SkipImplicitTyping(true);
     return true;
   }
 
diff --git a/flang/test/Semantics/OpenMP/declare-simd-linear.f90 b/flang/test/Semantics/OpenMP/declare-simd-linear.f90
new file mode 100644
index 0000000000000..681cac7f02e0f
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-simd-linear.f90
@@ -0,0 +1,18 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! Test declare simd with linear clause
+
+module mod
+contains
+subroutine sub(m,i)
+!$omp declare simd linear(i:1)
+  implicit none
+  integer*8 i,n
+  value i
+  parameter(n=10000)
+  real*4 a,b,m
+  common/com1/a(n)
+  common/com2/b(n)
+  a(i) = b(i) + m
+  i=i+2
+end subroutine
+end module

[kiranchandramohan]

This is potentially applicable to all declarative directives.

Would it make sense to add SkipImplicitTyping for all declarative directives?

  bool Pre(const parser::OpenMPDeclarativeConstruct &x) {
    SkipImplicitTyping(true)
    AddOmpSourceRange(x.source);
    return true;
  }

For threadprivate and declare target I see that SkipImplicitTyping is reset in the Post function. Is the same required here?

  bool Pre(const parser::OpenMPThreadprivate &) {
   SkipImplicitTyping(true);
   return true;
 }
 void Post(const parser::OpenMPThreadprivate &) { SkipImplicitTyping(false); }

[mrkajetanp]

We should try to work out whether this actually is the desired behaviour. Doing this for declare simd causes the test at Semantics/OpenMP/linear-clause01.f90 to fail because it seems to be expecting some errors:

expect at 31: The list item 'var' specified without the REF 'linear-modifier' must be of INTEGER type@@ -6,0 +8 @@
expect at 33: The type of 'var' has already been implicitly declared

[kiranchandramohan]

We should try to work out whether this actually is the desired behaviour. Doing this for declare simd causes the test at Semantics/OpenMP/linear-clause01.f90 to fail because it seems to be expecting some errors:

expect at 31: The list item 'var' specified without the REF 'linear-modifier' must be of INTEGER type@@ -6,0 +8 @@
expect at 33: The type of 'var' has already been implicitly declared

In this case at least, removal of these expectations is correct.
expect at 31 : var is of type integer so the previous error was incorrect
expect at 33: The var was implicitly declared because the linear clause in declare simd declared the variable during its processing.

It is OK to handle this more generally in a separate PR. But please consider resetting SkipImplicitType in the Post function.

[mrkajetanp]

I this is a spurious CI error? The failing test doesn't have any OpenMP in it, and it works when I run it locally.

[kiranchandramohan]

I this is a spurious CI error? The failing test doesn't have any OpenMP in it, and it works when I run it locally.

I think this was fixed in #142363. Could you rebase?

[kiranchandramohan]

LGTM. Please wait for @tblah.

In a separate patch we might be able to remove SkipImplicitTyping(true); for other declarative constructs as well.

A comment indicating why SkipImplicitTyping is required might be helpful.

[tblah]

LGTM.

In a separate patch we might be able to remove SkipImplicitTyping(true); for other declarative constructs as well.

A comment indicating why SkipImplicitTyping is required might be helpful.

+1