[flang-commits] [flang] [flang][OpenMP] Fix privatization of threadprivate common block (PR #77821)

[Leandro Lupori via flang-commits]

https://github.com/luporl created https://github.com/llvm/llvm-project/pull/77821

In some cases, when privatizing a threadprivate common block, the
original symbol will correspond to the common block, instead of
its threadprivate version. This can happen, for instance, with a
common block, declared in a separate module, used by a parent
procedure and privatized in its child procedure. In this case,
symbol lookup won't find a symbol in the parent procedure, but
only in the module where the common block was defined.

Fixes https://github.com/llvm/llvm-project/issues/65028


>From 1e1b515a7fbcbb717550a36eb2603ee72ce484b3 Mon Sep 17 00:00:00 2001
From: Leandro Lupori <leandro.lupori at linaro.org>
Date: Thu, 11 Jan 2024 16:17:19 -0300
Subject: [PATCH] [flang][OpenMP] Fix privatization of threadprivate common
 block

In some cases, when privatizing a threadprivate common block, the
original symbol will correspond to the common block, instead of
its threadprivate version. This can happen, for instance, with a
common block, declared in a separate module, used by a parent
procedure and privatized in its child procedure. In this case,
symbol lookup won't find a symbol in the parent procedure, but
only in the module where the common block was defined.

Fixes https://github.com/llvm/llvm-project/issues/65028
---
 flang/lib/Lower/OpenMP.cpp                    | 20 +++++++------
 .../OpenMP/threadprivate-commonblock-use.f90  | 29 +++++++++++++++++++
 2 files changed, 40 insertions(+), 9 deletions(-)
 create mode 100644 flang/test/Lower/OpenMP/threadprivate-commonblock-use.f90

diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index c3a570bf15ea0d..ce2ee4befddcf1 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -2029,18 +2029,20 @@ static void threadPrivatizeVars(Fortran::lower::AbstractConverter &converter,
   mlir::OpBuilder::InsertPoint insPt = firOpBuilder.saveInsertionPoint();
   firOpBuilder.setInsertionPointToStart(firOpBuilder.getAllocaBlock());
 
-  // Get the original ThreadprivateOp corresponding to the symbol and use the
-  // symbol value from that operation to create one ThreadprivateOp copy
-  // operation inside the parallel region.
+  // If the symbol corresponds to the original ThreadprivateOp, use the symbol
+  // value from that operation to create one ThreadprivateOp copy operation
+  // inside the parallel region.
+  // In some cases, however, the symbol will correspond to the original,
+  // non-threadprivate variable. This can happen, for instance, with a common
+  // block, declared in a separate module, used by a parent procedure and
+  // privatized in its child procedure.
   auto genThreadprivateOp = [&](Fortran::lower::SymbolRef sym) -> mlir::Value {
-    mlir::Value symOriThreadprivateValue = converter.getSymbolAddress(sym);
-    mlir::Operation *op = symOriThreadprivateValue.getDefiningOp();
+    mlir::Value symValue = converter.getSymbolAddress(sym);
+    mlir::Operation *op = symValue.getDefiningOp();
     if (auto declOp = mlir::dyn_cast<hlfir::DeclareOp>(op))
       op = declOp.getMemref().getDefiningOp();
-    assert(mlir::isa<mlir::omp::ThreadprivateOp>(op) &&
-           "Threadprivate operation not created");
-    mlir::Value symValue =
-        mlir::dyn_cast<mlir::omp::ThreadprivateOp>(op).getSymAddr();
+    if (mlir::isa<mlir::omp::ThreadprivateOp>(op))
+      symValue = mlir::dyn_cast<mlir::omp::ThreadprivateOp>(op).getSymAddr();
     return firOpBuilder.create<mlir::omp::ThreadprivateOp>(
         currentLocation, symValue.getType(), symValue);
   };
diff --git a/flang/test/Lower/OpenMP/threadprivate-commonblock-use.f90 b/flang/test/Lower/OpenMP/threadprivate-commonblock-use.f90
new file mode 100644
index 00000000000000..28616f7595a0d6
--- /dev/null
+++ b/flang/test/Lower/OpenMP/threadprivate-commonblock-use.f90
@@ -0,0 +1,29 @@
+! This test checks lowering of OpenMP Threadprivate Directive.
+! Test for common block, defined in one module, used in a subroutine of
+! another module and privatized in a nested subroutine.
+
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+!CHECK: fir.global common @cmn_(dense<0> : vector<4xi8>) : !fir.array<4xi8>
+module m0
+  common /cmn/ k1
+  !$omp threadprivate(/cmn/)
+end
+
+module  m1
+contains
+  subroutine ss1
+    use m0
+  contains
+!CHECK-LABEL: func @_QMm1Fss1Pss2
+!CHECK: %[[CMN:.*]] = fir.address_of(@cmn_) : !fir.ref<!fir.array<4xi8>>
+!CHECK: omp.parallel
+!CHECK: %{{.*}} = omp.threadprivate %[[CMN]] : !fir.ref<!fir.array<4xi8>> -> !fir.ref<!fir.array<4xi8>>
+    subroutine ss2
+      !$omp parallel copyin (k1)
+      !$omp end parallel
+    end subroutine ss2
+  end subroutine ss1
+end
+
+end