Skip to content

[flang][mlir][llvm][OpenMP] Add lowering and translation support for mergeable clause on task #114662

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.

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

Merged
merged 5 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions flang/lib/Lower/OpenMP/OpenMP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1348,8 +1348,8 @@ static void genTaskClauses(lower::AbstractConverter &converter,
cp.processUntied(clauseOps);
// TODO Support delayed privatization.

cp.processTODO<clause::Affinity, clause::Detach, clause::InReduction,
clause::Mergeable>(loc, llvm::omp::Directive::OMPD_task);
cp.processTODO<clause::Affinity, clause::Detach, clause::InReduction>(
loc, llvm::omp::Directive::OMPD_task);
}

static void genTaskgroupClauses(lower::AbstractConverter &converter,
Expand Down
13 changes: 0 additions & 13 deletions flang/test/Lower/OpenMP/Todo/task_mergeable.f90

This file was deleted.

8 changes: 8 additions & 0 deletions flang/test/Lower/OpenMP/task.f90
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,11 @@ subroutine task_multiple_clauses()
!CHECK: omp.terminator
!$omp end task
end subroutine task_multiple_clauses

subroutine task_mergeable()
!CHECK: omp.task mergeable {
!CHECK: omp.terminator
!CHECK: }
!$omp task mergeable
!$omp end task
end subroutine
12 changes: 6 additions & 6 deletions llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -1262,12 +1262,12 @@ class OpenMPIRBuilder {
/// cannot be resumed until execution of the structured
/// block that is associated with the generated task is
/// completed.
InsertPointOrErrorTy createTask(const LocationDescription &Loc,
InsertPointTy AllocaIP,
BodyGenCallbackTy BodyGenCB, bool Tied = true,
Value *Final = nullptr,
Value *IfCondition = nullptr,
SmallVector<DependData> Dependencies = {});
/// \param Mergeable If the given task is `mergeable`
InsertPointOrErrorTy
createTask(const LocationDescription &Loc, InsertPointTy AllocaIP,
BodyGenCallbackTy BodyGenCB, bool Tied = true,
Value *Final = nullptr, Value *IfCondition = nullptr,
SmallVector<DependData> Dependencies = {}, bool Mergeable = false);

/// Generator for the taskgroup construct
///
Expand Down
17 changes: 11 additions & 6 deletions llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1816,11 +1816,10 @@ static Value *emitTaskDependencies(
return DepArray;
}

OpenMPIRBuilder::InsertPointOrErrorTy
OpenMPIRBuilder::createTask(const LocationDescription &Loc,
InsertPointTy AllocaIP, BodyGenCallbackTy BodyGenCB,
bool Tied, Value *Final, Value *IfCondition,
SmallVector<DependData> Dependencies) {
OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
const LocationDescription &Loc, InsertPointTy AllocaIP,
BodyGenCallbackTy BodyGenCB, bool Tied, Value *Final, Value *IfCondition,
SmallVector<DependData> Dependencies, bool Mergeable) {

if (!updateToLocation(Loc))
return InsertPointTy();
Expand Down Expand Up @@ -1866,7 +1865,8 @@ OpenMPIRBuilder::createTask(const LocationDescription &Loc,
Builder, AllocaIP, ToBeDeleted, TaskAllocaIP, "global.tid", false));

OI.PostOutlineCB = [this, Ident, Tied, Final, IfCondition, Dependencies,
TaskAllocaBB, ToBeDeleted](Function &OutlinedFn) mutable {
Mergeable, TaskAllocaBB,
ToBeDeleted](Function &OutlinedFn) mutable {
// Replace the Stale CI by appropriate RTL function call.
assert(OutlinedFn.getNumUses() == 1 &&
"there must be a single user for the outlined function");
Expand All @@ -1891,6 +1891,8 @@ OpenMPIRBuilder::createTask(const LocationDescription &Loc,
// Task is untied iff (Flags & 1) == 0.
// Task is final iff (Flags & 2) == 2.
// Task is not final iff (Flags & 2) == 0.
// Task is mergeable iff (Flags & 4) == 4.
// Task is not mergeable iff (Flags & 4) == 0.
// TODO: Handle the other flags.
Value *Flags = Builder.getInt32(Tied);
if (Final) {
Expand All @@ -1899,6 +1901,9 @@ OpenMPIRBuilder::createTask(const LocationDescription &Loc,
Flags = Builder.CreateOr(FinalFlag, Flags);
}

if (Mergeable)
Flags = Builder.CreateOr(Builder.getInt32(4), Flags);

// Argument - `sizeof_kmp_task_t` (TaskSize)
// Tasksize refers to the size in bytes of kmp_task_t data structure
// including private vars accessed in task.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,6 @@ static LogicalResult checkImplementationStatus(Operation &op) {
if (!op.getLinearVars().empty() || !op.getLinearStepVars().empty())
result = todo("linear");
};
auto checkMergeable = [&todo](auto op, LogicalResult &result) {
if (op.getMergeable())
result = todo("mergeable");
};
auto checkNontemporal = [&todo](auto op, LogicalResult &result) {
if (!op.getNontemporalVars().empty())
result = todo("nontemporal");
Expand Down Expand Up @@ -257,7 +253,6 @@ static LogicalResult checkImplementationStatus(Operation &op) {
.Case([&](omp::TaskOp op) {
checkAllocate(op, result);
checkInReduction(op, result);
checkMergeable(op, result);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this is the only use of checkMergeable so it can now be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just double confirming: do you want me to add a comment here? I removed this check, because mergeable is a task specific clause only.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that you should also remove the definition of checkMergable as the lambda is now unused.

checkPriority(op, result);
checkUntied(op, result);
})
Expand Down Expand Up @@ -1665,7 +1660,8 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
moduleTranslation.getOpenMPBuilder()->createTask(
ompLoc, allocaIP, bodyCB, !taskOp.getUntied(),
moduleTranslation.lookupValue(taskOp.getFinal()),
moduleTranslation.lookupValue(taskOp.getIfExpr()), dds);
moduleTranslation.lookupValue(taskOp.getIfExpr()), dds,
taskOp.getMergeable());

if (failed(handleError(afterIP, *taskOp)))
return failure();
Expand Down
13 changes: 13 additions & 0 deletions mlir/test/Target/LLVMIR/openmp-llvm.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -3003,6 +3003,19 @@ module attributes {omp.is_target_device = true} {

// -----

// Third argument is 5: essentially (4 || 1)
// signifying this task is TIED and MERGEABLE

// CHECK: {{.*}} = call ptr @__kmpc_omp_task_alloc(ptr @1, i32 %omp_global_thread_num, i32 5, i64 40, i64 0, ptr @omp_task_mergeable..omp_par)
llvm.func @omp_task_mergeable() {
omp.task mergeable {
omp.terminator
}
llvm.return
}

// -----

llvm.func external @foo_before() -> ()
llvm.func external @foo() -> ()
llvm.func external @foo_after() -> ()
Expand Down
11 changes: 0 additions & 11 deletions mlir/test/Target/LLVMIR/openmp-todo.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -447,17 +447,6 @@ llvm.func @task_in_reduction(%x : !llvm.ptr) {

// -----

llvm.func @task_mergeable() {
// expected-error@below {{not yet implemented: Unhandled clause mergeable in omp.task operation}}
// expected-error@below {{LLVM Translation failed for operation: omp.task}}
omp.task mergeable {
omp.terminator
}
llvm.return
}

// -----

llvm.func @task_priority(%x : i32) {
// expected-error@below {{not yet implemented: Unhandled clause priority in omp.task operation}}
// expected-error@below {{LLVM Translation failed for operation: omp.task}}
Expand Down
Loading