[Flang][OpenMP] Add translation support for MutexInOutSet and InOutSet #120715
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
Thirumalai-Shaktivel
requested review from
tblah,
kiranchandramohan and
kparzysz
llvmbot
added
mlir:llvm
mlir
flang
Thirumalai-Shaktivel
removed
mlir:llvm
mlir
flang
|
@llvm/pr-subscribers-mlir-llvm @llvm/pr-subscribers-flang-fir-hlfir Author: Thirumalai Shaktivel (Thirumalai-Shaktivel) ChangesImplementatoin details: Full diff: https://github.com/llvm/llvm-project/pull/120715.diff 8 Files Affected:
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 3c9831120351ee..c4ab5e0033d04a 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -153,10 +153,13 @@ genDependKindAttr(lower::AbstractConverter &converter,
pbKind = mlir::omp::ClauseTaskDepend::taskdependinout;
break;
case omp::clause::DependenceType::Mutexinoutset:
+ pbKind = mlir::omp::ClauseTaskDepend::taskdependmutexinoutset;
+ break;
case omp::clause::DependenceType::Inoutset:
+ pbKind = mlir::omp::ClauseTaskDepend::taskdependinoutset;
+ break;
case omp::clause::DependenceType::Depobj:
- TODO(currentLocation,
- "INOUTSET, MUTEXINOUTSET and DEPOBJ dependence-types");
+ TODO(currentLocation, "DEPOBJ dependence-type");
break;
case omp::clause::DependenceType::Sink:
case omp::clause::DependenceType::Source:
diff --git a/flang/test/Lower/OpenMP/Todo/depend-clause-depobj.f90 b/flang/test/Lower/OpenMP/Todo/depend-clause-depobj.f90
index 3bc730f849192b..4e98d77d0bb3e3 100644
--- a/flang/test/Lower/OpenMP/Todo/depend-clause-depobj.f90
+++ b/flang/test/Lower/OpenMP/Todo/depend-clause-depobj.f90
@@ -1,7 +1,7 @@
!RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
!RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
-!CHECK: not yet implemented: INOUTSET, MUTEXINOUTSET and DEPOBJ dependence-types
+!CHECK: not yet implemented: DEPOBJ dependence-type
subroutine f00(x)
integer :: x
diff --git a/flang/test/Lower/OpenMP/Todo/depend-clause-inoutset.f90 b/flang/test/Lower/OpenMP/Todo/depend-clause-inoutset.f90
deleted file mode 100644
index 160893fccdc5f2..00000000000000
--- a/flang/test/Lower/OpenMP/Todo/depend-clause-inoutset.f90
+++ /dev/null
@@ -1,11 +0,0 @@
-!RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
-!RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
-
-!CHECK: not yet implemented: INOUTSET, MUTEXINOUTSET and DEPOBJ dependence-types
-subroutine f00(x)
- integer :: x
- !$omp task depend(inoutset: x)
- x = x + 1
- !$omp end task
-end
-
diff --git a/flang/test/Lower/OpenMP/Todo/depend-clause-mutexinoutset.f90 b/flang/test/Lower/OpenMP/Todo/depend-clause-mutexinoutset.f90
deleted file mode 100644
index 17cc3894c548f1..00000000000000
--- a/flang/test/Lower/OpenMP/Todo/depend-clause-mutexinoutset.f90
+++ /dev/null
@@ -1,11 +0,0 @@
-!RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
-!RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
-
-!CHECK: not yet implemented: INOUTSET, MUTEXINOUTSET and DEPOBJ dependence-types
-subroutine f00(x)
- integer :: x
- !$omp task depend(mutexinoutset: x)
- x = x + 1
- !$omp end task
-end
-
diff --git a/flang/test/Lower/OpenMP/task.f90 b/flang/test/Lower/OpenMP/task.f90
index 6e525a044b011e..f5591bd9d8609d 100644
--- a/flang/test/Lower/OpenMP/task.f90
+++ b/flang/test/Lower/OpenMP/task.f90
@@ -144,6 +144,18 @@ subroutine task_depend_multi_task()
x = x + 12
!CHECK: omp.terminator
!$omp end task
+ !CHECK: omp.task depend(taskdependmutexinoutset -> %{{.+}} : !fir.ref<i32>)
+ !$omp task depend(mutexinoutset : x)
+ !CHECK: arith.subi
+ x = x - 12
+ !CHECK: omp.terminator
+ !$omp end task
+ !CHECK: omp.task depend(taskdependinoutset -> %{{.+}} : !fir.ref<i32>)
+ !$omp task depend(inoutset : x)
+ !CHECK: arith.subi
+ x = x - 12
+ !CHECK: omp.terminator
+ !$omp end task
end subroutine task_depend_multi_task
!===============================================================================
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
index b1a9e3330522b2..2091c0c76dff72 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
@@ -108,14 +108,15 @@ def ClauseRequiresAttr : OpenMP_EnumAttr<ClauseRequires, "clause_requires">;
def ClauseTaskDependIn : I32EnumAttrCase<"taskdependin", 0>;
def ClauseTaskDependOut : I32EnumAttrCase<"taskdependout", 1>;
def ClauseTaskDependInOut : I32EnumAttrCase<"taskdependinout", 2>;
-
-def ClauseTaskDepend : OpenMP_I32EnumAttr<
- "ClauseTaskDepend",
- "depend clause in a target or task construct", [
- ClauseTaskDependIn,
- ClauseTaskDependOut,
- ClauseTaskDependInOut
- ]>;
+def ClauseTaskDependMutexInOutSet
+ : I32EnumAttrCase<"taskdependmutexinoutset", 3>;
+def ClauseTaskDependInOutSet : I32EnumAttrCase<"taskdependinoutset", 4>;
+
+def ClauseTaskDepend
+ : OpenMP_I32EnumAttr<
+ "ClauseTaskDepend", "depend clause in a target or task construct",
+ [ClauseTaskDependIn, ClauseTaskDependOut, ClauseTaskDependInOut,
+ ClauseTaskDependMutexInOutSet, ClauseTaskDependInOutSet]>;
def ClauseTaskDependAttr : OpenMP_EnumAttr<ClauseTaskDepend,
"clause_task_depend"> {
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 060113c4123241..9a30266103b154 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -1701,6 +1701,12 @@ buildDependData(std::optional<ArrayAttr> dependKinds, OperandRange dependVars,
case mlir::omp::ClauseTaskDepend::taskdependinout:
type = llvm::omp::RTLDependenceKindTy::DepInOut;
break;
+ case mlir::omp::ClauseTaskDepend::taskdependmutexinoutset:
+ type = llvm::omp::RTLDependenceKindTy::DepMutexInOutSet;
+ break;
+ case mlir::omp::ClauseTaskDepend::taskdependinoutset:
+ type = llvm::omp::RTLDependenceKindTy::DepInOutSet;
+ break;
};
llvm::Value *depVal = moduleTranslation.lookupValue(std::get<0>(dep));
llvm::OpenMPIRBuilder::DependData dd(type, depVal->getType(), depVal);
diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
index 5f8bdf8afdf783..c76541c8c52d46 100644
--- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -2590,6 +2590,34 @@ llvm.func @omp_task_attrs() -> () attributes {
// CHECK: store i64 8, ptr %[[dep_arr_addr_0_size]], align 4
// CHECK: %[[dep_arr_addr_0_kind:.+]] = getelementptr inbounds nuw %struct.kmp_dep_info, ptr %[[dep_arr_addr_0]], i32 0, i32 2
// CHECK: store i8 1, ptr %[[dep_arr_addr_0_kind]], align 1
+// -----
+// dependence_type: Out
+// CHECK: %[[dep_arr_addr1:.+]] = alloca [1 x %struct.kmp_dep_info], align 8
+// CHECK: %[[dep_arr_addr_1:.+]] = getelementptr inbounds [1 x %struct.kmp_dep_info], ptr %[[dep_arr_addr1]], i64 0, i64 0
+// [...]
+// CHECK: %[[dep_type_1:.+]] = getelementptr inbounds nuw %struct.kmp_dep_info, ptr %[[dep_arr_addr_1]], i32 0, i32 2
+// CHECK: store i8 3, ptr %[[dep_type_1]], align 1
+// -----
+// dependence_type: Inout
+// CHECK: %[[dep_arr_addr2:.+]] = alloca [1 x %struct.kmp_dep_info], align 8
+// CHECK: %[[dep_arr_addr_2:.+]] = getelementptr inbounds [1 x %struct.kmp_dep_info], ptr %[[dep_arr_addr2]], i64 0, i64 0
+// [...]
+// CHECK: %[[dep_type_2:.+]] = getelementptr inbounds nuw %struct.kmp_dep_info, ptr %[[dep_arr_addr_2]], i32 0, i32 2
+// CHECK: store i8 3, ptr %[[dep_type_2]], align 1
+// -----
+// dependence_type: Mutexinoutset
+// CHECK: %[[dep_arr_addr3:.+]] = alloca [1 x %struct.kmp_dep_info], align 8
+// CHECK: %[[dep_arr_addr_3:.+]] = getelementptr inbounds [1 x %struct.kmp_dep_info], ptr %[[dep_arr_addr3]], i64 0, i64 0
+// [...]
+// CHECK: %[[dep_type_3:.+]] = getelementptr inbounds nuw %struct.kmp_dep_info, ptr %[[dep_arr_addr_3]], i32 0, i32 2
+// CHECK: store i8 4, ptr %[[dep_type_3]], align 1
+// -----
+// dependence_type: Inoutset
+// CHECK: %[[dep_arr_addr4:.+]] = alloca [1 x %struct.kmp_dep_info], align 8
+// CHECK: %[[dep_arr_addr_4:.+]] = getelementptr inbounds [1 x %struct.kmp_dep_info], ptr %[[dep_arr_addr4]], i64 0, i64 0
+// [...]
+// CHECK: %[[dep_type_4:.+]] = getelementptr inbounds nuw %struct.kmp_dep_info, ptr %[[dep_arr_addr_4]], i32 0, i32 2
+// CHECK: store i8 8, ptr %[[dep_type_4]], align 1
llvm.func @omp_task_with_deps(%zaddr: !llvm.ptr) {
// CHECK: %[[omp_global_thread_num:.+]] = call i32 @__kmpc_global_thread_num({{.+}})
// CHECK: %[[task_data:.+]] = call ptr @__kmpc_omp_task_alloc
@@ -2604,6 +2632,18 @@ llvm.func @omp_task_with_deps(%zaddr: !llvm.ptr) {
llvm.store %double, %valaddr : i32, !llvm.ptr
omp.terminator
}
+ omp.task depend(taskdependout -> %zaddr : !llvm.ptr) {
+ omp.terminator
+ }
+ omp.task depend(taskdependinout -> %zaddr : !llvm.ptr) {
+ omp.terminator
+ }
+ omp.task depend(taskdependmutexinoutset -> %zaddr : !llvm.ptr) {
+ omp.terminator
+ }
+ omp.task depend(taskdependinoutset -> %zaddr : !llvm.ptr) {
+ omp.terminator
+ }
llvm.return
}
|
Thirumalai-Shaktivel
added
flang
kiranchandramohan
approved these changes
Dec 20, 2024
| // CHECK: store i8 3, ptr %[[dep_type_2]], align 1 | ||
| // ----- | ||
| // dependence_type: Mutexinoutset | ||
| // CHECK: %[[dep_arr_addr3:.+]] = alloca [1 x %struct.kmp_dep_info], align 8 |
There was a problem hiding this comment.
Nit: convert these check line variables to caps for ease of reading.
There was a problem hiding this comment.
Sure, I will update it soon.
dependence_type in depend clause Implementatoin details: Both Mutexinoutset and Inoutset is recognized as flag=0x4 and 0x8 respectively, the flags is set to `kmp_depend_info` and passed as arg to `__kmpc_omp_task_with_deps` runtime call
Thirumalai-Shaktivel
force-pushed
the
llvm/task_depend_01
branch
from
9574c2d to
db9d7f8
Compare
|
Thanks for the review! |
qiaojbao
pushed a commit
to GPUOpen-Drivers/llvm-project
that referenced
this pull request
Feb 7, 2025
Local branch amd-gfx 2cf84b6 Manually merged main:cea738bc9a9e into amd-gfx:0756140e2ff7 Remote branch main cbe583b [Flang] Add translation support for MutexInOutSet and InOutSet (llvm#120715)
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.
Implementatoin details:
Both Mutexinoutset and Inoutset is recognized as flag=0x4
and 0x8 respectively, the flags is set to
kmp_depend_infoandpassed as argument to
__kmpc_omp_task_with_depsruntime call