[flang-commits] [flang] 6607330 - [flang][OpenMP] Fix pointer variables in atomic read/write

via flang-commits flang-commits at lists.llvm.org
Sat May 28 01:43:08 PDT 2022


Author: Peixin-Qiao
Date: 2022-05-28T16:41:14+08:00
New Revision: 66073306d888acd9b3e8acbbb4bda54ba25bc7e9

URL: https://github.com/llvm/llvm-project/commit/66073306d888acd9b3e8acbbb4bda54ba25bc7e9
DIFF: https://github.com/llvm/llvm-project/commit/66073306d888acd9b3e8acbbb4bda54ba25bc7e9.diff

LOG: [flang][OpenMP] Fix pointer variables in atomic read/write

For pointer variables, using getSymbolAddress cannot get the coorect
address for atomic read/write operands. Use genExprAddr to fix it.

Reviewed By: shraiysh, NimishMishra

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

Added: 
    

Modified: 
    flang/lib/Lower/OpenMP.cpp
    flang/test/Lower/OpenMP/atomic-read.f90
    flang/test/Lower/OpenMP/atomic-write.f90

Removed: 
    


################################################################################
diff  --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index b42fb95d50ffe..535ffac82d9e5 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -841,11 +841,7 @@ genOmpAtomicWrite(Fortran::lower::AbstractConverter &converter,
                   const Fortran::parser::OmpAtomicWrite &atomicWrite) {
   auto &firOpBuilder = converter.getFirOpBuilder();
   auto currentLocation = converter.getCurrentLocation();
-  mlir::Value address;
-  // If no hint clause is specified, the effect is as if
-  // hint(omp_sync_hint_none) had been specified.
-  mlir::IntegerAttr hint = nullptr;
-  mlir::omp::ClauseMemoryOrderKindAttr memory_order = nullptr;
+  // Get the value and address of atomic write operands.
   const Fortran::parser::OmpAtomicClauseList &rightHandClauseList =
       std::get<2>(atomicWrite.t);
   const Fortran::parser::OmpAtomicClauseList &leftHandClauseList =
@@ -855,16 +851,14 @@ genOmpAtomicWrite(Fortran::lower::AbstractConverter &converter,
   const auto &assignmentStmtVariable = std::get<Fortran::parser::Variable>(
       std::get<3>(atomicWrite.t).statement.t);
   Fortran::lower::StatementContext stmtCtx;
-  auto value = fir::getBase(converter.genExprValue(
+  mlir::Value value = fir::getBase(converter.genExprValue(
       *Fortran::semantics::GetExpr(assignmentStmtExpr), stmtCtx));
-  if (auto varDesignator = std::get_if<
-          Fortran::common::Indirection<Fortran::parser::Designator>>(
-          &assignmentStmtVariable.u)) {
-    if (const auto *name = getDesignatorNameIfDataRef(varDesignator->value())) {
-      address = converter.getSymbolAddress(*name->symbol);
-    }
-  }
-
+  mlir::Value address = fir::getBase(converter.genExprAddr(
+      *Fortran::semantics::GetExpr(assignmentStmtVariable), stmtCtx));
+  // If no hint clause is specified, the effect is as if
+  // hint(omp_sync_hint_none) had been specified.
+  mlir::IntegerAttr hint = nullptr;
+  mlir::omp::ClauseMemoryOrderKindAttr memory_order = nullptr;
   genOmpAtomicHintAndMemoryOrderClauses(converter, leftHandClauseList, hint,
                                         memory_order);
   genOmpAtomicHintAndMemoryOrderClauses(converter, rightHandClauseList, hint,
@@ -878,12 +872,7 @@ static void genOmpAtomicRead(Fortran::lower::AbstractConverter &converter,
                              const Fortran::parser::OmpAtomicRead &atomicRead) {
   auto &firOpBuilder = converter.getFirOpBuilder();
   auto currentLocation = converter.getCurrentLocation();
-  mlir::Value to_address;
-  mlir::Value from_address;
-  // If no hint clause is specified, the effect is as if
-  // hint(omp_sync_hint_none) had been specified.
-  mlir::IntegerAttr hint = nullptr;
-  mlir::omp::ClauseMemoryOrderKindAttr memory_order = nullptr;
+  // Get the address of atomic read operands.
   const Fortran::parser::OmpAtomicClauseList &rightHandClauseList =
       std::get<2>(atomicRead.t);
   const Fortran::parser::OmpAtomicClauseList &leftHandClauseList =
@@ -892,23 +881,15 @@ static void genOmpAtomicRead(Fortran::lower::AbstractConverter &converter,
       std::get<Fortran::parser::Expr>(std::get<3>(atomicRead.t).statement.t);
   const auto &assignmentStmtVariable = std::get<Fortran::parser::Variable>(
       std::get<3>(atomicRead.t).statement.t);
-  if (auto exprDesignator = std::get_if<
-          Fortran::common::Indirection<Fortran::parser::Designator>>(
-          &assignmentStmtExpr.u)) {
-    if (const auto *name =
-            getDesignatorNameIfDataRef(exprDesignator->value())) {
-      from_address = converter.getSymbolAddress(*name->symbol);
-    }
-  }
-
-  if (auto varDesignator = std::get_if<
-          Fortran::common::Indirection<Fortran::parser::Designator>>(
-          &assignmentStmtVariable.u)) {
-    if (const auto *name = getDesignatorNameIfDataRef(varDesignator->value())) {
-      to_address = converter.getSymbolAddress(*name->symbol);
-    }
-  }
-
+  Fortran::lower::StatementContext stmtCtx;
+  mlir::Value from_address = fir::getBase(converter.genExprAddr(
+      *Fortran::semantics::GetExpr(assignmentStmtExpr), stmtCtx));
+  mlir::Value to_address = fir::getBase(converter.genExprAddr(
+      *Fortran::semantics::GetExpr(assignmentStmtVariable), stmtCtx));
+  // If no hint clause is specified, the effect is as if
+  // hint(omp_sync_hint_none) had been specified.
+  mlir::IntegerAttr hint = nullptr;
+  mlir::omp::ClauseMemoryOrderKindAttr memory_order = nullptr;
   genOmpAtomicHintAndMemoryOrderClauses(converter, leftHandClauseList, hint,
                                         memory_order);
   genOmpAtomicHintAndMemoryOrderClauses(converter, rightHandClauseList, hint,

diff  --git a/flang/test/Lower/OpenMP/atomic-read.f90 b/flang/test/Lower/OpenMP/atomic-read.f90
index 36ad61ff0e901..0f8a042be40ae 100644
--- a/flang/test/Lower/OpenMP/atomic-read.f90
+++ b/flang/test/Lower/OpenMP/atomic-read.f90
@@ -44,3 +44,35 @@ program OmpAtomic
        g = h
 end program OmpAtomic
 
+! Test lowering atomic read for pointer variables.
+! Please notice to use %[[VAL_4]] and %[[VAL_1]] for operands of atomic
+! operation, instead of %[[VAL_3]] and %[[VAL_0]].
+
+!CHECK-LABEL: func.func @_QPatomic_read_pointer() {
+!CHECK:         %[[VAL_0:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "x", uniq_name = "_QFatomic_read_pointerEx"}
+!CHECK:         %[[VAL_1:.*]] = fir.alloca !fir.ptr<i32> {uniq_name = "_QFatomic_read_pointerEx.addr"}
+!CHECK:         %[[VAL_2:.*]] = fir.zero_bits !fir.ptr<i32>
+!CHECK:         fir.store %[[VAL_2]] to %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:         %[[VAL_3:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "y", uniq_name = "_QFatomic_read_pointerEy"}
+!CHECK:         %[[VAL_4:.*]] = fir.alloca !fir.ptr<i32> {uniq_name = "_QFatomic_read_pointerEy.addr"}
+!CHECK:         %[[VAL_5:.*]] = fir.zero_bits !fir.ptr<i32>
+!CHECK:         fir.store %[[VAL_5]] to %[[VAL_4]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:         %[[VAL_6:.*]] = fir.load %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:         %[[VAL_7:.*]] = fir.load %[[VAL_4]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:         omp.atomic.read %[[VAL_7]] = %[[VAL_6]]   : !fir.ptr<i32>
+!CHECK:         %[[VAL_8:.*]] = fir.load %[[VAL_4]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:         %[[VAL_9:.*]] = fir.load %[[VAL_8]] : !fir.ptr<i32>
+!CHECK:         %[[VAL_10:.*]] = fir.load %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:         fir.store %[[VAL_9]] to %[[VAL_10]] : !fir.ptr<i32>
+!CHECK:         return
+!CHECK:       }
+
+subroutine atomic_read_pointer()
+  integer, pointer :: x, y
+
+  !$omp atomic read
+    y = x
+
+  x = y
+end
+

diff  --git a/flang/test/Lower/OpenMP/atomic-write.f90 b/flang/test/Lower/OpenMP/atomic-write.f90
index 1a9c264403563..ed8209092ec26 100644
--- a/flang/test/Lower/OpenMP/atomic-write.f90
+++ b/flang/test/Lower/OpenMP/atomic-write.f90
@@ -36,3 +36,30 @@ program OmpAtomicWrite
         y = 10*x + z/2
 end program OmpAtomicWrite
 
+! Test lowering atomic read for pointer variables.
+! Please notice to use %[[VAL_1]] for operands of atomic operation, instead
+! of %[[VAL_0]].
+
+!CHECK-LABEL: func.func @_QPatomic_write_pointer() {
+!CHECK:         %[[VAL_0:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "x", uniq_name = "_QFatomic_write_pointerEx"}
+!CHECK:         %[[VAL_1:.*]] = fir.alloca !fir.ptr<i32> {uniq_name = "_QFatomic_write_pointerEx.addr"}
+!CHECK:         %[[VAL_2:.*]] = fir.zero_bits !fir.ptr<i32>
+!CHECK:         fir.store %[[VAL_2]] to %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:         %[[VAL_3:.*]] = arith.constant 1 : i32
+!CHECK:         %[[VAL_4:.*]] = fir.load %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:         omp.atomic.write %[[VAL_4]] = %[[VAL_3]]   : !fir.ptr<i32>, i32
+!CHECK:         %[[VAL_5:.*]] = arith.constant 2 : i32
+!CHECK:         %[[VAL_6:.*]] = fir.load %[[VAL_1]] : !fir.ref<!fir.ptr<i32>>
+!CHECK:         fir.store %[[VAL_5]] to %[[VAL_6]] : !fir.ptr<i32>
+!CHECK:         return
+!CHECK:       }
+
+subroutine atomic_write_pointer()
+  integer, pointer :: x
+
+  !$omp atomic write
+    x = 1
+
+  x = 2
+end
+


        


More information about the flang-commits mailing list