[Mlir-commits] [flang] [mlir] [flang][OpenMP] Delayed privatization for variables with `equivalence` association (PR #100531)

Kareem Ergawy llvmlistbot at llvm.org
Wed Jul 31 21:46:41 PDT 2024


https://github.com/ergawy updated https://github.com/llvm/llvm-project/pull/100531

>From 067f4c7db932d71b2d1bd8e1018745e6c3308aff Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Thu, 25 Jul 2024 02:45:30 -0500
Subject: [PATCH 1/2] [flang][OpenMP] Delayed privatization for variable with
 `equivalence` association

Handles variables that are storage associated via `equivalence`. The
problem is that these variables are declared as `fir.ptr`s while their
privatized storage is declared as `fir.ref` which was triggering a
validation error in the OpenMP dialect.
---
 .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 14 +++++--
 .../DelayedPrivatization/equivalence.f90      | 37 +++++++++++++++++++
 mlir/test/Target/LLVMIR/openmp-private.mlir   | 35 ++++++++++++++++++
 3 files changed, 83 insertions(+), 3 deletions(-)
 create mode 100644 flang/test/Lower/OpenMP/DelayedPrivatization/equivalence.f90

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 78f83ede570a6..71c6d1ecb797e 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -549,9 +549,17 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
       symTable->addSymbol(*sym, localExV);
       symTable->pushScope();
       cloneSymbol(sym);
-      firOpBuilder.create<mlir::omp::YieldOp>(
-          hsb.getAddr().getLoc(),
-          symTable->shallowLookupSymbol(*sym).getAddr());
+      mlir::Value cloneAddr = symTable->shallowLookupSymbol(*sym).getAddr();
+      mlir::Type cloneType = cloneAddr.getType();
+
+      mlir::Value yieldedValue =
+          (symType == cloneType) ? cloneAddr
+                                 : firOpBuilder.createConvert(
+                                       cloneAddr.getLoc(), symType, cloneAddr);
+
+      auto yieldOp = firOpBuilder.create<mlir::omp::YieldOp>(
+          hsb.getAddr().getLoc(), yieldedValue);
+      yieldOp.getOperand(0).getType();
       symTable->popScope();
     }
 
diff --git a/flang/test/Lower/OpenMP/DelayedPrivatization/equivalence.f90 b/flang/test/Lower/OpenMP/DelayedPrivatization/equivalence.f90
new file mode 100644
index 0000000000000..1cbbcdcd0e4fd
--- /dev/null
+++ b/flang/test/Lower/OpenMP/DelayedPrivatization/equivalence.f90
@@ -0,0 +1,37 @@
+! Test delayed privatization for variables that are storage associated via `EQUIVALENCE`.
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN:   -o - %s 2>&1 | FileCheck %s
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %s 2>&1 \
+! RUN:   | FileCheck %s
+
+subroutine private_common
+  real x, y
+  equivalence (x,y)
+  !$omp parallel firstprivate(x)
+    x = 3.14
+  !$omp end parallel
+end subroutine
+
+! CHECK:  omp.private {type = firstprivate} @[[X_PRIVATIZER:.*]] : ![[X_TYPE:fir.ptr<f32>]] alloc {
+! CHECK:  ^bb0(%{{.*}}: ![[X_TYPE]]):
+! CHECK:    %[[PRIV_ALLOC:.*]] = fir.alloca f32 {bindc_name = "x", {{.*}}}
+! CHECK:    %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOC]] {{{.*}}} : (![[PRIV_TYPE:fir.ref<f32>]]) -> ({{.*}})
+! CHECK:    %[[PRIV_CONV:.*]] = fir.convert %[[PRIV_DECL]]#0 : (![[PRIV_TYPE]]) -> ![[X_TYPE]]
+! CHECK:    omp.yield(%[[PRIV_CONV]] : ![[X_TYPE]])
+! CHECK:  } copy {
+! CHECK:  ^bb0(%[[ORIG_PTR:.*]]: ![[X_TYPE]], %[[PRIV_REF:.*]]: ![[X_TYPE]]):
+! CHECK:    %[[ORIG_VAL:.*]] = fir.load %[[ORIG_PTR]] : !fir.ptr<f32>
+! CHECK:    hlfir.assign %[[ORIG_VAL]] to %[[PRIV_REF]] temporary_lhs : f32, ![[X_TYPE]]
+! CHECK:    omp.yield(%[[PRIV_REF]] : ![[X_TYPE]])
+! CHECK:  }
+
+! CHECK:  func.func @_QPprivate_common() {
+! CHECK:    omp.parallel private(@[[X_PRIVATIZER]] %{{.*}}#0 -> %[[PRIV_ARG:.*]] : ![[X_TYPE]]) {
+! CHECK:      %[[REG_DECL:.*]]:2 = hlfir.declare %[[PRIV_ARG]] {{{.*}}} : (![[X_TYPE]]) -> ({{.*}})
+! CHECK:      %[[CST:.*]] = arith.constant {{.*}}
+! CHECK:      hlfir.assign %[[CST]] to %[[REG_DECL]]#0 : {{.*}}
+! CHECK:      omp.terminator
+! CHECK:    }
+! CHECK:    return
+! CHECK:  }
diff --git a/mlir/test/Target/LLVMIR/openmp-private.mlir b/mlir/test/Target/LLVMIR/openmp-private.mlir
index 5924377581db6..6bc73fc6f102b 100644
--- a/mlir/test/Target/LLVMIR/openmp-private.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-private.mlir
@@ -234,3 +234,38 @@ omp.declare_reduction @reducer.part : !llvm.ptr init {
 ^bb0(%arg0: !llvm.ptr):
   omp.yield
 }
+
+// -----
+
+llvm.func @_QPequivalence() {
+  %0 = llvm.mlir.constant(1 : i64) : i64
+  %1 = llvm.alloca %0 x !llvm.array<4 x i8> : (i64) -> !llvm.ptr
+  %2 = llvm.mlir.constant(0 : index) : i64
+  %3 = llvm.getelementptr %1[0, %2] : (!llvm.ptr, i64) -> !llvm.ptr, !llvm.array<4 x i8>
+  omp.parallel private(@_QFequivalenceEx_firstprivate_ptr_f32 %3 -> %arg0 : !llvm.ptr) {
+    %4 = llvm.mlir.constant(3.140000e+00 : f32) : f32
+    llvm.store %4, %arg0 : f32, !llvm.ptr
+    omp.terminator
+  }
+  llvm.return
+}
+
+omp.private {type = firstprivate} @_QFequivalenceEx_firstprivate_ptr_f32 : !llvm.ptr alloc {
+^bb0(%arg0: !llvm.ptr):
+  %0 = llvm.mlir.constant(1 : i64) : i64
+  %1 = llvm.alloca %0 x f32 {bindc_name = "x", pinned} : (i64) -> !llvm.ptr
+  omp.yield(%1 : !llvm.ptr)
+} copy {
+^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+  %0 = llvm.load %arg0 : !llvm.ptr -> f32
+  llvm.store %0, %arg1 : f32, !llvm.ptr
+  omp.yield(%arg1 : !llvm.ptr)
+}
+
+// CHECK: define internal void @_QPequivalence..omp_par
+// CHECK:   %[[PRIV_ALLOC:.*]] = alloca float, i64 1, align 4
+// CHECK:   %[[HOST_VAL:.*]] = load float, ptr %{{.*}}, align 4
+// Test that we initialzie the firstprivate variable.
+// CHECK:   store float %[[HOST_VAL]], ptr %[[PRIV_ALLOC]], align 4
+// Test that we inlined the body of the parallel region.
+// CHECK:   store float 0x{{.*}}, ptr %[[PRIV_ALLOC]], align 4

>From 73855c0a69f79b2bb8dec5d07787809d1b4f541b Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Wed, 31 Jul 2024 07:38:18 -0500
Subject: [PATCH 2/2] handle review comments

---
 flang/lib/Lower/OpenMP/DataSharingProcessor.cpp | 5 ++---
 mlir/test/Target/LLVMIR/openmp-private.mlir     | 3 ++-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 71c6d1ecb797e..d785d24284352 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -557,9 +557,8 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
                                  : firOpBuilder.createConvert(
                                        cloneAddr.getLoc(), symType, cloneAddr);
 
-      auto yieldOp = firOpBuilder.create<mlir::omp::YieldOp>(
-          hsb.getAddr().getLoc(), yieldedValue);
-      yieldOp.getOperand(0).getType();
+      firOpBuilder.create<mlir::omp::YieldOp>(hsb.getAddr().getLoc(),
+                                              yieldedValue);
       symTable->popScope();
     }
 
diff --git a/mlir/test/Target/LLVMIR/openmp-private.mlir b/mlir/test/Target/LLVMIR/openmp-private.mlir
index 6bc73fc6f102b..e76f7e4d40f7a 100644
--- a/mlir/test/Target/LLVMIR/openmp-private.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-private.mlir
@@ -263,9 +263,10 @@ omp.private {type = firstprivate} @_QFequivalenceEx_firstprivate_ptr_f32 : !llvm
 }
 
 // CHECK: define internal void @_QPequivalence..omp_par
+// CHECK-NOT: define {{.*}} @{{.*}}
 // CHECK:   %[[PRIV_ALLOC:.*]] = alloca float, i64 1, align 4
 // CHECK:   %[[HOST_VAL:.*]] = load float, ptr %{{.*}}, align 4
-// Test that we initialzie the firstprivate variable.
+// Test that we initialize the firstprivate variable.
 // CHECK:   store float %[[HOST_VAL]], ptr %[[PRIV_ALLOC]], align 4
 // Test that we inlined the body of the parallel region.
 // CHECK:   store float 0x{{.*}}, ptr %[[PRIV_ALLOC]], align 4



More information about the Mlir-commits mailing list