[Mlir-commits] [flang] [mlir] [Flang][MLIR][OpenMP] - Add support for firstprivate when translating omp.target ops from MLIR to LLVMIR (PR #131213)
Pranav Bhandarkar
llvmlistbot at llvm.org
Thu Mar 13 13:57:07 PDT 2025
https://github.com/bhandarkar-pranav updated https://github.com/llvm/llvm-project/pull/131213
>From 947b4f1ebe3f3ba10e8ae6e08affc54cedededdd Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Mon, 24 Feb 2025 14:21:01 -0600
Subject: [PATCH 1/8] Some debugging prints
---
flang/lib/Lower/OpenMP/DataSharingProcessor.cpp | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 781b0dfceff9e..df213a3125585 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -25,6 +25,8 @@
#include "flang/Semantics/attr.h"
#include "flang/Semantics/tools.h"
+#define DEBUG_TYPE "flang-data-sharing"
+#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
namespace Fortran {
namespace lower {
namespace omp {
@@ -513,6 +515,11 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
hlfir::Entity entity{hsb.getAddr()};
bool cannotHaveNonDefaultLowerBounds = !entity.mayHaveNonDefaultLowerBounds();
+ LLVM_DEBUG(PDBGS() << "\n***********doPrivatize************\n");
+ LLVM_DEBUG(PDBGS() << "SymbolBox for privatization is \n"; PDBGS() << hsb;);
+ LLVM_DEBUG(PDBGS() << "Symbol is " << *sym << "\n");
+ LLVM_DEBUG(PDBGS() << "Symbol's address is " << hsb.getAddr() << "\n");
+ LLVM_DEBUG(PDBGS() << "Address Type is " << hsb.getAddr().getType() << "\n");
mlir::Location symLoc = hsb.getAddr().getLoc();
std::string privatizerName = sym->name().ToString() + ".privatizer";
bool isFirstPrivate = sym->test(semantics::Symbol::Flag::OmpFirstPrivate);
@@ -526,12 +533,13 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
if (!mlir::isa<fir::PointerType>(poly.getEleTy()) && isFirstPrivate)
TODO(symLoc, "create polymorphic host associated copy");
}
-
+ LLVM_DEBUG(PDBGS() << "allocType = " << allocType << "\n");
// fir.array<> cannot be converted to any single llvm type and fir helpers
// are not available in openmp to llvmir translation so we cannot generate
// an alloca for a fir.array type there. Get around this by boxing all
// arrays.
if (mlir::isa<fir::SequenceType>(allocType)) {
+ LLVM_DEBUG(PDBGS() << allocType << "Is a SequenceType\n");
entity = genVariableBox(symLoc, firOpBuilder, entity);
privVal = entity.getBase();
allocType = privVal.getType();
@@ -548,7 +556,10 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
}
mlir::Type argType = privVal.getType();
-
+ LLVM_DEBUG(PDBGS() << "** Summary **\n");
+ LLVM_DEBUG(PDBGS() << "allocType = " << allocType << "\n");
+ LLVM_DEBUG(PDBGS() << "privVal = " << privVal << "\n");
+ LLVM_DEBUG(PDBGS() << "privVal.getType() = " << privVal.getType() << "\n");
mlir::omp::PrivateClauseOp privatizerOp = [&]() {
auto moduleOp = firOpBuilder.getModule();
auto uniquePrivatizerName = fir::getTypeAsString(
>From 2fe2a8d5a600c998fb5abd649af972aec2f90e54 Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Thu, 27 Feb 2025 15:23:34 -0600
Subject: [PATCH 2/8] Some more debugging prints
---
.../lib/Lower/OpenMP/DataSharingProcessor.cpp | 21 ++++++++++++-------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index df213a3125585..20a765b3fa5ce 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -27,6 +27,7 @@
#define DEBUG_TYPE "flang-data-sharing"
#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
+#define PDBGS_NL() (llvm::dbgs() << "\n")
namespace Fortran {
namespace lower {
namespace omp {
@@ -515,16 +516,19 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
hlfir::Entity entity{hsb.getAddr()};
bool cannotHaveNonDefaultLowerBounds = !entity.mayHaveNonDefaultLowerBounds();
- LLVM_DEBUG(PDBGS() << "\n***********doPrivatize************\n");
- LLVM_DEBUG(PDBGS() << "SymbolBox for privatization is \n"; PDBGS() << hsb;);
- LLVM_DEBUG(PDBGS() << "Symbol is " << *sym << "\n");
- LLVM_DEBUG(PDBGS() << "Symbol's address is " << hsb.getAddr() << "\n");
- LLVM_DEBUG(PDBGS() << "Address Type is " << hsb.getAddr().getType() << "\n");
+ LLVM_DEBUG(PDBGS_NL());
+ LLVM_DEBUG(PDBGS() << "***********doPrivatize************\n");
+ LLVM_DEBUG(PDBGS() << "Symbol (sym): " << *sym << "\n");
+ LLVM_DEBUG(PDBGS() << "SymbolBox: (hsb): " << hsb);
+ LLVM_DEBUG(PDBGS() << "Address: (hsb.getAddr()): " << hsb.getAddr() << "\n");
+ LLVM_DEBUG(PDBGS() << "Address Type (hsb.getAddr.getType()): "
+ << hsb.getAddr().getType() << "\n");
mlir::Location symLoc = hsb.getAddr().getLoc();
std::string privatizerName = sym->name().ToString() + ".privatizer";
bool isFirstPrivate = sym->test(semantics::Symbol::Flag::OmpFirstPrivate);
mlir::Value privVal = hsb.getAddr();
+ LLVM_DEBUG(PDBGS() << "privVal: " << privVal << "\n");
mlir::Type allocType = privVal.getType();
if (!mlir::isa<fir::PointerType>(privVal.getType()))
allocType = fir::unwrapRefType(privVal.getType());
@@ -539,7 +543,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
// an alloca for a fir.array type there. Get around this by boxing all
// arrays.
if (mlir::isa<fir::SequenceType>(allocType)) {
- LLVM_DEBUG(PDBGS() << allocType << "Is a SequenceType\n");
+ LLVM_DEBUG(PDBGS() << "(flow_msg): " << allocType << "is a SequenceType\n");
entity = genVariableBox(symLoc, firOpBuilder, entity);
privVal = entity.getBase();
allocType = privVal.getType();
@@ -547,6 +551,8 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
if (mlir::isa<fir::BaseBoxType>(privVal.getType())) {
// Boxes should be passed by reference into nested regions:
+ LLVM_DEBUG(PDBGS() << "(flow_msg): " << privVal.getType()
+ << "is a BaseBoxType\n");
auto oldIP = firOpBuilder.saveInsertionPoint();
firOpBuilder.setInsertionPointToStart(firOpBuilder.getAllocaBlock());
auto alloca = firOpBuilder.create<fir::AllocaOp>(symLoc, privVal.getType());
@@ -556,8 +562,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
}
mlir::Type argType = privVal.getType();
- LLVM_DEBUG(PDBGS() << "** Summary **\n");
- LLVM_DEBUG(PDBGS() << "allocType = " << allocType << "\n");
+ LLVM_DEBUG(PDBGS() << "allocType : " << allocType << "\n");
LLVM_DEBUG(PDBGS() << "privVal = " << privVal << "\n");
LLVM_DEBUG(PDBGS() << "privVal.getType() = " << privVal.getType() << "\n");
mlir::omp::PrivateClauseOp privatizerOp = [&]() {
>From a80a9afe37dc69d2b90d131d2157c4c71248258c Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Thu, 27 Feb 2025 15:24:07 -0600
Subject: [PATCH 3/8] remove firstprivate todos
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 2 +-
.../OpenMP/OpenMPToLLVMIRTranslation.cpp | 20 +++++++++----------
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index b1568cc12a05a..7f85da162bbb3 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1666,7 +1666,7 @@ static void genTargetClauses(
cp.processNowait(clauseOps);
cp.processThreadLimit(stmtCtx, clauseOps);
- cp.processTODO<clause::Allocate, clause::Defaultmap, clause::Firstprivate,
+ cp.processTODO<clause::Allocate, clause::Defaultmap, // clause::Firstprivate,
clause::InReduction, clause::UsesAllocators>(
loc, llvm::omp::Directive::OMPD_target);
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 32c7c501d03c3..fdae8b88be676 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -214,16 +214,16 @@ static LogicalResult checkImplementationStatus(Operation &op) {
// Privatization clauses are supported, except on some situations, so we
// need to check here whether any of these unsupported cases are being
// translated.
- if (std::optional<ArrayAttr> privateSyms = op.getPrivateSyms()) {
- for (Attribute privatizerNameAttr : *privateSyms) {
- omp::PrivateClauseOp privatizer = findPrivatizer(
- op.getOperation(), cast<SymbolRefAttr>(privatizerNameAttr));
-
- if (privatizer.getDataSharingType() ==
- omp::DataSharingClauseType::FirstPrivate)
- result = todo("firstprivate");
- }
- }
+ // if (std::optional<ArrayAttr> privateSyms = op.getPrivateSyms()) {
+ // for (Attribute privatizerNameAttr : *privateSyms) {
+ // omp::PrivateClauseOp privatizer = findPrivatizer(
+ // op.getOperation(), cast<SymbolRefAttr>(privatizerNameAttr));
+
+ // if (privatizer.getDataSharingType() ==
+ // omp::DataSharingClauseType::FirstPrivate)
+ // result = todo("firstprivate");
+ // }
+ // }
} else {
if (!op.getPrivateVars().empty() || op.getPrivateSyms())
result = todo("privatization");
>From 4aa6fd658f72d235ad94242bc1b32f69dee752d5 Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Thu, 6 Mar 2025 13:47:17 -0600
Subject: [PATCH 4/8] some debug messages in OpenMPToLLVMIRTranslation
---
.../Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index fdae8b88be676..544f21cd65645 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -44,6 +44,8 @@
#include <optional>
#include <utility>
+#define DEBUG_TYPE "openmp-to-llvmir"
+#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
using namespace mlir;
namespace {
@@ -4793,6 +4795,17 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
collectPrivatizationDecls(targetOp, privateDecls);
for (mlir::Value privateVar : targetOp.getPrivateVars())
mlirPrivateVars.push_back(privateVar);
+ LLVM_DEBUG(PDBGS() << "Types of each PrivateDecl\n");
+ LLVM_DEBUG(
+ for (omp::PrivateClauseOp &privDecl : privateDecls) {
+ if (!privDecl.readsFromMold())
+ continue;
+ mlir::Type mlirType = privDecl.getType();
+ llvm::Type *llvmType = moduleTranslation.convertType(mlirType);
+ PDBGS() << "MLIR Type = " << mlirType << "\n";
+ PDBGS() << "LLVM Type = " << *llvmType << "\n";
+ }
+ );
llvm::Expected<llvm::BasicBlock *> afterAllocas = allocatePrivateVars(
builder, moduleTranslation, privateBlockArgs, privateDecls,
>From 9f0bf072330675c593547a87f75a521ffd7a35c5 Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Mon, 10 Mar 2025 15:26:02 -0500
Subject: [PATCH 5/8] Fix the definition of needsMap in PrivateClauseOp (omp
dialect)
When privatizing a variable, we need a corresponding map when
its PrivateClauseOp reads from the Mold arg. This is possible
when either the init reads from the mold or if the copy region
is not empty. Until now, we checked only the former case. This
patch adds the check for the second case as well.
---
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index f5a8a7ba04375..bac62da067db5 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -81,7 +81,7 @@ def PrivateClauseOp : OpenMP_Op<"private", [IsolatedFromAbove, RecipeInterface]>
There are no restrictions on the body except for:
- The `dealloc` regions has a single argument.
- - The `init & `copy` regions have 2 arguments.
+ - The `init` & `copy` regions have 2 arguments.
- All three regions are terminated by `omp.yield` ops.
The above restrictions and other obvious restrictions (e.g. verifying the
type of yielded values) are verified by the custom op verifier. The actual
@@ -90,15 +90,15 @@ def PrivateClauseOp : OpenMP_Op<"private", [IsolatedFromAbove, RecipeInterface]>
Instances of this op would then be used by ops that model directives that
accept data-sharing attribute clauses.
- The $sym_name attribute provides a symbol by which the privatizer op can be
+ The `sym_name` attribute provides a symbol by which the privatizer op can be
referenced by other dialect ops.
- The $type attribute is the type of the value being privatized. This type
+ The `type` attribute is the type of the value being privatized. This type
will be implicitly allocated in MLIR->LLVMIR conversion and passed as the
second argument to the init region. Therefore the type of arguments to
- the regions should be a type which represents a pointer to $type.
+ the regions should be a type which represents a pointer to `type`.
- The $data_sharing_type attribute specifies whether privatizer corresponds
+ The `data_sharing_type` attribute specifies whether privatizer corresponds
to a `private` or a `firstprivate` clause.
}];
@@ -161,9 +161,10 @@ def PrivateClauseOp : OpenMP_Op<"private", [IsolatedFromAbove, RecipeInterface]>
/// needsMap returns true if the value being privatized should additionally
/// be mapped to the target region using a MapInfoOp. This is most common
/// when an allocatable is privatized. In such cases, the descriptor is used
- /// in privatization and needs to be mapped on to the device.
+ /// in privatization and needs to be mapped on to the device. The use of
+ /// firstprivate also causes the need to map the host variable to the device.
bool needsMap() {
- return initReadsFromMold();
+ return readsFromMold();
}
/// Get the type for arguments to nested regions. This should
>From 62144485dd777e6266d0dd91ea633c99605f1fe4 Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Thu, 13 Mar 2025 14:51:43 -0500
Subject: [PATCH 6/8] Add some lit tests
---
.../Target/LLVMIR/openmp-target-private.mlir | 112 ++++++++++++++----
1 file changed, 92 insertions(+), 20 deletions(-)
diff --git a/mlir/test/Target/LLVMIR/openmp-target-private.mlir b/mlir/test/Target/LLVMIR/openmp-target-private.mlir
index f97360e2c6e84..0e651482647f0 100644
--- a/mlir/test/Target/LLVMIR/openmp-target-private.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-target-private.mlir
@@ -18,11 +18,6 @@ llvm.func @target_map_single_private() attributes {fir.internal_name = "_QPtarge
}
llvm.return
}
-// CHECK: define internal void @__omp_offloading_
-// CHECK-NOT: define {{.*}}
-// CHECK: %[[PRIV_ALLOC:.*]] = alloca i32, align 4
-// CHECK: %[[ADD:.*]] = add i32 {{.*}}, 10
-// CHECK: store i32 %[[ADD]], ptr %[[PRIV_ALLOC]], align 4
omp.private {type = private} @n.privatizer : f32
@@ -50,16 +45,6 @@ llvm.func @target_map_2_privates() attributes {fir.internal_name = "_QPtarget_ma
}
-// CHECK: define internal void @__omp_offloading_
-// CHECK: %[[PRIV_I32_ALLOC:.*]] = alloca i32, align 4
-// CHECK: %[[PRIV_FLOAT_ALLOC:.*]] = alloca float, align 4
-// CHECK: %[[ADD_I32:.*]] = add i32 {{.*}}, 10
-// CHECK: store i32 %[[ADD_I32]], ptr %[[PRIV_I32_ALLOC]], align 4
-// CHECK: %[[LOAD_I32_AGAIN:.*]] = load i32, ptr %[[PRIV_I32_ALLOC]], align 4
-// CHECK: %[[CAST_TO_FLOAT:.*]] = sitofp i32 %[[LOAD_I32_AGAIN]] to float
-// CHECK: %[[ADD_FLOAT:.*]] = fadd contract float %[[CAST_TO_FLOAT]], 1.100000e+01
-// CHECK: store float %[[ADD_FLOAT]], ptr %[[PRIV_FLOAT_ALLOC]], align 4
-
// An entirely artifical privatizer that is meant to check multi-block
// privatizers. The idea here is to prove that we set the correct
// insertion points for the builder when generating, first, LLVM IR for the
@@ -79,10 +64,6 @@ llvm.func @target_op_private_multi_block(%arg0: !llvm.ptr) {
}
llvm.return
}
-// CHECK: define internal void @__omp_offloading_
-// CHECK: %[[PRIV_ALLOC:.*]] = alloca float, align 4
-// CHECK: %[[PHI_ALLOCA:.*]] = phi ptr [ %[[PRIV_ALLOC]], {{.*}} ]
-// CHECK: %[[RESULT:.*]] = load float, ptr %[[PHI_ALLOCA]], align 4
// Descriptors are needed for CHARACTER arrays and their type is
// !fir.boxchar<KIND>. When such arrays are used in the private construct, the
@@ -165,10 +146,101 @@ llvm.func @llvm.memmove.p0.p0.i64(!llvm.ptr, !llvm.ptr, i64, i1) attributes {sym
-// CHECK: define internal void @__omp_offloading_{{.*}}(ptr %{{[^,]+}}, ptr %[[MAPPED_ARG:.*]]) {
+omp.private {type = firstprivate} @sf.firstprivate : f32 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)
+}
+omp.private {type = firstprivate} @sv.firstprivate : i32 copy {
+^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+ %0 = llvm.load %arg0 : !llvm.ptr -> i32
+ llvm.store %0, %arg1 : i32, !llvm.ptr
+ omp.yield(%arg1 : !llvm.ptr)
+}
+llvm.func @target_simple_() attributes {fir.internal_name = "_QPtarget_simple"} {
+ %0 = llvm.mlir.constant(1 : i64) : i64
+ %sv = llvm.alloca %0 x i32 {bindc_name = "sv"} : (i64) -> !llvm.ptr
+ %sf = llvm.alloca %0 x f32 {bindc_name = "sf"} : (i64) -> !llvm.ptr
+ %4 = llvm.mlir.constant(1 : i64) : i64
+ %5 = llvm.mlir.constant(1 : i64) : i64
+ %6 = omp.map.info var_ptr(%sv : !llvm.ptr, i32) map_clauses(to) capture(ByRef) -> !llvm.ptr
+ %7 = omp.map.info var_ptr(%sf : !llvm.ptr, f32) map_clauses(to) capture(ByRef) -> !llvm.ptr
+ omp.target map_entries(%6 -> %arg0, %7 -> %arg1 : !llvm.ptr, !llvm.ptr) private(@sv.firstprivate %sv -> %arg2 [map_idx=0], @sf.firstprivate %sf -> %arg3 [map_idx=1] : !llvm.ptr, !llvm.ptr) {
+ %8 = llvm.mlir.constant(2.000000e+00 : f64) : f64
+ %9 = llvm.mlir.constant(10 : i32) : i32
+ %10 = llvm.load %arg2 : !llvm.ptr -> i32
+ %11 = llvm.add %10, %9 : i32
+ llvm.store %11, %arg2 : i32, !llvm.ptr
+ %12 = llvm.load %arg3 : !llvm.ptr -> f32
+ %13 = llvm.fpext %12 : f32 to f64
+ %14 = llvm.fadd %13, %8 {fastmathFlags = #llvm.fastmath<contract>} : f64
+ %15 = llvm.fptrunc %14 : f64 to f32
+ llvm.store %15, %arg3 : f32, !llvm.ptr
+ omp.terminator
+ }
+ llvm.return
+}
+// CHECK: define void @target_map_single_private() {
+// CHECK: call void @__omp_offloading_[[MAP_SINGLE_PRIVATE_OFFLOADED_FUNCTION:.*]](ptr {{.*}})
+// CHECK: define void @target_map_2_privates() {
+// CHECK: call void @__omp_offloading_[[MAP_2_PRIVATES_OFFLOADED_FUNCTION:.*]](ptr {{.*}})
+// CHECK: define void @target_op_private_multi_block
+// CHECK: call void @__omp_offloading_[[PRIVATE_MULTI_BLOCK_OFFLOADED_FUNCTION:.*]]()
+// CHECK: define void @target_boxchar_
+// CHECK: call void @__omp_offloading_[[BOXCHAR_OFFLOADED_FUNCTION:.*]](ptr {{.*}}, ptr {{.*}})
+// CHECK: define void @target_simple_()
+// CHECK: call void @__omp_offloading_[[SIMPLE_OFFLOADED_FUNCTION:.*]](ptr {{.*}}, ptr {{.*}})
+
+// CHECK: define internal void @__omp_offloading_[[MAP_SINGLE_PRIVATE_OFFLOADED_FUNCTION]]
+// CHECK: %[[PRIV_ALLOC:.*]] = alloca i32, align 4
+// CHECK: %[[ADD:.*]] = add i32 {{.*}}, 10
+// CHECK: store i32 %[[ADD]], ptr %[[PRIV_ALLOC]], align 4
+
+
+
+
+// CHECK: define internal void @__omp_offloading_[[MAP_2_PRIVATES_OFFLOADED_FUNCTION]]
+// CHECK: %[[PRIV_I32_ALLOC:.*]] = alloca i32, align 4
+// CHECK: %[[PRIV_FLOAT_ALLOC:.*]] = alloca float, align 4
+// CHECK: %[[ADD_I32:.*]] = add i32 {{.*}}, 10
+// CHECK: store i32 %[[ADD_I32]], ptr %[[PRIV_I32_ALLOC]], align 4
+// CHECK: %[[LOAD_I32_AGAIN:.*]] = load i32, ptr %[[PRIV_I32_ALLOC]], align 4
+// CHECK: %[[CAST_TO_FLOAT:.*]] = sitofp i32 %[[LOAD_I32_AGAIN]] to float
+// CHECK: %[[ADD_FLOAT:.*]] = fadd contract float %[[CAST_TO_FLOAT]], 1.100000e+01
+// CHECK: store float %[[ADD_FLOAT]], ptr %[[PRIV_FLOAT_ALLOC]], align 4
+
+// CHECK: define internal void @__omp_offloading_[[PRIVATE_MULTI_BLOCK_OFFLOADED_FUNCTION]]
+// CHECK: %[[PRIV_ALLOC:.*]] = alloca float, align 4
+// CHECK: %[[PHI_ALLOCA:.*]] = phi ptr [ %[[PRIV_ALLOC]], {{.*}} ]
+// CHECK: %[[RESULT:.*]] = load float, ptr %[[PHI_ALLOCA]], align 4
+
+
+// CHECK: define internal void @__omp_offloading_[[BOXCHAR_OFFLOADED_FUNCTION]](ptr %{{[^,]+}}, ptr %[[MAPPED_ARG:.*]]) {
// CHECK: %[[BOXCHAR:.*]] = load { ptr, i64 }, ptr %[[MAPPED_ARG]]
// CHECK: %[[BOXCHAR_PTR:.*]] = extractvalue { ptr, i64 } %[[BOXCHAR]], 0
// CHECK: %[[BOXCHAR_i64:.*]] = extractvalue { ptr, i64 } %[[BOXCHAR]], 1
// CHECK: %[[MEM_ALLOC:.*]] = alloca i8, i64 %[[BOXCHAR_i64]]
// CHECK: %[[PRIV_BOXCHAR0:.*]] = insertvalue { ptr, i64 } undef, ptr %[[MEM_ALLOC]], 0
// CHECK: %[[PRIV_BOXCHAR1:.*]] = insertvalue { ptr, i64 } %[[PRIV_BOXCHAR0]], i64 %[[BOXCHAR_i64]], 1
+
+
+// CHECK: define internal void @__omp_offloading_[[SIMPLE_OFFLOADED_FUNCTION]](ptr %[[SV:.*]], ptr %[[SF:.*]])
+// CHECK: entry:
+// CHECK-NEXT: %[[SV_PRIV_ALLOCA:.*]] = alloca i32, align 4
+// CHECK-NEXT: %[[SF_PRIV_ALLOCA:.*]] = alloca float, align 4
+// CHECK: omp.private.copy:
+// CHECK-NEXT: %[[INIT_SV:.*]] = load i32, ptr %[[SV]], align 4
+// CHECK-NEXT: store i32 %[[INIT_SV]], ptr %[[SV_PRIV_ALLOCA]], align 4
+// CHECK: %[[INIT_SF:.*]] = load float, ptr %[[SF]], align 4
+// CHECK-NEXT store float %[[INIT_SF]], ptr %[[SF_PRIV_ALLOCA]], align 4
+// CHECK: omp.target
+// CHECK: %[[LOAD_SV:.*]] = load i32, ptr %[[SV_PRIV_ALLOCA]], align 4
+// CHECK-NEXT: %[[ADD_SV:.*]] = add i32 %[[LOAD_SV]], 10
+// CHECK-NEXT: store i32 %[[ADD_SV]], ptr %[[SV_PRIV_ALLOCA]], align 4
+// CHECK: %[[LOAD_SF:.*]] = load float, ptr %[[SF_PRIV_ALLOCA]], align 4
+// CHECK-NEXT: %[[SF_EXT:.*]] = fpext float %[[LOAD_SF]] to double
+// CHECK-NEXT: %[[ADD_SF:.*]] = fadd contract double %[[SF_EXT]], 2.000000e+00
+// CHECK-NEXT: %[[TRUNC_SF:.*]] = fptrunc double %[[ADD_SF]] to float
+// CHECK-NEXT: store float %[[TRUNC_SF]], ptr %[[SF_PRIV_ALLOCA]], align 4
+
>From 37b719afda8395c7ed69830b88b22a380f4fe356 Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Thu, 13 Mar 2025 15:49:58 -0500
Subject: [PATCH 7/8] Clean up
---
.../lib/Lower/OpenMP/DataSharingProcessor.cpp | 18 +----------
flang/lib/Lower/OpenMP/OpenMP.cpp | 12 +++++--
.../OpenMP/OpenMPToLLVMIRTranslation.cpp | 32 ++++---------------
3 files changed, 17 insertions(+), 45 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 20a765b3fa5ce..59d4a5563d3de 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -26,8 +26,6 @@
#include "flang/Semantics/tools.h"
#define DEBUG_TYPE "flang-data-sharing"
-#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
-#define PDBGS_NL() (llvm::dbgs() << "\n")
namespace Fortran {
namespace lower {
namespace omp {
@@ -516,19 +514,11 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
hlfir::Entity entity{hsb.getAddr()};
bool cannotHaveNonDefaultLowerBounds = !entity.mayHaveNonDefaultLowerBounds();
- LLVM_DEBUG(PDBGS_NL());
- LLVM_DEBUG(PDBGS() << "***********doPrivatize************\n");
- LLVM_DEBUG(PDBGS() << "Symbol (sym): " << *sym << "\n");
- LLVM_DEBUG(PDBGS() << "SymbolBox: (hsb): " << hsb);
- LLVM_DEBUG(PDBGS() << "Address: (hsb.getAddr()): " << hsb.getAddr() << "\n");
- LLVM_DEBUG(PDBGS() << "Address Type (hsb.getAddr.getType()): "
- << hsb.getAddr().getType() << "\n");
mlir::Location symLoc = hsb.getAddr().getLoc();
std::string privatizerName = sym->name().ToString() + ".privatizer";
bool isFirstPrivate = sym->test(semantics::Symbol::Flag::OmpFirstPrivate);
mlir::Value privVal = hsb.getAddr();
- LLVM_DEBUG(PDBGS() << "privVal: " << privVal << "\n");
mlir::Type allocType = privVal.getType();
if (!mlir::isa<fir::PointerType>(privVal.getType()))
allocType = fir::unwrapRefType(privVal.getType());
@@ -537,13 +527,12 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
if (!mlir::isa<fir::PointerType>(poly.getEleTy()) && isFirstPrivate)
TODO(symLoc, "create polymorphic host associated copy");
}
- LLVM_DEBUG(PDBGS() << "allocType = " << allocType << "\n");
+
// fir.array<> cannot be converted to any single llvm type and fir helpers
// are not available in openmp to llvmir translation so we cannot generate
// an alloca for a fir.array type there. Get around this by boxing all
// arrays.
if (mlir::isa<fir::SequenceType>(allocType)) {
- LLVM_DEBUG(PDBGS() << "(flow_msg): " << allocType << "is a SequenceType\n");
entity = genVariableBox(symLoc, firOpBuilder, entity);
privVal = entity.getBase();
allocType = privVal.getType();
@@ -551,8 +540,6 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
if (mlir::isa<fir::BaseBoxType>(privVal.getType())) {
// Boxes should be passed by reference into nested regions:
- LLVM_DEBUG(PDBGS() << "(flow_msg): " << privVal.getType()
- << "is a BaseBoxType\n");
auto oldIP = firOpBuilder.saveInsertionPoint();
firOpBuilder.setInsertionPointToStart(firOpBuilder.getAllocaBlock());
auto alloca = firOpBuilder.create<fir::AllocaOp>(symLoc, privVal.getType());
@@ -562,9 +549,6 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
}
mlir::Type argType = privVal.getType();
- LLVM_DEBUG(PDBGS() << "allocType : " << allocType << "\n");
- LLVM_DEBUG(PDBGS() << "privVal = " << privVal << "\n");
- LLVM_DEBUG(PDBGS() << "privVal.getType() = " << privVal.getType() << "\n");
mlir::omp::PrivateClauseOp privatizerOp = [&]() {
auto moduleOp = firOpBuilder.getModule();
auto uniquePrivatizerName = fir::getTypeAsString(
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 7f85da162bbb3..0094abe044b86 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1666,13 +1666,19 @@ static void genTargetClauses(
cp.processNowait(clauseOps);
cp.processThreadLimit(stmtCtx, clauseOps);
- cp.processTODO<clause::Allocate, clause::Defaultmap, // clause::Firstprivate,
- clause::InReduction, clause::UsesAllocators>(
- loc, llvm::omp::Directive::OMPD_target);
+ cp.processTODO<clause::Allocate, clause::Defaultmap, clause::InReduction,
+ clause::UsesAllocators>(loc,
+ llvm::omp::Directive::OMPD_target);
// `target private(..)` is only supported in delayed privatization mode.
if (!enableDelayedPrivatizationStaging)
cp.processTODO<clause::Private>(loc, llvm::omp::Directive::OMPD_target);
+
+ // We do not yet have MLIR to LLVMIR translation for privatization in
+ // for deferred target tasks.
+ if (clauseOps.nowait)
+ cp.processTODO<clause::Private, clause::Firstprivate>(
+ loc, llvm::omp::Directive::OMPD_target);
}
static void genTargetDataClauses(
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 544f21cd65645..87b7ab9b6b0a3 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -45,7 +45,6 @@
#include <utility>
#define DEBUG_TYPE "openmp-to-llvmir"
-#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
using namespace mlir;
namespace {
@@ -213,19 +212,9 @@ static LogicalResult checkImplementationStatus(Operation &op) {
};
auto checkPrivate = [&todo](auto op, LogicalResult &result) {
if constexpr (std::is_same_v<std::decay_t<decltype(op)>, omp::TargetOp>) {
- // Privatization clauses are supported, except on some situations, so we
- // need to check here whether any of these unsupported cases are being
- // translated.
- // if (std::optional<ArrayAttr> privateSyms = op.getPrivateSyms()) {
- // for (Attribute privatizerNameAttr : *privateSyms) {
- // omp::PrivateClauseOp privatizer = findPrivatizer(
- // op.getOperation(), cast<SymbolRefAttr>(privatizerNameAttr));
-
- // if (privatizer.getDataSharingType() ==
- // omp::DataSharingClauseType::FirstPrivate)
- // result = todo("firstprivate");
- // }
- // }
+ // Privatization is supported only for include target tasks.
+ if (!op.getPrivateVars().empty() && op.getNowait())
+ result = todo("privatization for deferred target tasks");
} else {
if (!op.getPrivateVars().empty() || op.getPrivateSyms())
result = todo("privatization");
@@ -4795,17 +4784,6 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
collectPrivatizationDecls(targetOp, privateDecls);
for (mlir::Value privateVar : targetOp.getPrivateVars())
mlirPrivateVars.push_back(privateVar);
- LLVM_DEBUG(PDBGS() << "Types of each PrivateDecl\n");
- LLVM_DEBUG(
- for (omp::PrivateClauseOp &privDecl : privateDecls) {
- if (!privDecl.readsFromMold())
- continue;
- mlir::Type mlirType = privDecl.getType();
- llvm::Type *llvmType = moduleTranslation.convertType(mlirType);
- PDBGS() << "MLIR Type = " << mlirType << "\n";
- PDBGS() << "LLVM Type = " << *llvmType << "\n";
- }
- );
llvm::Expected<llvm::BasicBlock *> afterAllocas = allocatePrivateVars(
builder, moduleTranslation, privateBlockArgs, privateDecls,
@@ -4823,6 +4801,10 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
.failed())
return llvm::make_error<PreviouslyReportedError>();
+ if (failed(copyFirstPrivateVars(builder, moduleTranslation, mlirPrivateVars,
+ llvmPrivateVars, privateDecls)))
+ return llvm::make_error<PreviouslyReportedError>();
+
SmallVector<Region *> privateCleanupRegions;
llvm::transform(privateDecls, std::back_inserter(privateCleanupRegions),
[](omp::PrivateClauseOp privatizer) {
>From 4b6fd699c799b1e9353fd47b5e2f0e8da478248a Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar <pranav.bhandarkar at amd.com>
Date: Thu, 13 Mar 2025 15:56:51 -0500
Subject: [PATCH 8/8] More cleanup
---
flang/lib/Lower/OpenMP/DataSharingProcessor.cpp | 1 -
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 2 +-
.../Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp | 3 +--
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 59d4a5563d3de..89f576fdb9331 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -25,7 +25,6 @@
#include "flang/Semantics/attr.h"
#include "flang/Semantics/tools.h"
-#define DEBUG_TYPE "flang-data-sharing"
namespace Fortran {
namespace lower {
namespace omp {
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index bac62da067db5..66c4b51c3b2e3 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -162,7 +162,7 @@ def PrivateClauseOp : OpenMP_Op<"private", [IsolatedFromAbove, RecipeInterface]>
/// be mapped to the target region using a MapInfoOp. This is most common
/// when an allocatable is privatized. In such cases, the descriptor is used
/// in privatization and needs to be mapped on to the device. The use of
- /// firstprivate also causes the need to map the host variable to the device.
+ /// firstprivate also creates the need to map the host variable to the device.
bool needsMap() {
return readsFromMold();
}
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 87b7ab9b6b0a3..680371df93c18 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -44,7 +44,6 @@
#include <optional>
#include <utility>
-#define DEBUG_TYPE "openmp-to-llvmir"
using namespace mlir;
namespace {
@@ -212,7 +211,7 @@ static LogicalResult checkImplementationStatus(Operation &op) {
};
auto checkPrivate = [&todo](auto op, LogicalResult &result) {
if constexpr (std::is_same_v<std::decay_t<decltype(op)>, omp::TargetOp>) {
- // Privatization is supported only for include target tasks.
+ // Privatization is supported only for included target tasks.
if (!op.getPrivateVars().empty() && op.getNowait())
result = todo("privatization for deferred target tasks");
} else {
More information about the Mlir-commits
mailing list