[flang][OpenMP] Catch threadprivate common block vars that appear in equivalence #127642

luporl · 2025-02-18T14:38:24Z

Semantics were not checking for variables appearing in equivalence
statements when those were part of a threadprivate common block.

Fixes #122825

…equivalence Semantics were not checking for variables appearing in equivalence statements when those were part of a threadprivate common block. Fixes llvm#122825

llvmbot · 2025-02-18T14:39:01Z

@llvm/pr-subscribers-flang-semantics

Author: Leandro Lupori (luporl)

Changes

Semantics were not checking for variables appearing in equivalence
statements when those were part of a threadprivate common block.

Fixes #122825

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

2 Files Affected:

(modified) flang/lib/Semantics/check-omp-structure.cpp (+16-1)
(modified) flang/test/Semantics/OpenMP/threadprivate02.f90 (+6)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index fd2893998205c..62de902726c8b 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1479,7 +1479,22 @@ void OmpStructureChecker::CheckThreadprivateOrDeclareTargetVar(
                 }
               }
             },
-            [&](const parser::Name &) {}, // common block
+            [&](const parser::Name &name) {
+              if (!name.symbol) {
+                return;
+              }
+              if (auto *cb{name.symbol->detailsIf<CommonBlockDetails>()}) {
+                for (const auto &obj : cb->objects()) {
+                  if (FindEquivalenceSet(*obj)) {
+                    context_.Say(name.source,
+                        "A variable in a %s directive cannot appear in an EQUIVALENCE statement"
+                        " (variable '%s' from common block '/%s/')"_err_en_US,
+                        ContextDirectiveAsFortran(), obj->name(),
+                        name.symbol->name());
+                  }
+                }
+              }
+            },
         },
         ompObject.u);
   }
diff --git a/flang/test/Semantics/OpenMP/threadprivate02.f90 b/flang/test/Semantics/OpenMP/threadprivate02.f90
index 7f6e8dcc8e8ab..9dc031a8ce47e 100644
--- a/flang/test/Semantics/OpenMP/threadprivate02.f90
+++ b/flang/test/Semantics/OpenMP/threadprivate02.f90
@@ -7,6 +7,9 @@ program threadprivate02
   integer :: arr1(10)
   common /blk1/ a1
   real, save :: eq_a, eq_b, eq_c, eq_d
+  integer :: eq_e, eq_f
+  equivalence(eq_e, eq_f)
+  common /blk2/ eq_e
 
   !$omp threadprivate(arr1)
 
@@ -25,6 +28,9 @@ program threadprivate02
   !$omp threadprivate(eq_c)
   equivalence(eq_c, eq_d)
 
+  !ERROR: A variable in a THREADPRIVATE directive cannot appear in an EQUIVALENCE statement (variable 'eq_e' from common block '/blk2/')
+  !$omp threadprivate(/blk2/)
+
 contains
   subroutine func()
     integer :: arr2(10)

llvmbot · 2025-02-18T14:39:01Z

@llvm/pr-subscribers-flang-openmp

Author: Leandro Lupori (luporl)

Changes

Semantics were not checking for variables appearing in equivalence
statements when those were part of a threadprivate common block.

Fixes #122825

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

2 Files Affected:

(modified) flang/lib/Semantics/check-omp-structure.cpp (+16-1)
(modified) flang/test/Semantics/OpenMP/threadprivate02.f90 (+6)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index fd2893998205c..62de902726c8b 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1479,7 +1479,22 @@ void OmpStructureChecker::CheckThreadprivateOrDeclareTargetVar(
                 }
               }
             },
-            [&](const parser::Name &) {}, // common block
+            [&](const parser::Name &name) {
+              if (!name.symbol) {
+                return;
+              }
+              if (auto *cb{name.symbol->detailsIf<CommonBlockDetails>()}) {
+                for (const auto &obj : cb->objects()) {
+                  if (FindEquivalenceSet(*obj)) {
+                    context_.Say(name.source,
+                        "A variable in a %s directive cannot appear in an EQUIVALENCE statement"
+                        " (variable '%s' from common block '/%s/')"_err_en_US,
+                        ContextDirectiveAsFortran(), obj->name(),
+                        name.symbol->name());
+                  }
+                }
+              }
+            },
         },
         ompObject.u);
   }
diff --git a/flang/test/Semantics/OpenMP/threadprivate02.f90 b/flang/test/Semantics/OpenMP/threadprivate02.f90
index 7f6e8dcc8e8ab..9dc031a8ce47e 100644
--- a/flang/test/Semantics/OpenMP/threadprivate02.f90
+++ b/flang/test/Semantics/OpenMP/threadprivate02.f90
@@ -7,6 +7,9 @@ program threadprivate02
   integer :: arr1(10)
   common /blk1/ a1
   real, save :: eq_a, eq_b, eq_c, eq_d
+  integer :: eq_e, eq_f
+  equivalence(eq_e, eq_f)
+  common /blk2/ eq_e
 
   !$omp threadprivate(arr1)
 
@@ -25,6 +28,9 @@ program threadprivate02
   !$omp threadprivate(eq_c)
   equivalence(eq_c, eq_d)
 
+  !ERROR: A variable in a THREADPRIVATE directive cannot appear in an EQUIVALENCE statement (variable 'eq_e' from common block '/blk2/')
+  !$omp threadprivate(/blk2/)
+
 contains
   subroutine func()
     integer :: arr2(10)

github-actions · 2025-02-18T14:42:12Z

✅ With the latest revision this PR passed the C/C++ code formatter.

luporl · 2025-02-18T14:44:42Z

The clang-format issue is caused by not breaking up the error message.

kiranchandramohan

The clang-format issue is caused by not breaking up the error message.

Will putting it in a single line help. Having the error message in a single line makes it easier to search for it in the code base.

flang/lib/Semantics/check-omp-structure.cpp

luporl · 2025-02-19T13:37:10Z

Will putting it in a single line help. Having the error message in a single line makes it easier to search for it in the code base.

It helped, clang-format didn't complain with the error message in a single line.

kiranchandramohan

LG. Thanks @luporl

tblah

Thanks!

sscalpone · 2025-02-21T15:17:37Z

Hi @luporl , I think this change caused fujitsu tests 0685 0685_0007.f90 to change behavior.

Before, it was compiling, executing, and matching the expected output.

Now, there's a compile-time error:

0685/0685_0007.f90:3:22: error: A variable in a THREADPRIVATE directive cannot appear in an EQUIVALENCE statement (variable 'ia' from common block '/com1/')
  !$omp threadprivate(/com1/)

I am not an expert in OpenMP semantics. Shall we update the test's expected output?

@ohno-fj @kiranchandramohan fyi

kiranchandramohan · 2025-02-21T16:50:16Z

I think @luporl is basing the change of this PR on the following points from the standard.

Page 61 : OpenMP 5.2

11 When a named common block appears in an OpenMP argument list, it has the same meaning and
12 restrictions as if every explicit member of the common block appeared in the list. An explicit
13 member of a common block is a variable that is named in a COMMON statement that specifies the
14 common block name and is declared in the same scoping unit in which the clause appears.

Page 105 : OpenMP 5.2

8 A variable may only appear as an argument in a threadprivate directive in the scope in
9 which it is declared. It must not be an element of a common block or appear in an
10 EQUIVALENCE statement.

I see that gfortran also adopts this interpretation. But ifx and classic flang/nvfortran do not and allow the usage in https://github.com/fujitsu/compiler-test-suite/blob/main/Fortran/0685/0685_0007.f90
@kkwli @mjklemm Any opinion here?

kkwli · 2025-02-21T19:36:25Z

I think @luporl is basing the change of this PR on the following points from the standard.
Page 61 : OpenMP 5.2

11 When a named common block appears in an OpenMP argument list, it has the same meaning and
12 restrictions as if every explicit member of the common block appeared in the list. An explicit
13 member of a common block is a variable that is named in a COMMON statement that specifies the
14 common block name and is declared in the same scoping unit in which the clause appears.

Page 105 : OpenMP 5.2

8 A variable may only appear as an argument in a threadprivate directive in the scope in
9 which it is declared. It must not be an element of a common block or appear in an
10 EQUIVALENCE statement.
I see that gfortran also adopts this interpretation. But ifx and classic flang/nvfortran do not and allow the usage in https://github.com/fujitsu/compiler-test-suite/blob/main/Fortran/0685/0685_0007.f90 @kkwli @mjklemm Any opinion here?

@kiranchandramohan I agree. The idea is that threadprivate(/com1/) is the same as threadprivate(ia,ib). If iz is referenced in the parallel region, there is no way to tell which copy of the ia is actually referenced. The code in 0685_0007.f90 is nonconforming. In my opinion, it would be better to issue an error.

[flang][OpenMP] Catch threadprivate common block vars that appear in …

1cbe00d

…equivalence Semantics were not checking for variables appearing in equivalence statements when those were part of a threadprivate common block. Fixes llvm#122825

llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics labels Feb 18, 2025

luporl requested review from tblah, kiranchandramohan and kparzysz February 18, 2025 14:42

kiranchandramohan reviewed Feb 18, 2025

View reviewed changes

flang/lib/Semantics/check-omp-structure.cpp Outdated Show resolved Hide resolved

Fix style and format

b9c55a2

kiranchandramohan approved these changes Feb 19, 2025

View reviewed changes

tblah approved these changes Feb 19, 2025

View reviewed changes

luporl merged commit d7784a6 into llvm:main Feb 20, 2025
8 checks passed

luporl deleted the luporl-omp-tp-cb-eqv branch February 20, 2025 11:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang][OpenMP] Catch threadprivate common block vars that appear in equivalence #127642

[flang][OpenMP] Catch threadprivate common block vars that appear in equivalence #127642

Uh oh!

luporl commented Feb 18, 2025

Uh oh!

llvmbot commented Feb 18, 2025

Uh oh!

llvmbot commented Feb 18, 2025

Uh oh!

github-actions bot commented Feb 18, 2025 •

edited

Loading

Uh oh!

luporl commented Feb 18, 2025

Uh oh!

kiranchandramohan left a comment

Uh oh!

Uh oh!

luporl commented Feb 19, 2025

Uh oh!

kiranchandramohan left a comment

Uh oh!

tblah left a comment

Uh oh!

Uh oh!

sscalpone commented Feb 21, 2025

Uh oh!

kiranchandramohan commented Feb 21, 2025

Uh oh!

kkwli commented Feb 21, 2025

Uh oh!

Uh oh!

[flang][OpenMP] Catch threadprivate common block vars that appear in equivalence #127642

[flang][OpenMP] Catch threadprivate common block vars that appear in equivalence #127642

Uh oh!

Conversation

luporl commented Feb 18, 2025

Uh oh!

llvmbot commented Feb 18, 2025

Uh oh!

llvmbot commented Feb 18, 2025

Uh oh!

github-actions bot commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luporl commented Feb 18, 2025

Uh oh!

kiranchandramohan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

luporl commented Feb 19, 2025

Uh oh!

kiranchandramohan left a comment

Choose a reason for hiding this comment

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sscalpone commented Feb 21, 2025

Uh oh!

kiranchandramohan commented Feb 21, 2025

Uh oh!

kkwli commented Feb 21, 2025

Uh oh!

Uh oh!

github-actions bot commented Feb 18, 2025 •

edited

Loading