[flang-commits] [flang] 8b834ca - [Flang][OpenMP] Fix HLFIR lowering for commonblock threadprivate

[Kiran Chandramohan via flang-commits]

Author: Kiran Chandramohan
Date: 2023-08-23T11:37:17Z
New Revision: 8b834caa62a279a0b4136bf3c8950b4f7162308e

URL: https://github.com/llvm/llvm-project/commit/8b834caa62a279a0b4136bf3c8950b4f7162308e
DIFF: https://github.com/llvm/llvm-project/commit/8b834caa62a279a0b4136bf3c8950b4f7162308e.diff

LOG: [Flang][OpenMP] Fix HLFIR lowering for commonblock threadprivate

Commonblock names are not variables, but they can be marked as
threadprivate in OpenMP. This requires the commonblock name to
be bound to the address of the Commonblock. hlfir.declares are
not required for these, but we should be able to retrieve the
mlir Value corresponding to the Commonblock. This patch enables
this by special casing the Commonblocks like procedures.

Reviewed By: tblah, vzakhari

Differential Revision: https://reviews.llvm.org/D158070

Added: 
    flang/test/Lower/OpenMP/threadprivate-common-block-hlfir.f90

Modified: 
    flang/lib/Lower/Bridge.cpp
    flang/lib/Lower/ConvertVariable.cpp

Removed: 
    


################################################################################
diff  --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index be6004d57b36fc..51018b8122e81c 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -954,6 +954,13 @@ class FirConverter : public Fortran::lower::AbstractConverter {
       // Do a regular lookup.
       if (Fortran::semantics::IsProcedure(sym))
         return symMap->lookupSymbol(sym);
+
+      // Commonblock names are not variables, but in some lowerings (like
+      // OpenMP) it is useful to maintain the address of the commonblock in an
+      // MLIR value and query it. hlfir.declare need not be created for these.
+      if (sym.detailsIf<Fortran::semantics::CommonBlockDetails>())
+        return symMap->lookupSymbol(sym);
+
       return {};
     }
     if (Fortran::lower::SymbolBox v = symMap->lookupSymbol(sym))

diff  --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index b0dcf4d4256a81..20c99558b1a9d8 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -1470,8 +1470,12 @@ static void genDeclareSymbol(Fortran::lower::AbstractConverter &converter,
   // would add too much complexity to hlfir.declare to support this case, and
   // this would bring very little (the only point being debug info, that are not
   // yet emitted) since alias analysis is meaningless for those.
+  // Commonblock names are not variables, but in some lowerings (like OpenMP) it
+  // is useful to maintain the address of the commonblock in an MLIR value and
+  // query it. hlfir.declare need not be created for these.
   if (converter.getLoweringOptions().getLowerToHighLevelFIR() &&
-      !Fortran::semantics::IsProcedure(sym)) {
+      !Fortran::semantics::IsProcedure(sym) &&
+      !sym.detailsIf<Fortran::semantics::CommonBlockDetails>()) {
     fir::FirOpBuilder &builder = converter.getFirOpBuilder();
     const mlir::Location loc = genLocation(converter, sym);
     mlir::Value shapeOrShift;
@@ -1521,7 +1525,8 @@ void Fortran::lower::genDeclareSymbol(
     Fortran::lower::SymMap &symMap, const Fortran::semantics::Symbol &sym,
     const fir::ExtendedValue &exv, bool force) {
   if (converter.getLoweringOptions().getLowerToHighLevelFIR() &&
-      !Fortran::semantics::IsProcedure(sym)) {
+      !Fortran::semantics::IsProcedure(sym) &&
+      !sym.detailsIf<Fortran::semantics::CommonBlockDetails>()) {
     fir::FirOpBuilder &builder = converter.getFirOpBuilder();
     const mlir::Location loc = genLocation(converter, sym);
     fir::FortranVariableFlagsAttr attributes =

diff  --git a/flang/test/Lower/OpenMP/threadprivate-common-block-hlfir.f90 b/flang/test/Lower/OpenMP/threadprivate-common-block-hlfir.f90
new file mode 100644
index 00000000000000..29e3e277dc8960
--- /dev/null
+++ b/flang/test/Lower/OpenMP/threadprivate-common-block-hlfir.f90
@@ -0,0 +1,29 @@
+! Simple test for lowering of OpenMP Threadprivate Directive with HLFIR.
+! Test for common block.
+
+!RUN: %flang_fc1 -flang-experimental-hlfir -emit-hlfir -fopenmp %s -o - | FileCheck %s
+!RUN: bbc -hlfir -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+
+!CHECK: %[[CBLK_ADDR:.*]] = fir.address_of(@_QCblk) : !fir.ref<!fir.array<4xi8>>
+!CHECK: {{.*}} = omp.threadprivate %[[CBLK_ADDR]] : !fir.ref<!fir.array<4xi8>> -> !fir.ref<!fir.array<4xi8>>
+!CHECK: omp.parallel   {
+!CHECK:   %[[TP_PARALLEL:.*]] = omp.threadprivate %[[CBLK_ADDR]] : !fir.ref<!fir.array<4xi8>> -> !fir.ref<!fir.array<4xi8>>
+!CHECK:   %[[TP_PARALLEL_ADDR:.*]] = fir.convert %[[TP_PARALLEL]] : (!fir.ref<!fir.array<4xi8>>) -> !fir.ref<!fir.array<?xi8>>
+!CHECK:   %[[A_ADDR:.*]] = fir.coordinate_of %[[TP_PARALLEL_ADDR]], %c0_1 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
+!CHECK:   %[[A_ADDR_CVT:.*]] = fir.convert %[[A_ADDR]] : (!fir.ref<i8>) -> !fir.ref<i32>
+!CHECK:   %[[A_DECL:.*]]:2 = hlfir.declare %[[A_ADDR_CVT]] {uniq_name = "_QFsub_commonblockEa"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK:   %[[A_VAL:.*]] = fir.load %[[A_DECL]]#0 : !fir.ref<i32>
+!CHECK:   {{.*}} = fir.call @_FortranAioOutputInteger32({{.*}}, %[[A_VAL]]) fastmath<contract> : (!fir.ref<i8>, i32) -> i1
+!CHECK:   omp.terminator
+!CHECK: }
+
+subroutine sub_commonblock()
+  integer:: a
+  common /blk/ a
+  !$omp threadprivate(/blk/)
+
+  !$omp parallel
+    print *, a
+  !$omp end parallel
+end