[flang-commits] [flang] [flang][OpenMP] Fix common block missing symbol crash (PR #67330)

[via flang-commits]

https://github.com/NimishMishra updated https://github.com/llvm/llvm-project/pull/67330

>From ee5ce632a7649cd21586790a04902c3a1f2d5e0e Mon Sep 17 00:00:00 2001
From: Nimish Mishra <neelam.nimish at gmail.com>
Date: Mon, 23 Oct 2023 10:51:59 +0530
Subject: [PATCH 1/2] [flang][OpenMP] Fix common block missing symbol crash

---
 flang/include/flang/Lower/AbstractConverter.h |  4 ++
 flang/lib/Lower/Bridge.cpp                    |  6 +++
 flang/lib/Lower/OpenMP.cpp                    |  3 +-
 flang/test/Lower/OpenMP/FIR/copyin.f90        | 40 +++++++++++++++++++
 4 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/flang/include/flang/Lower/AbstractConverter.h b/flang/include/flang/Lower/AbstractConverter.h
index c792e75f1146499..3e79d3dc22bc7e8 100644
--- a/flang/include/flang/Lower/AbstractConverter.h
+++ b/flang/include/flang/Lower/AbstractConverter.h
@@ -108,6 +108,10 @@ class AbstractConverter {
       const Fortran::semantics::Symbol &sym,
       mlir::OpBuilder::InsertPoint *copyAssignIP = nullptr) = 0;
 
+  /// For a given symbol, check if it is present in the inner-most
+  /// level of the symbol map.
+  virtual bool isPresentShallowLookup(Fortran::semantics::Symbol &sym) = 0;
+
   /// Collect the set of symbols with \p flag in \p eval
   /// region if \p collectSymbols is true. Likewise, collect the
   /// set of the host symbols with \p flag of the associated symbols in \p eval
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index c3afd91d7453caa..2d934c688af22bb 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -585,6 +585,12 @@ class FirConverter : public Fortran::lower::AbstractConverter {
         std::nullopt);
   }
 
+  bool isPresentShallowLookup(Fortran::semantics::Symbol &sym) override final {
+    if (shallowLookupSymbol(sym))
+      return true;
+    return false;
+  }
+
   bool createHostAssociateVarClone(
       const Fortran::semantics::Symbol &sym) override final {
     mlir::Location loc = genLocation(sym.name());
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 5f5e968eaaa6414..1b81336831361c7 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -1551,7 +1551,8 @@ bool ClauseProcessor::processCopyin() const {
           mlir::OpBuilder::InsertPoint *copyAssignIP = nullptr) {
         assert(sym->has<Fortran::semantics::HostAssocDetails>() &&
                "No host-association found");
-        converter.copyHostAssociateVar(*sym, copyAssignIP);
+        if (converter.isPresentShallowLookup(*sym))
+          converter.copyHostAssociateVar(*sym, copyAssignIP);
       };
   bool hasCopyin = findRepeatableClause<ClauseTy::Copyin>(
       [&](const ClauseTy::Copyin *copyinClause,
diff --git a/flang/test/Lower/OpenMP/FIR/copyin.f90 b/flang/test/Lower/OpenMP/FIR/copyin.f90
index 2b3f382240958c4..7de0fb0e3b96af1 100644
--- a/flang/test/Lower/OpenMP/FIR/copyin.f90
+++ b/flang/test/Lower/OpenMP/FIR/copyin.f90
@@ -310,3 +310,43 @@ subroutine common_2()
      end do
   !$omp end parallel do
 end subroutine
+
+!CHECK: func.func @_QPcommon_3() {
+!CHECK: %[[val_0:.*]] = fir.address_of(@blk_) : !fir.ref<!fir.array<8xi8>>
+!CHECK: %[[val_1:.*]] = omp.threadprivate %[[val_0]] : !fir.ref<!fir.array<8xi8>> -> !fir.ref<!fir.array<8xi8>>
+!CHECK: %[[val_2:.*]] = fir.convert %[[val_1]] : (!fir.ref<!fir.array<8xi8>>) -> !fir.ref<!fir.array<?xi8>>
+!CHECK: %[[val_c4:.*]] = arith.constant 4 : index
+!CHECK: %[[val_3:.*]] = fir.coordinate_of %[[val_2]], %[[val_c4]] : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
+!CHECK: %[[val_4:.*]] = fir.convert %[[val_3]] : (!fir.ref<i8>) -> !fir.ref<i32>
+!CHECK: omp.parallel {
+!CHECK: %[[val_5:.*]] = omp.threadprivate %[[val_0]] : !fir.ref<!fir.array<8xi8>> -> !fir.ref<!fir.array<8xi8>>
+!CHECK: %[[val_6:.*]] = fir.convert %[[val_5]] : (!fir.ref<!fir.array<8xi8>>) -> !fir.ref<!fir.array<?xi8>>
+!CHECK: %[[val_c4_0:.*]] = arith.constant 4 : index
+!CHECK: %[[val_7:.*]] = fir.coordinate_of %[[val_6]], %[[val_c4_0]] : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
+!CHECK: %[[val_8:.*]] = fir.convert %[[val_7]] : (!fir.ref<i8>) -> !fir.ref<i32>
+!CHECK: %[[val_9:.*]] = fir.load %[[val_4]] : !fir.ref<i32>
+!CHECK: fir.store %[[val_9]] to %[[val_8]] : !fir.ref<i32>
+!CHECK: omp.barrier
+!CHECK: omp.sections {
+!CHECK: omp.section {
+!CHECK: %[[val_10:.*]] = fir.load %[[val_8]] : !fir.ref<i32>
+!CHECK: %[[val_c3_i32:.*]] = arith.constant 3 : i32
+!CHECK: %[[val_11:.*]] = arith.addi %[[val_10]], %[[val_c3_i32]] : i32
+!CHECK: fir.store %[[val_11]] to %[[val_8]] : !fir.ref<i32>
+!CHECK: omp.terminator
+!CHECK: }
+!CHECK: omp.terminator
+!CHECK: }
+!CHECK: return
+!CHECK: }
+subroutine common_3()
+  integer :: x
+  integer :: y
+  common /blk/ x, y
+  !$omp threadprivate (/blk/)
+
+  !$omp parallel sections copyin(/blk/)
+        !$omp section
+            y = y + 3
+  !$omp end parallel sections
+end subroutine

>From 9c8341bfb140494b8c53dedbe54e680b31211eb9 Mon Sep 17 00:00:00 2001
From: NimishMishra <42909663+NimishMishra at users.noreply.github.com>
Date: Tue, 7 Nov 2023 21:49:41 -0800
Subject: [PATCH 2/2] Update flang/lib/Lower/Bridge.cpp

Co-authored-by: Shraiysh <shraiysh at gmail.com>
---
 flang/lib/Lower/Bridge.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 2d934c688af22bb..825afd43e6923f5 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -586,7 +586,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
   }
 
   bool isPresentShallowLookup(Fortran::semantics::Symbol &sym) override final {
-    if (shallowLookupSymbol(sym))
+    return bool(shallowLookupSymbol(sym));
       return true;
     return false;
   }