[OpenMP]Update use_device_clause lowering #101703
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TIFitis
requested review from
skatrak,
kiranchandramohan,
raghavendhra,
agozillon and
kiranktp
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-openmp Author: Akash Banerjee (TIFitis) ChangesThis patch updates the use_device_ptr and use_device_addr clauses to use the mapInfoOps for lowering. This allows all the types that are handle by the map clauses such as derived types to also be supported by the use_device_clauses. This is patch 1/2 in a series of patches. Co-authored-by: Raghu Maddhipatla [email protected] Patch is 29.30 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/101703.diff 6 Files Affected:
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 310c0b0b5fb63..0afd316fd42c2 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1072,27 +1072,133 @@ bool ClauseProcessor::processEnter(
}
bool ClauseProcessor::processUseDeviceAddr(
+ Fortran::lower::StatementContext &stmtCtx,
mlir::omp::UseDeviceAddrClauseOps &result,
llvm::SmallVectorImpl<mlir::Type> &useDeviceTypes,
llvm::SmallVectorImpl<mlir::Location> &useDeviceLocs,
- llvm::SmallVectorImpl<const semantics::Symbol *> &useDeviceSyms) const {
- return findRepeatableClause<omp::clause::UseDeviceAddr>(
- [&](const omp::clause::UseDeviceAddr &clause, const parser::CharBlock &) {
- addUseDeviceClause(converter, clause.v, result.useDeviceAddrVars,
- useDeviceTypes, useDeviceLocs, useDeviceSyms);
+ llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> &useDeviceSyms)
+ const {
+ std::map<const Fortran::semantics::Symbol *,
+ llvm::SmallVector<OmpMapMemberIndicesData>>
+ parentMemberIndices;
+ bool clauseFound = findRepeatableClause<omp::clause::UseDeviceAddr>(
+ [&](const omp::clause::UseDeviceAddr &clause,
+ const Fortran::parser::CharBlock &) {
+ const Fortran::parser::CharBlock source;
+ mlir::Location location = converter.genLocation(source);
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ llvm::omp::OpenMPOffloadMappingFlags mapTypeBits =
+ llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO |
+ llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
+ for (const omp::Object &object : clause.v) {
+ llvm::SmallVector<mlir::Value> bounds;
+ std::stringstream asFortran;
+
+ Fortran::lower::AddrAndBoundsInfo info =
+ Fortran::lower::gatherDataOperandAddrAndBounds<
+ mlir::omp::MapBoundsOp, mlir::omp::MapBoundsType>(
+ converter, firOpBuilder, semaCtx, stmtCtx, *object.sym(),
+ object.ref(), location, asFortran, bounds,
+ treatIndexAsSection);
+
+ auto origSymbol = converter.getSymbolAddress(*object.sym());
+ mlir::Value symAddr = info.addr;
+ if (origSymbol && fir::isTypeWithDescriptor(origSymbol.getType()))
+ symAddr = origSymbol;
+
+ // Explicit map captures are captured ByRef by default,
+ // optimisation passes may alter this to ByCopy or other capture
+ // types to optimise
+ mlir::omp::MapInfoOp mapOp = createMapInfoOp(
+ firOpBuilder, location, symAddr,
+ /*varPtrPtr=*/mlir::Value{}, asFortran.str(), bounds,
+ /*members=*/{}, /*membersIndex=*/mlir::DenseIntElementsAttr{},
+ static_cast<
+ std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+ mapTypeBits),
+ mlir::omp::VariableCaptureKind::ByRef, symAddr.getType());
+
+ if (object.sym()->owner().IsDerivedType()) {
+ addChildIndexAndMapToParent(object, parentMemberIndices, mapOp,
+ semaCtx);
+ } else {
+ useDeviceSyms.push_back(object.sym());
+ useDeviceTypes.push_back(symAddr.getType());
+ useDeviceLocs.push_back(symAddr.getLoc());
+ result.useDeviceAddrVars.push_back(mapOp);
+ }
+ }
});
+
+ insertChildMapInfoIntoParent(converter, parentMemberIndices,
+ result.useDeviceAddrVars, useDeviceSyms,
+ &useDeviceTypes, &useDeviceLocs);
+ return clauseFound;
}
bool ClauseProcessor::processUseDevicePtr(
+ Fortran::lower::StatementContext &stmtCtx,
mlir::omp::UseDevicePtrClauseOps &result,
llvm::SmallVectorImpl<mlir::Type> &useDeviceTypes,
llvm::SmallVectorImpl<mlir::Location> &useDeviceLocs,
- llvm::SmallVectorImpl<const semantics::Symbol *> &useDeviceSyms) const {
- return findRepeatableClause<omp::clause::UseDevicePtr>(
- [&](const omp::clause::UseDevicePtr &clause, const parser::CharBlock &) {
- addUseDeviceClause(converter, clause.v, result.useDevicePtrVars,
- useDeviceTypes, useDeviceLocs, useDeviceSyms);
+ llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> &useDeviceSyms)
+ const {
+ std::map<const Fortran::semantics::Symbol *,
+ llvm::SmallVector<OmpMapMemberIndicesData>>
+ parentMemberIndices;
+ bool clauseFound = findRepeatableClause<omp::clause::UseDevicePtr>(
+ [&](const omp::clause::UseDevicePtr &clause,
+ const Fortran::parser::CharBlock &) {
+ const Fortran::parser::CharBlock source;
+ mlir::Location location = converter.genLocation(source);
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ llvm::omp::OpenMPOffloadMappingFlags mapTypeBits =
+ llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO |
+ llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
+ for (const omp::Object &object : clause.v) {
+ llvm::SmallVector<mlir::Value> bounds;
+ std::stringstream asFortran;
+
+ Fortran::lower::AddrAndBoundsInfo info =
+ Fortran::lower::gatherDataOperandAddrAndBounds<
+ mlir::omp::MapBoundsOp, mlir::omp::MapBoundsType>(
+ converter, firOpBuilder, semaCtx, stmtCtx, *object.sym(),
+ object.ref(), location, asFortran, bounds,
+ treatIndexAsSection);
+
+ auto origSymbol = converter.getSymbolAddress(*object.sym());
+ mlir::Value symAddr = info.addr;
+ if (origSymbol && fir::isTypeWithDescriptor(origSymbol.getType()))
+ symAddr = origSymbol;
+
+ // Explicit map captures are captured ByRef by default,
+ // optimisation passes may alter this to ByCopy or other capture
+ // types to optimise
+ mlir::omp::MapInfoOp mapOp = createMapInfoOp(
+ firOpBuilder, location, symAddr,
+ /*varPtrPtr=*/mlir::Value{}, asFortran.str(), bounds,
+ /*members=*/{}, /*membersIndex=*/mlir::DenseIntElementsAttr{},
+ static_cast<
+ std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+ mapTypeBits),
+ mlir::omp::VariableCaptureKind::ByRef, symAddr.getType());
+
+ if (object.sym()->owner().IsDerivedType()) {
+ addChildIndexAndMapToParent(object, parentMemberIndices, mapOp,
+ semaCtx);
+ } else {
+ useDeviceSyms.push_back(object.sym());
+ useDeviceTypes.push_back(symAddr.getType());
+ useDeviceLocs.push_back(symAddr.getLoc());
+ result.useDevicePtrVars.push_back(mapOp);
+ }
+ }
});
+
+ insertChildMapInfoIntoParent(converter, parentMemberIndices,
+ result.useDevicePtrVars, useDeviceSyms,
+ &useDeviceTypes, &useDeviceLocs);
+ return clauseFound;
}
} // namespace omp
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.h b/flang/lib/Lower/OpenMP/ClauseProcessor.h
index 2c4b3857fda9f..d33873516e996 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.h
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.h
@@ -128,16 +128,20 @@ class ClauseProcessor {
llvm::SmallVectorImpl<const semantics::Symbol *> *reductionSyms =
nullptr) const;
bool processTo(llvm::SmallVectorImpl<DeclareTargetCapturePair> &result) const;
- bool processUseDeviceAddr(
- mlir::omp::UseDeviceAddrClauseOps &result,
- llvm::SmallVectorImpl<mlir::Type> &useDeviceTypes,
- llvm::SmallVectorImpl<mlir::Location> &useDeviceLocs,
- llvm::SmallVectorImpl<const semantics::Symbol *> &useDeviceSyms) const;
- bool processUseDevicePtr(
- mlir::omp::UseDevicePtrClauseOps &result,
- llvm::SmallVectorImpl<mlir::Type> &useDeviceTypes,
- llvm::SmallVectorImpl<mlir::Location> &useDeviceLocs,
- llvm::SmallVectorImpl<const semantics::Symbol *> &useDeviceSyms) const;
+ bool
+ processUseDeviceAddr(Fortran::lower::StatementContext &stmtCtx,
+ mlir::omp::UseDeviceAddrClauseOps &result,
+ llvm::SmallVectorImpl<mlir::Type> &useDeviceTypes,
+ llvm::SmallVectorImpl<mlir::Location> &useDeviceLocs,
+ llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
+ &useDeviceSyms) const;
+ bool
+ processUseDevicePtr(Fortran::lower::StatementContext &stmtCtx,
+ mlir::omp::UseDevicePtrClauseOps &result,
+ llvm::SmallVectorImpl<mlir::Type> &useDeviceTypes,
+ llvm::SmallVectorImpl<mlir::Location> &useDeviceLocs,
+ llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
+ &useDeviceSyms) const;
template <typename T>
bool processMotionClauses(lower::StatementContext &stmtCtx,
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 2b1839b5270d4..6e8cfc3cd594c 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -702,32 +702,73 @@ static void genBodyOfTargetDataOp(
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
mlir::Region ®ion = dataOp.getRegion();
- firOpBuilder.createBlock(®ion, {}, useDeviceTypes, useDeviceLocs);
+ auto *regionBlock =
+ firOpBuilder.createBlock(®ion, {}, useDeviceTypes, useDeviceLocs);
+
+ // Clones the `bounds` placing them inside the target region and returns them.
+ auto cloneBound = [&](mlir::Value bound) {
+ if (mlir::isMemoryEffectFree(bound.getDefiningOp())) {
+ mlir::Operation *clonedOp = bound.getDefiningOp()->clone();
+ regionBlock->push_back(clonedOp);
+ return clonedOp->getResult(0);
+ }
+ TODO(converter.getCurrentLocation(),
+ "target map clause operand unsupported bound type");
+ };
+
+ auto cloneBounds = [cloneBound](llvm::ArrayRef<mlir::Value> bounds) {
+ llvm::SmallVector<mlir::Value> clonedBounds;
+ for (mlir::Value bound : bounds)
+ clonedBounds.emplace_back(cloneBound(bound));
+ return clonedBounds;
+ };
for (auto [argIndex, argSymbol] : llvm::enumerate(useDeviceSymbols)) {
const mlir::BlockArgument &arg = region.front().getArgument(argIndex);
fir::ExtendedValue extVal = converter.getSymbolExtendedValue(*argSymbol);
- if (auto refType = mlir::dyn_cast<fir::ReferenceType>(arg.getType())) {
- if (fir::isa_builtin_cptr_type(refType.getElementType())) {
- converter.bindSymbol(*argSymbol, arg);
- } else {
- // Avoid capture of a reference to a structured binding.
- const semantics::Symbol *sym = argSymbol;
- extVal.match(
- [&](const fir::MutableBoxValue &mbv) {
- converter.bindSymbol(
- *sym,
- fir::MutableBoxValue(
- arg, fir::factory::getNonDeferredLenParams(extVal), {}));
- },
- [&](const auto &) {
- TODO(converter.getCurrentLocation(),
- "use_device clause operand unsupported type");
- });
- }
+ auto refType = mlir::dyn_cast<fir::ReferenceType>(arg.getType());
+ if (refType && fir::isa_builtin_cptr_type(refType.getElementType())) {
+ converter.bindSymbol(*argSymbol, arg);
} else {
- TODO(converter.getCurrentLocation(),
- "use_device clause operand unsupported type");
+ // Avoid capture of a reference to a structured binding.
+ const Fortran::semantics::Symbol *sym = argSymbol;
+ // Structure component symbols don't have bindings.
+ if (sym->owner().IsDerivedType())
+ continue;
+ fir::ExtendedValue extVal = converter.getSymbolExtendedValue(*sym);
+ extVal.match(
+ [&](const fir::BoxValue &v) {
+ converter.bindSymbol(*sym,
+ fir::BoxValue(arg, cloneBounds(v.getLBounds()),
+ v.getExplicitParameters(),
+ v.getExplicitExtents()));
+ },
+ [&](const fir::MutableBoxValue &v) {
+ converter.bindSymbol(
+ *sym, fir::MutableBoxValue(arg, cloneBounds(v.getLBounds()),
+ v.getMutableProperties()));
+ },
+ [&](const fir::ArrayBoxValue &v) {
+ converter.bindSymbol(
+ *sym, fir::ArrayBoxValue(arg, cloneBounds(v.getExtents()),
+ cloneBounds(v.getLBounds()),
+ v.getSourceBox()));
+ },
+ [&](const fir::CharArrayBoxValue &v) {
+ converter.bindSymbol(
+ *sym, fir::CharArrayBoxValue(arg, cloneBound(v.getLen()),
+ cloneBounds(v.getExtents()),
+ cloneBounds(v.getLBounds())));
+ },
+ [&](const fir::CharBoxValue &v) {
+ converter.bindSymbol(
+ *sym, fir::CharBoxValue(arg, cloneBound(v.getLen())));
+ },
+ [&](const fir::UnboxedValue &v) { converter.bindSymbol(*sym, arg); },
+ [&](const auto &) {
+ TODO(converter.getCurrentLocation(),
+ "target map clause operand unsupported type");
+ });
}
}
@@ -1191,9 +1232,9 @@ static void genTargetDataClauses(
cp.processDevice(stmtCtx, clauseOps);
cp.processIf(llvm::omp::Directive::OMPD_target_data, clauseOps);
cp.processMap(loc, stmtCtx, clauseOps);
- cp.processUseDeviceAddr(clauseOps, useDeviceTypes, useDeviceLocs,
+ cp.processUseDeviceAddr(stmtCtx, clauseOps, useDeviceTypes, useDeviceLocs,
useDeviceSyms);
- cp.processUseDevicePtr(clauseOps, useDeviceTypes, useDeviceLocs,
+ cp.processUseDevicePtr(stmtCtx, clauseOps, useDeviceTypes, useDeviceLocs,
useDeviceSyms);
// This function implements the deprecated functionality of use_device_ptr
diff --git a/flang/lib/Optimizer/Transforms/OMPMapInfoFinalization.cpp b/flang/lib/Optimizer/Transforms/OMPMapInfoFinalization.cpp
index ddaa3c5f404f0..e3a8129a9fb73 100644
--- a/flang/lib/Optimizer/Transforms/OMPMapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/Transforms/OMPMapInfoFinalization.cpp
@@ -106,13 +106,12 @@ class OMPMapInfoFinalizationPass
// TODO: map the addendum segment of the descriptor, similarly to the
// above base address/data pointer member.
- if (auto mapClauseOwner =
- llvm::dyn_cast<mlir::omp::MapClauseOwningOpInterface>(target)) {
+ auto addOperands = [&](mlir::OperandRange &operandsArr,
+ mlir::MutableOperandRange &mutableOpRange,
+ auto directiveOp) {
llvm::SmallVector<mlir::Value> newMapOps;
- mlir::OperandRange mapVarsArr = mapClauseOwner.getMapVars();
-
- for (size_t i = 0; i < mapVarsArr.size(); ++i) {
- if (mapVarsArr[i] == op) {
+ for (size_t i = 0; i < operandsArr.size(); ++i) {
+ if (operandsArr[i] == op) {
// Push new implicit maps generated for the descriptor.
newMapOps.push_back(baseAddr);
@@ -120,13 +119,29 @@ class OMPMapInfoFinalizationPass
// new additional map operand with an appropriate BlockArgument,
// as the printing and later processing currently requires a 1:1
// mapping of BlockArgs to MapInfoOp's at the same placement in
- // each array (BlockArgs and MapVars).
- if (auto targetOp = llvm::dyn_cast<mlir::omp::TargetOp>(target))
- targetOp.getRegion().insertArgument(i, baseAddr.getType(), loc);
+ // each array (BlockArgs and MapOperands).
+ if (directiveOp) {
+ directiveOp.getRegion().insertArgument(i, baseAddr.getType(), loc);
+ }
}
- newMapOps.push_back(mapVarsArr[i]);
+ newMapOps.push_back(operandsArr[i]);
}
- mapClauseOwner.getMapVarsMutable().assign(newMapOps);
+ mutableOpRange.assign(newMapOps);
+ };
+ if (auto mapClauseOwner =
+ llvm::dyn_cast<mlir::omp::MapClauseOwningOpInterface>(target)) {
+ mlir::OperandRange mapOperandsArr = mapClauseOwner.getMapVars();
+ mlir::MutableOperandRange mapMutableOpRange =
+ mapClauseOwner.getMapVarsMutable();
+ mlir::omp::TargetOp targetOp =
+ llvm::dyn_cast<mlir::omp::TargetOp>(target);
+ addOperands(mapOperandsArr, mapMutableOpRange, targetOp);
+ }
+ if (auto targetDataOp = llvm::dyn_cast<mlir::omp::TargetDataOp>(target)) {
+ mlir::OperandRange useDevAddrArr = targetDataOp.getUseDeviceAddrVars();
+ mlir::MutableOperandRange useDevAddrMutableOpRange =
+ targetDataOp.getUseDeviceAddrVarsMutable();
+ addOperands(useDevAddrArr, useDevAddrMutableOpRange, targetDataOp);
}
mlir::Value newDescParentMapOp = builder.create<mlir::omp::MapInfoOp>(
diff --git a/flang/test/Lower/OpenMP/target.f90 b/flang/test/Lower/OpenMP/target.f90
index 9b92293cbf92f..c37897df55ec2 100644
--- a/flang/test/Lower/OpenMP/target.f90
+++ b/flang/test/Lower/OpenMP/target.f90
@@ -527,21 +527,23 @@ end subroutine omp_target_device_ptr
!===============================================================================
!CHECK-LABEL: func.func @_QPomp_target_device_addr() {
- subroutine omp_target_device_addr
+subroutine omp_target_device_addr
integer, pointer :: a
!CHECK: %[[VAL_0:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "a", uniq_name = "_QFomp_target_device_addrEa"}
!CHECK: %[[VAL_0_DECL:.*]]:2 = hlfir.declare %0 {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFomp_target_device_addrEa"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) -> (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
!CHECK: %[[MAP_MEMBERS:.*]] = omp.map.info var_ptr({{.*}} : !fir.ref<!fir.box<!fir.ptr<i32>>>, i32) var_ptr_ptr({{.*}} : !fir.llvm_ptr<!fir.ref<i32>>) map_clauses(tofrom) capture(ByRef) -> !fir.llvm_ptr<!fir.ref<i32>> {name = ""}
!CHECK: %[[MAP:.*]] = omp.map.info var_ptr({{.*}} : !fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.box<!fir.ptr<i32>>) map_clauses(tofrom) capture(ByRef) members(%[[MAP_MEMBERS]] : [0] : !fir.llvm_ptr<!fir.ref<i32>>) -> !fir.ref<!fir.box<!fir.ptr<i32>>> {name = "a"}
- !CHECK: omp.target_data map_entries(%[[MAP_MEMBERS]], %[[MAP]] : {{.*}}) use_device_addr(%[[VAL_0_DECL]]#1 : !fir.ref<!fir.box<!fir.ptr<i32>>>) {
+ !CHECK: %[[DEV_ADDR_MEMBERS:.*]] = omp.map.info var_ptr({{.*}} : !fir.ref<!fir.box<!fir.ptr<i32>>>, i32) var_ptr_ptr({{.*}} : !fir.llvm_ptr<!fir.ref<i32>>) map_clauses(tofrom) capture(ByRef) -> !fir.llvm_ptr<!fir.ref<i32>> {name = ""}
+ !CHECK: %[[DEV_ADDR:.*]] = omp.map.info var_ptr({{.*}} : !fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.box<!fir.ptr<i32>>) map_clauses(tofrom) capture(ByRef) members(%[[DEV_ADDR_MEMBERS]] : [0] : !fir.llvm_ptr<!fir.ref<i32>>) -> !fir.ref<!fir.box<!fir.ptr<i32>>> {name = "a"}
+ !CHECK: omp.target_data map_entries(%[[MAP_MEMBERS]], %[[MAP]] : {{.*}}) use_device_addr(%[[DEV_ADDR_MEMBERS]], %[[DEV_ADDR]] : {{.*}}) {
!$omp target data map(tofrom: a) use_device_addr(a)
- !CHECK: ^bb0(%[[VAL_1:.*]]: !fir.ref<!fir.box<!fir.ptr<i32>>>):
- !CHECK: %[[VAL_1_DECL:.*]]:2 = hlfir.declare %[[VAL_1]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFomp_target_device_addrEa"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) -> (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
+ !CHECK: ^bb0(%[[ARG_0:.*]]: !fir.llvm_ptr<!fir.ref<i32>>, %[[ARG_1:.*]]: !fir.ref<!fir.box<!fir.ptr<i32>>>):
+ !CHECK: %[[VAL_1_DECL:.*]]:2 = hlfir.declare %[[ARG_1]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFomp_target_device_addrEa"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) -> (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
!CHECK: %[[C10:.*]] = arith.constant 10 : i32
!CHECK: %[[A_BOX:.*]] = fir.load %[[VAL_1_DECL]]#0 : !fir.ref<!fir.box<!fir.ptr<i32>>>
!CHECK: %...
[truncated]
|
skatrak
reviewed
Aug 6, 2024
TIFitis
force-pushed
the
users/akash/use-device-lowering-1
branch
from
d996277 to
74535f5
Compare
|
@skatrak Thanks for the review. I have addressed the comments in the latest revision. |
skatrak
reviewed
Aug 8, 2024
flang/lib/Lower/OpenMP/OpenMP.cpp
Outdated
Comment on lines
895
to
897
| if (!isTargetOp && refType && | ||
| fir::isa_builtin_cptr_type(refType.getElementType())) { | ||
| converter.bindSymbol(*argSymbol, arg); |
There was a problem hiding this comment.
If this is only needed for target data and not target, I think we need a comment here to explain why. Otherwise, we should decide whether we are supposed to do this in both cases or in none of them, since it looks to me like something that may have slipped through the cracks due to having this code duplicated between genBodyOfTargetOp and genBodyOfTargetDataOp. Perhaps @agozillon can comment on that.
TIFitis
force-pushed
the
users/akash/use-device-lowering-1
branch
from
74535f5 to
7e4b6d7
Compare
|
@skatrak I have addressed the latest comments in the new revision. Thanks again for the review. |
skatrak
reviewed
Aug 21, 2024
flang/lib/Lower/OpenMP/OpenMP.cpp
Outdated
| "use_device clause operand unsupported type"); | ||
| } | ||
| } | ||
| mapBodySymbols(converter, region, regionBlock, useDeviceSymbols); |
There was a problem hiding this comment.
Would it be correct for a bound cloned into the omp.target_data region to take an outside value as an argument? For omp.target this is disallowed due to the IsolatedFromAbove trait, so there's some additional post-processing to handle them. Here we don't have the same restriction, but I wonder if we would produce valid code if we don't map these too.
There was a problem hiding this comment.
Yes, it's okay for outside values to reach here for target_data.
|
@skatrak Thanks for the review. I've addressed the latest comments in the new revision. |
|
Ping for review. |
skatrak
approved these changes
Aug 29, 2024
flang/lib/Lower/OpenMP/OpenMP.cpp
Outdated
| @@ -692,53 +692,102 @@ static void createBodyOfOp(mlir::Operation &op, const OpWithBodyGenInfo &info, | |||
| marker->erase(); | |||
| } | |||
|
|
|||
| void mapBodySymbols(lower::AbstractConverter &converter, mlir::Region ®ion, | |||
| llvm::ArrayRef<const semantics::Symbol *> mapSyms) { | |||
| assert(region.hasOneBlock()); | |||
There was a problem hiding this comment.
Nit: Add a description to the assert.
This patch updates the use_device_ptr and use_device_addr clauses to use the mapInfoOps for lowering. This allows all the types that are handle by the map clauses such as derived types to also be supported by the use_device_clauses. This is patch 1/2 in a series of patches. Co-authored-by: Raghu Maddhipatla [email protected]
TIFitis
force-pushed
the
users/akash/use-device-lowering-1
branch
from
f6a6b00 to
b8d523a
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/6773 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch updates the use_device_ptr and use_device_addr clauses to use the mapInfoOps for lowering. This allows all the types that are handle by the map clauses such as derived types to also be supported by the use_device_clauses.
This is patch 1/2 in a series of patches.
Co-authored-by: Raghu Maddhipatla [email protected]