[flang-commits] [flang] [flang][OpenMP] Allow common blocks in nested directives (PR #88430)

[Leandro Lupori via flang-commits]

https://github.com/luporl updated https://github.com/llvm/llvm-project/pull/88430

>From f11723b2da4a54675c4eb6539ba25adb85d8a360 Mon Sep 17 00:00:00 2001
From: Leandro Lupori <leandro.lupori at linaro.org>
Date: Thu, 11 Apr 2024 16:33:39 -0300
Subject: [PATCH 1/2] [flang][OpenMP] Allow common blocks in nested directives

COMMON block names must be declared in the same scoping unit in
which the OpenMP directive or clause appears. However, given that
common blocks and threadprivate directives can't be nested inside
other directives, such as parallel, and based on the behavior of
other compilers, it seems that OpenMP block constructs should not
be considered as scoping units.

This patch implements this behavior by also looking for common
block names in the scoping unit of the outermost directive.
---
 flang/lib/Semantics/resolve-directives.cpp | 17 +++++++++++++----
 flang/test/Semantics/OpenMP/resolve03.f90  | 16 ++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index f4ac7f198d854e..2d7cc4c45a1b91 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2101,10 +2101,19 @@ Symbol *OmpAttributeVisitor::ResolveOmpCommonBlockName(
     name->symbol = cur;
     return cur;
   }
-  // Then check parent scope
-  if (auto *prev{GetContext().scope.parent().FindCommonBlock(name->source)}) {
-    name->symbol = prev;
-    return prev;
+  // Then check parent scope, skipping OpenMP scopes and making sure
+  // that the top directive started its own scope.
+  DirContext &topDirContext = dirContext_.front();
+  Scope &topDirScope = topDirContext.scope;
+  assert(!topDirContext.directiveSource.empty());
+  assert(!topDirScope.sourceRange().empty());
+  if (!topDirScope.IsTopLevel() &&
+      topDirContext.directiveSource.begin() ==
+          topDirScope.sourceRange().begin()) {
+    if (auto *prev{topDirScope.parent().FindCommonBlock(name->source)}) {
+      name->symbol = prev;
+      return prev;
+    }
   }
   return nullptr;
 }
diff --git a/flang/test/Semantics/OpenMP/resolve03.f90 b/flang/test/Semantics/OpenMP/resolve03.f90
index b9306c4fe9cb4d..1228f070e0595f 100644
--- a/flang/test/Semantics/OpenMP/resolve03.f90
+++ b/flang/test/Semantics/OpenMP/resolve03.f90
@@ -8,6 +8,9 @@
 
   common /c/ a, b
   integer a(3), b
+  common /tc/ xy
+  integer xz
+  !$omp threadprivate(/tc/)
 
   A = 1
   B = 2
@@ -19,4 +22,17 @@
     !$omp end parallel
   end block
   print *, a, b
+
+  ! Common block names may be used inside nested OpenMP directives.
+  !$omp parallel
+    !$omp parallel copyin(/tc/)
+      xz = xz + 10
+    !$omp end parallel
+  !$omp end parallel
+
+  !$omp parallel
+    !$omp single
+      xz = 18
+    !$omp end single copyprivate(/tc/)
+  !$omp end parallel
 end

>From 54a0df15df8ba59e318deed33bdb1879d977f710 Mon Sep 17 00:00:00 2001
From: Leandro Lupori <leandro.lupori at linaro.org>
Date: Wed, 17 Apr 2024 14:04:25 -0300
Subject: [PATCH 2/2] Search for common blocks in the current scoping unit

---
 flang/lib/Semantics/resolve-directives.cpp | 22 ++++------------------
 flang/test/Semantics/OpenMP/resolve03.f90  | 17 +++++++++++++----
 2 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 2d7cc4c45a1b91..318687508ff1f5 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2096,24 +2096,10 @@ Symbol *OmpAttributeVisitor::ResolveOmpCommonBlockName(
   if (!name) {
     return nullptr;
   }
-  // First check if the Common Block is declared in the current scope
-  if (auto *cur{GetContext().scope.FindCommonBlock(name->source)}) {
-    name->symbol = cur;
-    return cur;
-  }
-  // Then check parent scope, skipping OpenMP scopes and making sure
-  // that the top directive started its own scope.
-  DirContext &topDirContext = dirContext_.front();
-  Scope &topDirScope = topDirContext.scope;
-  assert(!topDirContext.directiveSource.empty());
-  assert(!topDirScope.sourceRange().empty());
-  if (!topDirScope.IsTopLevel() &&
-      topDirContext.directiveSource.begin() ==
-          topDirScope.sourceRange().begin()) {
-    if (auto *prev{topDirScope.parent().FindCommonBlock(name->source)}) {
-      name->symbol = prev;
-      return prev;
-    }
+  if (auto *cb{GetProgramUnitOrBlockConstructContaining(GetContext().scope)
+                   .FindCommonBlock(name->source)}) {
+    name->symbol = cb;
+    return cb;
   }
   return nullptr;
 }
diff --git a/flang/test/Semantics/OpenMP/resolve03.f90 b/flang/test/Semantics/OpenMP/resolve03.f90
index 1228f070e0595f..ebc66ca12ebf44 100644
--- a/flang/test/Semantics/OpenMP/resolve03.f90
+++ b/flang/test/Semantics/OpenMP/resolve03.f90
@@ -8,8 +8,8 @@
 
   common /c/ a, b
   integer a(3), b
-  common /tc/ xy
-  integer xz
+  common /tc/ x
+  integer x
   !$omp threadprivate(/tc/)
 
   A = 1
@@ -23,16 +23,25 @@
   end block
   print *, a, b
 
+  !$omp parallel
+    block
+      !$omp single
+        x = 18
+      !ERROR: COMMON block must be declared in the same scoping unit in which the OpenMP directive or clause appears
+      !$omp end single copyprivate(/tc/)
+    end block
+  !$omp end parallel
+
   ! Common block names may be used inside nested OpenMP directives.
   !$omp parallel
     !$omp parallel copyin(/tc/)
-      xz = xz + 10
+      x = x + 10
     !$omp end parallel
   !$omp end parallel
 
   !$omp parallel
     !$omp single
-      xz = 18
+      x = 18
     !$omp end single copyprivate(/tc/)
   !$omp end parallel
 end