Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MLIR][OpenMP] NFC: Uniformize OpenMP ops names (llvm#85393) #53

Merged
merged 2 commits into from
Mar 20, 2024
Merged

[MLIR][OpenMP] NFC: Uniformize OpenMP ops names (llvm#85393) #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4382511
[MLIR][OpenMP] NFC: Uniformize OpenMP ops names (#85393)
skatrak Mar 20, 2024
5bc2285
Fix downstream naming issues
skatrak Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions flang/docs/OpenMP-descriptor-management.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,16 @@ the lowering to FIR/HLFIR has been performed an OpenMP dialect specific pass for
mappings, with one extra per pointer member in the descriptor that is supported on top of the original
descriptor map operation. These pointers members are linked to the parent descriptor by adding them to
the member field of the original descriptor map operation, they are then inserted into the relevant map
owning operation's (`omp.TargetOp`, `omp.DataOp` etc.) map operand list and in cases where the owning operation
is `IsolatedFromAbove`, it also inserts them as `BlockArgs` to canonicalize the mappings and simplify lowering.
owning operation's (`omp.TargetOp`, `omp.TargetDataOp` etc.) map operand list and in cases where the owning
operation is `IsolatedFromAbove`, it also inserts them as `BlockArgs` to canonicalize the mappings and
simplify lowering.

An example transformation by the `OMPDescriptorMapInfoGenPass`:

```

...
%12 = omp.map_info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.box<!fir.ptr<!fir.array<?xi32>>>) map_clauses(tofrom) capture(ByRef) bounds(%11) -> !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>> {name = "arg_alloc"}
%12 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.box<!fir.ptr<!fir.array<?xi32>>>) map_clauses(tofrom) capture(ByRef) bounds(%11) -> !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>> {name = "arg_alloc"}
...
omp.target map_entries(%12 -> %arg1, %13 -> %arg2 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.ref<i32>) {
^bb0(%arg1: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, %arg2: !fir.ref<i32>):
Expand All @@ -67,8 +68,8 @@ omp.target map_entries(%12 -> %arg1, %13 -> %arg2 : !fir.ref<!fir.box<!fir.ptr<!

...
%12 = fir.box_offset %1#1 base_addr : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>
%13 = omp.map_info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.array<?xi32>) var_ptr_ptr(%12 : !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>) map_clauses(tofrom) capture(ByRef) bounds(%11) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>> {name = ""}
%14 = omp.map_info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.box<!fir.ptr<!fir.array<?xi32>>>) map_clauses(tofrom) capture(ByRef) members(%13 : !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>> {name = "arg_alloc"}
%13 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.array<?xi32>) var_ptr_ptr(%12 : !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>) map_clauses(tofrom) capture(ByRef) bounds(%11) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>> {name = ""}
%14 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.box<!fir.ptr<!fir.array<?xi32>>>) map_clauses(tofrom) capture(ByRef) members(%13 : !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>> {name = "arg_alloc"}
...
omp.target map_entries(%13 -> %arg1, %14 -> %arg2, %15 -> %arg3 : !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.ref<i32>) {
^bb0(%arg1: !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>, %arg2: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, %arg3: !fir.ref<i32>):
Expand Down
10 changes: 5 additions & 5 deletions flang/lib/Lower/OpenMP/ClauseProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -924,8 +924,8 @@ bool ClauseProcessor::processMap(

Fortran::lower::AddrAndBoundsInfo info =
Fortran::lower::gatherDataOperandAddrAndBounds<
Fortran::parser::OmpObject, mlir::omp::DataBoundsOp,
mlir::omp::DataBoundsType>(
Fortran::parser::OmpObject, mlir::omp::MapBoundsOp,
mlir::omp::MapBoundsType>(
converter, firOpBuilder, semaCtx, stmtCtx, ompObject,
clauseLocation, asFortran, bounds, treatIndexAsSection);

Expand Down Expand Up @@ -979,9 +979,9 @@ bool ClauseProcessor::processReduction(
[&](const ClauseTy::Reduction *reductionClause,
const Fortran::parser::CharBlock &) {
ReductionProcessor rp;
rp.addReductionDecl(currentLocation, converter, reductionClause->v,
reductionVars, reductionDeclSymbols,
reductionSymbols);
rp.addDeclareReduction(currentLocation, converter, reductionClause->v,
reductionVars, reductionDeclSymbols,
reductionSymbols);
});
}

Expand Down
4 changes: 2 additions & 2 deletions flang/lib/Lower/OpenMP/ClauseProcessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ bool ClauseProcessor::processMotionClauses(
std::stringstream asFortran;
Fortran::lower::AddrAndBoundsInfo info =
Fortran::lower::gatherDataOperandAddrAndBounds<
Fortran::parser::OmpObject, mlir::omp::DataBoundsOp,
mlir::omp::DataBoundsType>(
Fortran::parser::OmpObject, mlir::omp::MapBoundsOp,
mlir::omp::MapBoundsType>(
converter, firOpBuilder, semaCtx, stmtCtx, ompObject,
clauseLocation, asFortran, bounds, treatIndexAsSection);

Expand Down
6 changes: 3 additions & 3 deletions flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
firOpBuilder.restoreInsertionPoint(unstructuredSectionsIP);
}
}
} else if (mlir::isa<mlir::omp::WsLoopOp>(op)) {
} else if (mlir::isa<mlir::omp::WsloopOp>(op)) {
// Update the original variable just before exiting the worksharing
// loop. Conversion as follows:
//
Expand Down Expand Up @@ -250,8 +250,8 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {

mlir::Value iv = op->getRegion(0).front().getArguments()[0];
mlir::Value ub =
mlir::dyn_cast<mlir::omp::WsLoopOp>(op).getUpperBound()[0];
mlir::Value step = mlir::dyn_cast<mlir::omp::WsLoopOp>(op).getStep()[0];
mlir::dyn_cast<mlir::omp::WsloopOp>(op).getUpperBound()[0];
mlir::Value step = mlir::dyn_cast<mlir::omp::WsloopOp>(op).getStep()[0];

// v = iv + step
// cmp = step < 0 ? v < ub : v > ub
Expand Down
85 changes: 42 additions & 43 deletions flang/lib/Lower/OpenMP/OpenMP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ static void createBodyOfOp(Op &op, OpWithBodyGenInfo &info) {

// Start with privatization, so that the lowering of the nested
// code will use the right symbols.
constexpr bool isLoop = std::is_same_v<Op, mlir::omp::WsLoopOp> ||
constexpr bool isLoop = std::is_same_v<Op, mlir::omp::WsloopOp> ||
std::is_same_v<Op, mlir::omp::SimdLoopOp>;
bool privatize = info.clauses && !info.outerCombined;

Expand Down Expand Up @@ -611,7 +611,7 @@ static void genBodyOfTargetDataOp(
Fortran::lower::AbstractConverter &converter,
Fortran::semantics::SemanticsContext &semaCtx,
Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::omp::DataOp &dataOp,
mlir::omp::TargetDataOp &dataOp,
const llvm::SmallVector<mlir::Type> &useDeviceTypes,
const llvm::SmallVector<mlir::Location> &useDeviceLocs,
const llvm::SmallVector<const Fortran::semantics::Symbol *>
Expand Down Expand Up @@ -822,7 +822,6 @@ genParallelOp(Fortran::lower::AbstractConverter &converter,
dsp.processStep2();
}


const auto &delayedPrivatizationInfo = dsp.getDelayedPrivatizationInfo();

auto genRegionEntryCB = [&](mlir::Operation *op) {
Expand Down Expand Up @@ -977,8 +976,8 @@ genTaskOp(Fortran::lower::AbstractConverter &converter,
dependOperands, allocateOperands, allocatorOperands);
}

static mlir::omp::TaskGroupOp
genTaskGroupOp(Fortran::lower::AbstractConverter &converter,
static mlir::omp::TaskgroupOp
genTaskgroupOp(Fortran::lower::AbstractConverter &converter,
Fortran::semantics::SemanticsContext &semaCtx,
Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation,
Expand All @@ -988,20 +987,20 @@ genTaskGroupOp(Fortran::lower::AbstractConverter &converter,
cp.processAllocate(allocatorOperands, allocateOperands);
cp.processTODO<Fortran::parser::OmpClause::TaskReduction>(
currentLocation, llvm::omp::Directive::OMPD_taskgroup);
return genOpWithBody<mlir::omp::TaskGroupOp>(
return genOpWithBody<mlir::omp::TaskgroupOp>(
OpWithBodyGenInfo(converter, semaCtx, currentLocation, eval)
.setGenNested(genNested)
.setClauses(&clauseList),
/*task_reduction_vars=*/mlir::ValueRange(),
/*task_reductions=*/nullptr, allocateOperands, allocatorOperands);
}

static mlir::omp::DataOp
genDataOp(Fortran::lower::AbstractConverter &converter,
Fortran::semantics::SemanticsContext &semaCtx,
Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList) {
static mlir::omp::TargetDataOp
genTargetDataOp(Fortran::lower::AbstractConverter &converter,
Fortran::semantics::SemanticsContext &semaCtx,
Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList) {
Fortran::lower::StatementContext stmtCtx;
mlir::Value ifClauseOperand, deviceOperand;
llvm::SmallVector<mlir::Value> mapOperands, devicePtrOperands,
Expand All @@ -1021,7 +1020,7 @@ genDataOp(Fortran::lower::AbstractConverter &converter,
cp.processMap(currentLocation, llvm::omp::Directive::OMPD_target_data,
stmtCtx, mapOperands);

auto dataOp = converter.getFirOpBuilder().create<mlir::omp::DataOp>(
auto dataOp = converter.getFirOpBuilder().create<mlir::omp::TargetDataOp>(
currentLocation, ifClauseOperand, deviceOperand, devicePtrOperands,
deviceAddrOperands, mapOperands);
genBodyOfTargetDataOp(converter, semaCtx, eval, genNested, dataOp,
Expand All @@ -1031,11 +1030,11 @@ genDataOp(Fortran::lower::AbstractConverter &converter,
}

template <typename OpTy>
static OpTy
genEnterExitUpdateDataOp(Fortran::lower::AbstractConverter &converter,
Fortran::semantics::SemanticsContext &semaCtx,
mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList) {
static OpTy genTargetEnterExitDataUpdateOp(
Fortran::lower::AbstractConverter &converter,
Fortran::semantics::SemanticsContext &semaCtx,
mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
Fortran::lower::StatementContext stmtCtx;
mlir::Value ifClauseOperand, deviceOperand;
Expand All @@ -1046,15 +1045,15 @@ genEnterExitUpdateDataOp(Fortran::lower::AbstractConverter &converter,
Fortran::parser::OmpIfClause::DirectiveNameModifier directiveName;
// GCC 9.3.0 emits a (probably) bogus warning about an unused variable.
[[maybe_unused]] llvm::omp::Directive directive;
if constexpr (std::is_same_v<OpTy, mlir::omp::EnterDataOp>) {
if constexpr (std::is_same_v<OpTy, mlir::omp::TargetEnterDataOp>) {
directiveName =
Fortran::parser::OmpIfClause::DirectiveNameModifier::TargetEnterData;
directive = llvm::omp::Directive::OMPD_target_enter_data;
} else if constexpr (std::is_same_v<OpTy, mlir::omp::ExitDataOp>) {
} else if constexpr (std::is_same_v<OpTy, mlir::omp::TargetExitDataOp>) {
directiveName =
Fortran::parser::OmpIfClause::DirectiveNameModifier::TargetExitData;
directive = llvm::omp::Directive::OMPD_target_exit_data;
} else if constexpr (std::is_same_v<OpTy, mlir::omp::UpdateDataOp>) {
} else if constexpr (std::is_same_v<OpTy, mlir::omp::TargetUpdateOp>) {
directiveName =
Fortran::parser::OmpIfClause::DirectiveNameModifier::TargetUpdate;
directive = llvm::omp::Directive::OMPD_target_update;
Expand All @@ -1068,7 +1067,7 @@ genEnterExitUpdateDataOp(Fortran::lower::AbstractConverter &converter,
cp.processDepend(dependTypeOperands, dependOperands);
cp.processNowait(nowaitAttr);

if constexpr (std::is_same_v<OpTy, mlir::omp::UpdateDataOp>) {
if constexpr (std::is_same_v<OpTy, mlir::omp::TargetUpdateOp>) {
cp.processMotionClauses<Fortran::parser::OmpClause::To>(stmtCtx,
mapOperands);
cp.processMotionClauses<Fortran::parser::OmpClause::From>(stmtCtx,
Expand Down Expand Up @@ -1265,7 +1264,7 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,

ClauseProcessor cp(converter, semaCtx, clauseList);
cp.processIf(Fortran::parser::OmpIfClause::DirectiveNameModifier::Target,
ifClauseOperand);
ifClauseOperand);
cp.processDevice(stmtCtx, deviceOperand);
cp.processThreadLimit(stmtCtx, threadLimitOperand);
cp.processDepend(dependTypeOperands, dependOperands);
Expand Down Expand Up @@ -1322,15 +1321,15 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
converter.getCurrentLocation());
if (fir::unwrapRefType(info.addr.getType()).isa<fir::BaseBoxType>())
bounds =
Fortran::lower::genBoundsOpsFromBox<mlir::omp::DataBoundsOp,
mlir::omp::DataBoundsType>(
Fortran::lower::genBoundsOpsFromBox<mlir::omp::MapBoundsOp,
mlir::omp::MapBoundsType>(
converter.getFirOpBuilder(), converter.getCurrentLocation(),
converter, dataExv, info);
if (fir::unwrapRefType(info.addr.getType()).isa<fir::SequenceType>()) {
bool dataExvIsAssumedSize =
Fortran::semantics::IsAssumedSizeArray(sym.GetUltimate());
bounds = Fortran::lower::genBaseBoundsOps<mlir::omp::DataBoundsOp,
mlir::omp::DataBoundsType>(
bounds = Fortran::lower::genBaseBoundsOps<mlir::omp::MapBoundsOp,
mlir::omp::MapBoundsType>(
converter.getFirOpBuilder(), converter.getCurrentLocation(),
converter, dataExv, dataExvIsAssumedSize);
}
Expand Down Expand Up @@ -1620,19 +1619,19 @@ genOmpSimpleStandalone(Fortran::lower::AbstractConverter &converter,
firOpBuilder.create<mlir::omp::TaskyieldOp>(currentLocation);
break;
case llvm::omp::Directive::OMPD_target_data:
genDataOp(converter, semaCtx, eval, genNested, currentLocation,
opClauseList);
genTargetDataOp(converter, semaCtx, eval, genNested, currentLocation,
opClauseList);
break;
case llvm::omp::Directive::OMPD_target_enter_data:
genEnterExitUpdateDataOp<mlir::omp::EnterDataOp>(
genTargetEnterExitDataUpdateOp<mlir::omp::TargetEnterDataOp>(
converter, semaCtx, currentLocation, opClauseList);
break;
case llvm::omp::Directive::OMPD_target_exit_data:
genEnterExitUpdateDataOp<mlir::omp::ExitDataOp>(
genTargetEnterExitDataUpdateOp<mlir::omp::TargetExitDataOp>(
converter, semaCtx, currentLocation, opClauseList);
break;
case llvm::omp::Directive::OMPD_target_update:
genEnterExitUpdateDataOp<mlir::omp::UpdateDataOp>(
genTargetEnterExitDataUpdateOp<mlir::omp::TargetUpdateOp>(
converter, semaCtx, currentLocation, opClauseList);
break;
case llvm::omp::Directive::OMPD_ordered:
Expand Down Expand Up @@ -1850,7 +1849,7 @@ createSimdLoop(Fortran::lower::AbstractConverter &converter,
.setGenRegionEntryCb(ivCallback));
}

static void createWsLoop(Fortran::lower::AbstractConverter &converter,
static void createWsloop(Fortran::lower::AbstractConverter &converter,
Fortran::semantics::SemanticsContext &semaCtx,
Fortran::lower::pft::Evaluation &eval,
llvm::omp::Directive ompDirective,
Expand Down Expand Up @@ -1887,7 +1886,7 @@ static void createWsLoop(Fortran::lower::AbstractConverter &converter,
convertLoopBounds(converter, loc, lowerBound, upperBound, step,
loopVarTypeSize);

auto wsLoopOp = firOpBuilder.create<mlir::omp::WsLoopOp>(
auto wsLoopOp = firOpBuilder.create<mlir::omp::WsloopOp>(
loc, lowerBound, upperBound, step, linearVars, linearStepVars,
reductionVars,
reductionDeclSymbols.empty()
Expand Down Expand Up @@ -1934,7 +1933,7 @@ static void createWsLoop(Fortran::lower::AbstractConverter &converter,
reductionTypes);
};

createBodyOfOp<mlir::omp::WsLoopOp>(
createBodyOfOp<mlir::omp::WsloopOp>(
wsLoopOp, OpWithBodyGenInfo(converter, semaCtx, loc, *nestedEval)
.setClauses(&beginClauseList)
.setDataSharingProcessor(&dsp)
Expand Down Expand Up @@ -1962,7 +1961,7 @@ static void createWsLoop(Fortran::lower::AbstractConverter &converter,
}
}

static void createSimdWsLoop(
static void createSimdWsloop(
Fortran::lower::AbstractConverter &converter,
Fortran::semantics::SemanticsContext &semaCtx,
Fortran::lower::pft::Evaluation &eval, llvm::omp::Directive ompDirective,
Expand All @@ -1983,7 +1982,7 @@ static void createSimdWsLoop(
// if clause. Currently if clause can be skipped because we always assume
// SIMD length = 1.
DataSharingProcessor dsp(converter, beginClauseList, eval);
createWsLoop(converter, semaCtx, eval, ompDirective, beginClauseList,
createWsloop(converter, semaCtx, eval, ompDirective, beginClauseList,
endClauseList, loc, dsp);
}

Expand Down Expand Up @@ -2058,7 +2057,7 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,

if (llvm::omp::allDoSimdSet.test(ompDirective)) {
// 2.9.3.2 Workshare SIMD construct
createSimdWsLoop(converter, semaCtx, eval, ompDirective, loopOpClauseList,
createSimdWsloop(converter, semaCtx, eval, ompDirective, loopOpClauseList,
endClauseList, currentLocation);

} else if (llvm::omp::allSimdSet.test(ompDirective)) {
Expand All @@ -2067,7 +2066,7 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
currentLocation);
genOpenMPReduction(converter, semaCtx, loopOpClauseList);
} else {
createWsLoop(converter, semaCtx, eval, ompDirective, loopOpClauseList,
createWsloop(converter, semaCtx, eval, ompDirective, loopOpClauseList,
endClauseList, currentLocation, dsp);
}
}
Expand Down Expand Up @@ -2144,15 +2143,15 @@ genOMP(Fortran::lower::AbstractConverter &converter,
beginClauseList, directive.v);
break;
case llvm::omp::Directive::OMPD_target_data:
genDataOp(converter, semaCtx, eval, /*genNested=*/true, currentLocation,
beginClauseList);
genTargetDataOp(converter, semaCtx, eval, /*genNested=*/true,
currentLocation, beginClauseList);
break;
case llvm::omp::Directive::OMPD_task:
genTaskOp(converter, semaCtx, eval, /*genNested=*/true, currentLocation,
beginClauseList);
break;
case llvm::omp::Directive::OMPD_taskgroup:
genTaskGroupOp(converter, semaCtx, eval, /*genNested=*/true,
genTaskgroupOp(converter, semaCtx, eval, /*genNested=*/true,
currentLocation, beginClauseList);
break;
case llvm::omp::Directive::OMPD_teams:
Expand Down Expand Up @@ -2505,7 +2504,7 @@ genOMP(Fortran::lower::AbstractConverter &converter,
mlir::Operation *Fortran::lower::genOpenMPTerminator(fir::FirOpBuilder &builder,
mlir::Operation *op,
mlir::Location loc) {
if (mlir::isa<mlir::omp::WsLoopOp, mlir::omp::ReductionDeclareOp,
if (mlir::isa<mlir::omp::WsloopOp, mlir::omp::DeclareReductionOp,
mlir::omp::AtomicUpdateOp, mlir::omp::SimdLoopOp>(op))
return builder.create<mlir::omp::YieldOp>(loc);
else
Expand Down
Loading