[RFC][OpenMP] Should type declaration be allowed after threadprivate?

Subprojects Flang
1

Hello,

Currently, Flang reports an error if the type of a variable is specified after it was declared as threadprivate. There is an open issue for it: [Flang][OpenMP] Compilation error when declarations within threadprivate directive and explicit declarations · Issue #106021 · llvm/llvm-project · GitHub.

The example below triggers this error:

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

error: Semantic errors in test.f90
./test.f90:4:11: error: The type of 'x1' has already been implicitly declared
    integer x1
            ^^
./test.f90:2:8: Implicit declaration of 'x1'
    save x1
         ^^

However, I couldn’t find in the OpenMP standard if a type declaration after threadprivate is used should be allowed or not. Gfortran allows it, but ifort and ifx report the following error:

This name cannot be assigned this data type because it conflicts with prior uses of the name

With Flang, AFAIU, what happens is that x1 in !$omp threadprivate (x1) is handled as a data reference and its type is determined at that moment. In the example above, implicit rules are used and the resulting type is REAL. When integer x1 is processed, an error occurs, as the type was already implicitly declared as REAL. This behavior seems similar to that of ifort and ifx.

A change that could be made to support declarations with multiple parts, where !$omp threadprivate is not the last part, would be to skip resolving threadprivate variable names in ResolveSpecificationParts() and only resolve them later, in ResolveOmpParts().

The changes below are enough to support the previous example:

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 4aecb8b8e7b4..34b3b37e2900 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2319,6 +2319,9 @@ void OmpAttributeVisitor::ResolveOmpObject(
           [&](const parser::Designator &designator) {
             if (const auto *name{
                     semantics::getDesignatorNameIfDataRef(designator)}) {
+              if (!name->symbol) {
+                name->symbol = ResolveName(name);
+              }
               if (auto *symbol{ResolveOmp(*name, ompFlag, currScope())}) {
                 auto checkExclusivelists =
                     [&](const Symbol *symbol1, Symbol::Flag firstOmpFlag,
@@ -2429,6 +2432,9 @@ void OmpAttributeVisitor::ResolveOmpObject(
             }
           },
           [&](const parser::Name &name) { // common block
+            if (!name.symbol) {
+              name.symbol = ResolveName(&name);
+            }
             if (auto *symbol{ResolveOmpCommonBlockName(&name)}) {
               if (!dataCopyingAttributeFlags.test(ompFlag)) {
                 CheckMultipleAppearances(
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index ec8f854f64d1..a37d00291866 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1489,6 +1489,9 @@ public:
   void Post(const parser::OmpEndCriticalDirective &) {
     messageHandler().set_currStmtSource(std::nullopt);
   }
+  bool Pre(const parser::OpenMPThreadprivate &) {
+    return false;
+  }
 };

 bool OmpVisitor::NeedsScope(const parser::OpenMPBlockConstruct &x) {

But as OmpAttributeVisitor::ResolveName performs only a simple symbol lookup, it can’t handle more complex specifications, such as structure elements, that are handled by DeclarationVisitor::ResolveDesignator. Exporting it somehow to OmpAttributeVisitor would be a bigger change.

But should Flang support type declaration after threadprivate?

2

What would we gain by allowing it?

The C/C++ section of the OpenMP 5.2 spec requires the variable type to be complete. My personal though on this is that in a similar spirit we should require that all statements that contribute to the variable declaration precede the threadprivate directive.

1 Like
3

Better compatibility with gfortran and possibly with other compilers. I have checked only with gfortran, ifort and ifx. We would also support programs that use threadprivate in this way, although I can’t give any examples.

As the standard isn’t clear whether this should be allowed or not, the main downside would be increased complexity in the implementation.

That’s a good point. In my tests, both clang and gcc disallowed this, even when the complete type appeared right after threadprivate.

4

Is it possible to check whether this issue originally came from an application?

5

It originally came from a test for Fujitsu Fortran Compiler.
(and Fujitsu Fortran Compiler allows it.)

6

Should I consider that, at least for now, it’s better not to support this in Flang?

7

Do you know whether Classic Flang or nagfortran upports it?

8

I have just tried the example program with Classic Flang 17.0.2 and it compiled without errors.

I don’t have access to nagfortran.

9

This code with NAG Fortran compiler, latest release:

$ nagfor -openmp -c flang-threadprivate.f90 
NAG Fortran Compiler Release 7.2(Shin-Urayasu) Build 7219
Warning: flang-threadprivate.f90, line 5: Unused local variable X1
[NAG Fortran Compiler normal termination, 1 warning]
1 Like
10

If we wanted to implement this, we’d need to handle “incomplete” declarations in all declarative directives. How much work would that take?

Derived type members shouldn’t be an issue, since AFAIR you can’t have OpenMP directives inside of a derived type definition, and after the definition the type is completely defined.

11

Do you mean all OpenMP directives which belong to the declarative category, such as declare target, declare reduction, declare mapper, etc?
If so, this would become a much bigger change.

Would it be acceptable to change only threadprivate for now?
Then I could investigate how much work it would require and, if it’s not that much, propose a patch for it, that could be used as a template for the other directives.

12

Proposed patch: [flang] Allow OpenMP declarations before type declarations by luporl · Pull Request #112414 · llvm/llvm-project · GitHub

1 Like