[flang] Allow OpenMP declarations before type declarations #112414

luporl · 2024-10-15T18:26:18Z

Skip resolving implicit types for OpenMP declarative directives, to
allow them to appear before type declarations, which is supported
by several compilers. This was discussed in
https://discourse.llvm.org/t/rfc-openmp-should-type-declaration-be-allowed-after-threadprivate/81345.

This fixes the semantic errors of
#106021.

Skip resolving implicit types for OpenMP declarative directives, to allow them to appear before type declarations, which is supported by several compilers. This was discussed in https://discourse.llvm.org/t/rfc-openmp-should-type-declaration-be-allowed-after-threadprivate/81345. This fixes the semantic errors of llvm#106021.

llvmbot · 2024-10-15T18:26:53Z

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-semantics

Author: Leandro Lupori (luporl)

Changes

Skip resolving implicit types for OpenMP declarative directives, to
allow them to appear before type declarations, which is supported
by several compilers. This was discussed in
https://discourse.llvm.org/t/rfc-openmp-should-type-declaration-be-allowed-after-threadprivate/81345.

This fixes the semantic errors of
#106021.

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

4 Files Affected:

(modified) flang/lib/Semantics/resolve-names.cpp (+28-2)
(renamed) flang/test/Semantics/OpenMP/declarative-directive01.f90 ()
(added) flang/test/Semantics/OpenMP/declarative-directive02.f90 (+56)
(modified) flang/test/Semantics/OpenMP/declare-target06.f90 (-5)

diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index e5e03f644f1b00..3ab93d93b9f54e 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -720,6 +720,7 @@ class ScopeHandler : public ImplicitRulesVisitor {
 
   bool inSpecificationPart_{false};
   bool deferImplicitTyping_{false};
+  bool skipImplicitTyping_{false};
   bool inEquivalenceStmt_{false};
 
   // Some information is collected from a specification part for deferred
@@ -758,6 +759,10 @@ class ScopeHandler : public ImplicitRulesVisitor {
     }
   }
 
+  void SkipImplicitTyping(bool skip) {
+    deferImplicitTyping_ = skipImplicitTyping_ = skip;
+  }
+
 private:
   Scope *currScope_{nullptr};
   FuncResultStack funcResultStack_{*this};
@@ -1506,6 +1511,25 @@ class OmpVisitor : public virtual DeclarationVisitor {
   void Post(const parser::OmpEndCriticalDirective &) {
     messageHandler().set_currStmtSource(std::nullopt);
   }
+  bool Pre(const parser::OpenMPThreadprivate &) {
+    SkipImplicitTyping(true);
+    return true;
+  }
+  void Post(const parser::OpenMPThreadprivate &) { SkipImplicitTyping(false); }
+  bool Pre(const parser::OpenMPDeclareTargetConstruct &) {
+    SkipImplicitTyping(true);
+    return true;
+  }
+  void Post(const parser::OpenMPDeclareTargetConstruct &) {
+    SkipImplicitTyping(false);
+  }
+  bool Pre(const parser::OpenMPDeclarativeAllocate &) {
+    SkipImplicitTyping(true);
+    return true;
+  }
+  void Post(const parser::OpenMPDeclarativeAllocate &) {
+    SkipImplicitTyping(false);
+  }
 };
 
 bool OmpVisitor::NeedsScope(const parser::OpenMPBlockConstruct &x) {
@@ -2556,8 +2580,10 @@ void ScopeHandler::ApplyImplicitRules(
     return;
   }
   if (const DeclTypeSpec * type{GetImplicitType(symbol)}) {
-    symbol.set(Symbol::Flag::Implicit);
-    symbol.SetType(*type);
+    if (!skipImplicitTyping_) {
+      symbol.set(Symbol::Flag::Implicit);
+      symbol.SetType(*type);
+    }
     return;
   }
   if (symbol.has<ProcEntityDetails>() && !symbol.attrs().test(Attr::EXTERNAL)) {
diff --git a/flang/test/Semantics/OpenMP/declarative-directive.f90 b/flang/test/Semantics/OpenMP/declarative-directive01.f90
similarity index 100%
rename from flang/test/Semantics/OpenMP/declarative-directive.f90
rename to flang/test/Semantics/OpenMP/declarative-directive01.f90
diff --git a/flang/test/Semantics/OpenMP/declarative-directive02.f90 b/flang/test/Semantics/OpenMP/declarative-directive02.f90
new file mode 100644
index 00000000000000..dcde963689eb0d
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declarative-directive02.f90
@@ -0,0 +1,56 @@
+! RUN: %flang -fsyntax-only -fopenmp %s 2>&1
+
+! Check that OpenMP declarative directives can be used with objects that have
+! an incomplete type.
+
+subroutine test_decl
+  ! OMPv5.2 5.2 threadprivate
+  ! OMPv5.2 6.5 allocate
+  implicit none
+  save :: x1, y1
+  !$omp threadprivate(x1)
+  !$omp allocate(y1)
+  integer :: x1, y1
+
+  ! OMPv5.2 7.7 declare-simd
+  external :: simd_func
+  !$omp declare simd(simd_func)
+  logical :: simd_func
+
+  ! OMPv5.2 7.8.1 declare-target
+  allocatable :: j
+  !$omp declare target(j)
+  save :: j
+  real(kind=8) :: j(:)
+
+  ! OMPv5.2 5.5.11 declare-reduction - crashes
+  !external :: my_add_red
+  !!$omp declare reduction(my_add_red : integer : my_add_red(omp_out, omp_in)) &
+  !!$omp&  initializer(omp_priv=0)
+  !integer :: my_add_red
+end subroutine
+
+subroutine test_decl2
+  save x1, y1
+  !$omp threadprivate(x1)
+  !$omp allocate(y1)
+  integer :: x1, y1
+
+  ! implicit decl
+  !$omp threadprivate(x2)
+  !$omp allocate(y2)
+  save x2, y2
+end subroutine
+
+module m1
+  ! implicit decl
+  !$omp threadprivate(x, y, z)
+  integer :: y
+  real :: z
+
+contains
+  subroutine sub
+    !$omp parallel copyin(x, y, z)
+    !$omp end parallel
+  end subroutine
+end module
diff --git a/flang/test/Semantics/OpenMP/declare-target06.f90 b/flang/test/Semantics/OpenMP/declare-target06.f90
index 9abcfcecb681ab..7df0a73123094b 100644
--- a/flang/test/Semantics/OpenMP/declare-target06.f90
+++ b/flang/test/Semantics/OpenMP/declare-target06.f90
@@ -6,21 +6,16 @@
 
 module test_0
     implicit none
-!ERROR: The given DECLARE TARGET directive clause has an invalid argument
 !ERROR: No explicit type declared for 'no_implicit_materialization_1'
 !$omp declare target(no_implicit_materialization_1)
 
-!ERROR: The given DECLARE TARGET directive clause has an invalid argument
 !ERROR: No explicit type declared for 'no_implicit_materialization_2'
 !$omp declare target link(no_implicit_materialization_2)
 
-!ERROR: The given DECLARE TARGET directive clause has an invalid argument
 !WARNING: The usage of TO clause on DECLARE TARGET directive has been deprecated. Use ENTER clause instead.
 !ERROR: No explicit type declared for 'no_implicit_materialization_3'
 !$omp declare target to(no_implicit_materialization_3)
 
-!ERROR: The given DECLARE TARGET directive clause has an invalid argument
-!ERROR: No explicit type declared for 'no_implicit_materialization_3'
 !$omp declare target enter(no_implicit_materialization_3)
 
 INTEGER :: data_int = 10

kparzysz

LGTM

tblah · 2024-10-16T09:39:06Z

flang/lib/Semantics/resolve-names.cpp

@@ -758,6 +759,10 @@ class ScopeHandler : public ImplicitRulesVisitor {
    }
  }

+  void SkipImplicitTyping(bool skip) {
+    deferImplicitTyping_ = skipImplicitTyping_ = skip;


Why does deferImplicitTyping_ need to be set here? What is the difference between defer and skip?

When deferImplicitTyping_ is set, checks for explicit types in contexts that require it (e.g. implicit none) are performed later. Not setting it would result in errors, as in the following example:

implicit none save :: x1 !$omp threadprivate(x1) integer :: x1

error: No explicit type declared for 'x1'

This happens because there is a data reference to x1, in !$omp threadprivate(x1), which in other cases would require its explicit type to have already been specified.

The skipImplicitTyping_ flag handles another case. The idea is that it skips setting implicit types. Even when deferImplicitTyping_ is set, the implicit type of an entity is set when it is referenced, as in the example above. The problem is that, if the type that is declared after the reference doesn't match the previous implicit type, an error is reported.
In the example above, with skipImplicitTyping_ = false, x1 type is set to real in the threadprivate line, which doesn't match the integer type specified later.

Thank you for explaining!

tblah

LGTM

ashe2 · 2024-10-16T16:00:02Z

What happens if implicit typing is required? For example,

subroutine s1
  save x1
  !$omp threadprivate (x1)
end subroutine

luporl · 2024-10-16T17:16:29Z

What happens if implicit typing is required? For example,
subroutine s1
  save x1
  !$omp threadprivate (x1)
end subroutine

It works fine, the implicit type is set later, in ResolveNamesVisitor::FinishSpecificationPart for the example above.
We can check that it is set to real/f32 in HLFIR:

  func.func @_QPs1() {
    %0 = fir.address_of(@_QFs1Ex1) : !fir.ref<f32>
    %1:2 = hlfir.declare %0 {uniq_name = "_QFs1Ex1"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
    %2 = omp.threadprivate %1#1 : !fir.ref<f32> -> !fir.ref<f32>
    %3:2 = hlfir.declare %2 {uniq_name = "_QFs1Ex1"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
    return
  }

llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics labels Oct 15, 2024

luporl requested review from tblah, kiranchandramohan, klausler and kparzysz October 15, 2024 19:08

klausler removed their request for review October 15, 2024 19:55

kparzysz approved these changes Oct 15, 2024

View reviewed changes

tblah reviewed Oct 16, 2024

View reviewed changes

tblah approved these changes Oct 16, 2024

View reviewed changes

luporl merged commit a1ac5a5 into llvm:main Oct 17, 2024
12 checks passed

luporl deleted the luporl-omp-skip-impl-typing branch October 17, 2024 14:32

luporl mentioned this pull request Oct 17, 2024

[Flang][OpenMP] Compilation error when declarations within threadprivate directive and explicit declarations #106021

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang] Allow OpenMP declarations before type declarations #112414

[flang] Allow OpenMP declarations before type declarations #112414

Uh oh!

luporl commented Oct 15, 2024

Uh oh!

llvmbot commented Oct 15, 2024 •

edited

Loading

Uh oh!

kparzysz left a comment

Uh oh!

tblah Oct 16, 2024

Uh oh!

luporl Oct 16, 2024

Uh oh!

tblah Oct 16, 2024

Uh oh!

tblah left a comment

Uh oh!

ashe2 commented Oct 16, 2024

Uh oh!

luporl commented Oct 16, 2024

Uh oh!

Uh oh!

Uh oh!

[flang] Allow OpenMP declarations before type declarations #112414

[flang] Allow OpenMP declarations before type declarations #112414

Uh oh!

Conversation

luporl commented Oct 15, 2024

Uh oh!

llvmbot commented Oct 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kparzysz left a comment

Choose a reason for hiding this comment

Uh oh!

tblah Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

luporl Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

tblah Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

ashe2 commented Oct 16, 2024

Uh oh!

luporl commented Oct 16, 2024

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Oct 15, 2024 •

edited

Loading