[Flang] [OpenMP] Add LLVM lowering support for PRIORITY in TASK #120710

Thirumalai-Shaktivel · 2024-12-20T10:11:03Z

Implementation details:
The PRIORITY clause is recognized by setting the flags = 32 to the
__kmpc_omp_task_alloc runtime call. Also, Store the priority-value
to the kmp_task_t struct

Implementation details: The PRIORITY clause is recognized by setting the flags = 32 to the `__kmpc_omp_task_alloc` runtime call. Also, Store the priority-value to the `kmp_task_t` struct

llvmbot · 2024-12-20T10:11:24Z

@llvm/pr-subscribers-mlir-openmp
@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-flang-openmp

Author: Thirumalai Shaktivel (Thirumalai-Shaktivel)

Changes

Implementation details:
The PRIORITY clause is recognized by setting the flags = 32 to the
__kmpc_omp_task_alloc runtime call. Also, Store the priority-value
to the kmp_task_t struct

Full diff: https://github.com/llvm/llvm-project/pull/120710.diff

5 Files Affected:

(modified) llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h (+3-1)
(modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+31-2)
(modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+7-2)
(modified) mlir/test/Target/LLVMIR/openmp-llvm.mlir (+36)
(modified) mlir/test/Target/LLVMIR/openmp-todo.mlir (-11)

diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 9a7c7f72711cc5..c9b08218ca6770 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -1265,12 +1265,14 @@ class OpenMPIRBuilder {
   /// \param EventHandle If present, signifies the event handle as part of
   ///			 the detach clause
   /// \param Mergeable	 If the given task is `mergeable`
+  /// \param priority `priority-value' specifies the execution order of the
+  ///                 tasks that is generated by the construct
   InsertPointOrErrorTy
   createTask(const LocationDescription &Loc, InsertPointTy AllocaIP,
              BodyGenCallbackTy BodyGenCB, bool Tied = true,
              Value *Final = nullptr, Value *IfCondition = nullptr,
              SmallVector<DependData> Dependencies = {}, bool Mergeable = false,
-             Value *EventHandle = nullptr);
+             Value *EventHandle = nullptr, Value *Priority = nullptr);
 
   /// Generator for the taskgroup construct
   ///
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 0d8dbbe3a8a718..7a36aaad277249 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -1818,7 +1818,8 @@ static Value *emitTaskDependencies(
 OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
     const LocationDescription &Loc, InsertPointTy AllocaIP,
     BodyGenCallbackTy BodyGenCB, bool Tied, Value *Final, Value *IfCondition,
-    SmallVector<DependData> Dependencies, bool Mergeable, Value *EventHandle) {
+    SmallVector<DependData> Dependencies, bool Mergeable, Value *EventHandle,
+    Value *Priority) {
 
   if (!updateToLocation(Loc))
     return InsertPointTy();
@@ -1864,7 +1865,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
       Builder, AllocaIP, ToBeDeleted, TaskAllocaIP, "global.tid", false));
 
   OI.PostOutlineCB = [this, Ident, Tied, Final, IfCondition, Dependencies,
-                      Mergeable, EventHandle, TaskAllocaBB,
+                      Mergeable, Priority, EventHandle, TaskAllocaBB,
                       ToBeDeleted](Function &OutlinedFn) mutable {
     // Replace the Stale CI by appropriate RTL function call.
     assert(OutlinedFn.getNumUses() == 1 &&
@@ -1892,6 +1893,8 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
     // Task is not final iff (Flags & 2) == 0.
     // Task is mergeable iff (Flags & 4) == 4.
     // Task is not mergeable iff (Flags & 4) == 0.
+    // Task is priority iff (Flags & 32) == 32.
+    // Task is not priority iff (Flags & 32) == 0.
     // TODO: Handle the other flags.
     Value *Flags = Builder.getInt32(Tied);
     if (Final) {
@@ -1902,6 +1905,8 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
 
     if (Mergeable)
       Flags = Builder.CreateOr(Builder.getInt32(4), Flags);
+    if (Priority)
+      Flags = Builder.CreateOr(Builder.getInt32(32), Flags);
 
     // Argument - `sizeof_kmp_task_t` (TaskSize)
     // Tasksize refers to the size in bytes of kmp_task_t data structure
@@ -1958,6 +1963,30 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
                            SharedsSize);
     }
 
+    if (Priority) {
+      //
+      // The return type of "__kmpc_omp_task_alloc" is "kmp_task_t *",
+      // we populate the priority information into the "kmp_task_t" here
+      //
+      // The struct "kmp_task_t" definition is available in kmp.h
+      // kmp_task_t = { shareds, routine, part_id, data1, data2 }
+      // data2 is used for priority
+      //
+      Type *Int32Ty = Builder.getInt32Ty();
+      // kmp_task_t* => { ptr }
+      Type *taskPtr = StructType::get(VoidPtr);
+      Value *taskGEP = Builder.CreateInBoundsGEP(
+          taskPtr, TaskData,
+          {ConstantInt::get(Int32Ty, 0), ConstantInt::get(Int32Ty, 0)});
+      // kmp_task_t => { ptr, ptr, i32, ptr, ptr }
+      Type *taskStructType = StructType::get(
+          VoidPtr, VoidPtr, Builder.getInt32Ty(), VoidPtr, VoidPtr);
+      Value *priorityData = Builder.CreateInBoundsGEP(
+          taskStructType, taskGEP,
+          {ConstantInt::get(Int32Ty, 0), ConstantInt::get(Int32Ty, 4)});
+      Builder.CreateStore(Priority, priorityData);
+    }
+
     Value *DepArray = nullptr;
     if (Dependencies.size()) {
       InsertPointTy OldIP = Builder.saveIP();
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 060113c4123241..c4e72b16cc221b 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -257,7 +257,6 @@ static LogicalResult checkImplementationStatus(Operation &op) {
       .Case([&](omp::TaskOp op) {
         checkAllocate(op, result);
         checkInReduction(op, result);
-        checkPriority(op, result);
         checkUntied(op, result);
       })
       .Case([&](omp::TaskgroupOp op) {
@@ -273,6 +272,11 @@ static LogicalResult checkImplementationStatus(Operation &op) {
         checkLinear(op, result);
         checkOrder(op, result);
       })
+      .Case([&](omp::TaskloopOp op) {
+        // TODO: Add other clauses check
+        checkPriority(op, result);
+        checkUntied(op, result);
+      })
       .Case([&](omp::ParallelOp op) { checkAllocate(op, result); })
       .Case([&](omp::SimdOp op) {
         checkAligned(op, result);
@@ -1775,7 +1779,8 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
           moduleTranslation.lookupValue(taskOp.getFinal()),
           moduleTranslation.lookupValue(taskOp.getIfExpr()), dds,
           taskOp.getMergeable(),
-          moduleTranslation.lookupValue(taskOp.getEventHandle()));
+          moduleTranslation.lookupValue(taskOp.getEventHandle()),
+          moduleTranslation.lookupValue(taskOp.getPriority()));
 
   if (failed(handleError(afterIP, *taskOp)))
     return failure();
diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
index 5f8bdf8afdf783..fc21a008bf7caa 100644
--- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -2834,6 +2834,42 @@ llvm.func @omp_taskgroup_task(%x: i32, %y: i32, %zaddr: !llvm.ptr) {
 
 // -----
 
+llvm.func @test_01() attributes {sym_visibility = "private"}
+llvm.func @test_02() attributes {sym_visibility = "private"}
+// CHECK-LABEL: define void @_QPomp_task_priority() {
+llvm.func @_QPomp_task_priority() {
+  %0 = llvm.mlir.constant(1 : i64) : i64
+  %1 = llvm.alloca %0 x i32 {bindc_name = "x"} : (i64) -> !llvm.ptr
+  %2 = llvm.mlir.constant(4 : i32) : i32
+  %3 = llvm.mlir.constant(true) : i1
+  %4 = llvm.load %1 : !llvm.ptr -> i32
+// CHECK:   %[[Gid_01:.*]] = call i32 @__kmpc_global_thread_num(ptr {{.*}})
+// CHECK:   %[[I_01:.*]] = call ptr @__kmpc_omp_task_alloc(ptr {{.*}}, i32 %[[Gid_01:.*]], i32 33, i64 40, i64 0, ptr @{{.*}})
+// CHECK:   %[[I_02:.*]] = getelementptr inbounds { ptr }, ptr %[[I_01:.*]], i32 0, i32 0
+// CHECK:   %[[I_03:.*]] = getelementptr inbounds { ptr, ptr, i32, ptr, ptr }, ptr %[[I_02:.*]], i32 0, i32 4
+// CHECK:   store i32 %[[I_00:.*]], ptr %[[I_03:.*]], align 4
+// CHECK:   %{{.*}} = call i32 @__kmpc_omp_task(ptr {{.*}}, i32 %[[Gid_01:.*]], ptr %[[I_01:.*]])
+  omp.task priority(%4 : i32) {
+    llvm.call @test_01() : () -> ()
+    omp.terminator
+  }
+// CHECK:   %[[Gid_02:.*]] = call i32 @__kmpc_global_thread_num(ptr {{.*}})
+// CHECK:   %[[I_04:.*]] = call ptr @__kmpc_omp_task_alloc(ptr {{.*}}, i32 %[[Gid_02:.*]], i32 35, i64 40, i64 0, ptr @{{.*}})
+// CHECK:   %[[I_05:.*]] = getelementptr inbounds { ptr }, ptr %[[I_04:.*]], i32 0, i32 0
+// CHECK:   %[[I_06:.*]] = getelementptr inbounds { ptr, ptr, i32, ptr, ptr }, ptr %[[I_05:.*]], i32 0, i32 4
+// CHECK:   store i32 4, ptr %[[I_06:.*]], align 4
+// CHECK:   %{{.*}} = call i32 @__kmpc_omp_task(ptr {{.*}}, i32 %[[Gid_02:.*]], ptr %[[I_04:.*]])
+  omp.task final(%3) priority(%2 : i32) {
+    llvm.call @test_02() : () -> ()
+    omp.terminator
+  }
+  llvm.return
+// CHECK:   ret void
+// CHECK: }
+}
+
+// -----
+
 // CHECK-LABEL: @omp_opaque_pointers
 // CHECK-SAME: (ptr %[[ARG0:.*]], ptr %[[ARG1:.*]], i32 %[[EXPR:.*]])
 llvm.func @omp_opaque_pointers(%arg0 : !llvm.ptr, %arg1: !llvm.ptr, %expr: i32) -> () {
diff --git a/mlir/test/Target/LLVMIR/openmp-todo.mlir b/mlir/test/Target/LLVMIR/openmp-todo.mlir
index 8f3e466cfbbeb6..6bbf34502b4970 100644
--- a/mlir/test/Target/LLVMIR/openmp-todo.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir
@@ -429,17 +429,6 @@ llvm.func @task_in_reduction(%x : !llvm.ptr) {
 
 // -----
 
-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}}
-  omp.task priority(%x : i32) {
-    omp.terminator
-  }
-  llvm.return
-}
-
-// -----
-
 llvm.func @task_untied() {
   // expected-error@below {{not yet implemented: Unhandled clause untied in omp.task operation}}
   // expected-error@below {{LLVM Translation failed for operation: omp.task}}

llvmbot · 2024-12-20T10:11:24Z

@llvm/pr-subscribers-flang-codegen

Author: Thirumalai Shaktivel (Thirumalai-Shaktivel)

Changes

Implementation details:
The PRIORITY clause is recognized by setting the flags = 32 to the
__kmpc_omp_task_alloc runtime call. Also, Store the priority-value
to the kmp_task_t struct

Full diff: https://github.com/llvm/llvm-project/pull/120710.diff

5 Files Affected:

(modified) llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h (+3-1)
(modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+31-2)
(modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+7-2)
(modified) mlir/test/Target/LLVMIR/openmp-llvm.mlir (+36)
(modified) mlir/test/Target/LLVMIR/openmp-todo.mlir (-11)

diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 9a7c7f72711cc5..c9b08218ca6770 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -1265,12 +1265,14 @@ class OpenMPIRBuilder {
   /// \param EventHandle If present, signifies the event handle as part of
   ///			 the detach clause
   /// \param Mergeable	 If the given task is `mergeable`
+  /// \param priority `priority-value' specifies the execution order of the
+  ///                 tasks that is generated by the construct
   InsertPointOrErrorTy
   createTask(const LocationDescription &Loc, InsertPointTy AllocaIP,
              BodyGenCallbackTy BodyGenCB, bool Tied = true,
              Value *Final = nullptr, Value *IfCondition = nullptr,
              SmallVector<DependData> Dependencies = {}, bool Mergeable = false,
-             Value *EventHandle = nullptr);
+             Value *EventHandle = nullptr, Value *Priority = nullptr);
 
   /// Generator for the taskgroup construct
   ///
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 0d8dbbe3a8a718..7a36aaad277249 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -1818,7 +1818,8 @@ static Value *emitTaskDependencies(
 OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
     const LocationDescription &Loc, InsertPointTy AllocaIP,
     BodyGenCallbackTy BodyGenCB, bool Tied, Value *Final, Value *IfCondition,
-    SmallVector<DependData> Dependencies, bool Mergeable, Value *EventHandle) {
+    SmallVector<DependData> Dependencies, bool Mergeable, Value *EventHandle,
+    Value *Priority) {
 
   if (!updateToLocation(Loc))
     return InsertPointTy();
@@ -1864,7 +1865,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
       Builder, AllocaIP, ToBeDeleted, TaskAllocaIP, "global.tid", false));
 
   OI.PostOutlineCB = [this, Ident, Tied, Final, IfCondition, Dependencies,
-                      Mergeable, EventHandle, TaskAllocaBB,
+                      Mergeable, Priority, EventHandle, TaskAllocaBB,
                       ToBeDeleted](Function &OutlinedFn) mutable {
     // Replace the Stale CI by appropriate RTL function call.
     assert(OutlinedFn.getNumUses() == 1 &&
@@ -1892,6 +1893,8 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
     // Task is not final iff (Flags & 2) == 0.
     // Task is mergeable iff (Flags & 4) == 4.
     // Task is not mergeable iff (Flags & 4) == 0.
+    // Task is priority iff (Flags & 32) == 32.
+    // Task is not priority iff (Flags & 32) == 0.
     // TODO: Handle the other flags.
     Value *Flags = Builder.getInt32(Tied);
     if (Final) {
@@ -1902,6 +1905,8 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
 
     if (Mergeable)
       Flags = Builder.CreateOr(Builder.getInt32(4), Flags);
+    if (Priority)
+      Flags = Builder.CreateOr(Builder.getInt32(32), Flags);
 
     // Argument - `sizeof_kmp_task_t` (TaskSize)
     // Tasksize refers to the size in bytes of kmp_task_t data structure
@@ -1958,6 +1963,30 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
                            SharedsSize);
     }
 
+    if (Priority) {
+      //
+      // The return type of "__kmpc_omp_task_alloc" is "kmp_task_t *",
+      // we populate the priority information into the "kmp_task_t" here
+      //
+      // The struct "kmp_task_t" definition is available in kmp.h
+      // kmp_task_t = { shareds, routine, part_id, data1, data2 }
+      // data2 is used for priority
+      //
+      Type *Int32Ty = Builder.getInt32Ty();
+      // kmp_task_t* => { ptr }
+      Type *taskPtr = StructType::get(VoidPtr);
+      Value *taskGEP = Builder.CreateInBoundsGEP(
+          taskPtr, TaskData,
+          {ConstantInt::get(Int32Ty, 0), ConstantInt::get(Int32Ty, 0)});
+      // kmp_task_t => { ptr, ptr, i32, ptr, ptr }
+      Type *taskStructType = StructType::get(
+          VoidPtr, VoidPtr, Builder.getInt32Ty(), VoidPtr, VoidPtr);
+      Value *priorityData = Builder.CreateInBoundsGEP(
+          taskStructType, taskGEP,
+          {ConstantInt::get(Int32Ty, 0), ConstantInt::get(Int32Ty, 4)});
+      Builder.CreateStore(Priority, priorityData);
+    }
+
     Value *DepArray = nullptr;
     if (Dependencies.size()) {
       InsertPointTy OldIP = Builder.saveIP();
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 060113c4123241..c4e72b16cc221b 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -257,7 +257,6 @@ static LogicalResult checkImplementationStatus(Operation &op) {
       .Case([&](omp::TaskOp op) {
         checkAllocate(op, result);
         checkInReduction(op, result);
-        checkPriority(op, result);
         checkUntied(op, result);
       })
       .Case([&](omp::TaskgroupOp op) {
@@ -273,6 +272,11 @@ static LogicalResult checkImplementationStatus(Operation &op) {
         checkLinear(op, result);
         checkOrder(op, result);
       })
+      .Case([&](omp::TaskloopOp op) {
+        // TODO: Add other clauses check
+        checkPriority(op, result);
+        checkUntied(op, result);
+      })
       .Case([&](omp::ParallelOp op) { checkAllocate(op, result); })
       .Case([&](omp::SimdOp op) {
         checkAligned(op, result);
@@ -1775,7 +1779,8 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
           moduleTranslation.lookupValue(taskOp.getFinal()),
           moduleTranslation.lookupValue(taskOp.getIfExpr()), dds,
           taskOp.getMergeable(),
-          moduleTranslation.lookupValue(taskOp.getEventHandle()));
+          moduleTranslation.lookupValue(taskOp.getEventHandle()),
+          moduleTranslation.lookupValue(taskOp.getPriority()));
 
   if (failed(handleError(afterIP, *taskOp)))
     return failure();
diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
index 5f8bdf8afdf783..fc21a008bf7caa 100644
--- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -2834,6 +2834,42 @@ llvm.func @omp_taskgroup_task(%x: i32, %y: i32, %zaddr: !llvm.ptr) {
 
 // -----
 
+llvm.func @test_01() attributes {sym_visibility = "private"}
+llvm.func @test_02() attributes {sym_visibility = "private"}
+// CHECK-LABEL: define void @_QPomp_task_priority() {
+llvm.func @_QPomp_task_priority() {
+  %0 = llvm.mlir.constant(1 : i64) : i64
+  %1 = llvm.alloca %0 x i32 {bindc_name = "x"} : (i64) -> !llvm.ptr
+  %2 = llvm.mlir.constant(4 : i32) : i32
+  %3 = llvm.mlir.constant(true) : i1
+  %4 = llvm.load %1 : !llvm.ptr -> i32
+// CHECK:   %[[Gid_01:.*]] = call i32 @__kmpc_global_thread_num(ptr {{.*}})
+// CHECK:   %[[I_01:.*]] = call ptr @__kmpc_omp_task_alloc(ptr {{.*}}, i32 %[[Gid_01:.*]], i32 33, i64 40, i64 0, ptr @{{.*}})
+// CHECK:   %[[I_02:.*]] = getelementptr inbounds { ptr }, ptr %[[I_01:.*]], i32 0, i32 0
+// CHECK:   %[[I_03:.*]] = getelementptr inbounds { ptr, ptr, i32, ptr, ptr }, ptr %[[I_02:.*]], i32 0, i32 4
+// CHECK:   store i32 %[[I_00:.*]], ptr %[[I_03:.*]], align 4
+// CHECK:   %{{.*}} = call i32 @__kmpc_omp_task(ptr {{.*}}, i32 %[[Gid_01:.*]], ptr %[[I_01:.*]])
+  omp.task priority(%4 : i32) {
+    llvm.call @test_01() : () -> ()
+    omp.terminator
+  }
+// CHECK:   %[[Gid_02:.*]] = call i32 @__kmpc_global_thread_num(ptr {{.*}})
+// CHECK:   %[[I_04:.*]] = call ptr @__kmpc_omp_task_alloc(ptr {{.*}}, i32 %[[Gid_02:.*]], i32 35, i64 40, i64 0, ptr @{{.*}})
+// CHECK:   %[[I_05:.*]] = getelementptr inbounds { ptr }, ptr %[[I_04:.*]], i32 0, i32 0
+// CHECK:   %[[I_06:.*]] = getelementptr inbounds { ptr, ptr, i32, ptr, ptr }, ptr %[[I_05:.*]], i32 0, i32 4
+// CHECK:   store i32 4, ptr %[[I_06:.*]], align 4
+// CHECK:   %{{.*}} = call i32 @__kmpc_omp_task(ptr {{.*}}, i32 %[[Gid_02:.*]], ptr %[[I_04:.*]])
+  omp.task final(%3) priority(%2 : i32) {
+    llvm.call @test_02() : () -> ()
+    omp.terminator
+  }
+  llvm.return
+// CHECK:   ret void
+// CHECK: }
+}
+
+// -----
+
 // CHECK-LABEL: @omp_opaque_pointers
 // CHECK-SAME: (ptr %[[ARG0:.*]], ptr %[[ARG1:.*]], i32 %[[EXPR:.*]])
 llvm.func @omp_opaque_pointers(%arg0 : !llvm.ptr, %arg1: !llvm.ptr, %expr: i32) -> () {
diff --git a/mlir/test/Target/LLVMIR/openmp-todo.mlir b/mlir/test/Target/LLVMIR/openmp-todo.mlir
index 8f3e466cfbbeb6..6bbf34502b4970 100644
--- a/mlir/test/Target/LLVMIR/openmp-todo.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir
@@ -429,17 +429,6 @@ llvm.func @task_in_reduction(%x : !llvm.ptr) {
 
 // -----
 
-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}}
-  omp.task priority(%x : i32) {
-    omp.terminator
-  }
-  llvm.return
-}
-
-// -----
-
 llvm.func @task_untied() {
   // expected-error@below {{not yet implemented: Unhandled clause untied in omp.task operation}}
   // expected-error@below {{LLVM Translation failed for operation: omp.task}}

Thirumalai-Shaktivel · 2024-12-20T10:11:45Z

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

+        // TODO: Add other clauses check
+        checkPriority(op, result);
+        checkUntied(op, result);
+      })


These changes are taken from here: https://github.com/llvm/llvm-project/pull/115283/files#diff-2cbb5651f4570d81d55ac4198deda0f6f7341b2503479752ef2295da3774c586R266-R269.

I will merge that PR soon.

kiranchandramohan

Thanks for the patch. A few minor comments.

kiranchandramohan · 2024-12-20T12:32:17Z

mlir/test/Target/LLVMIR/openmp-llvm.mlir

+  %2 = llvm.mlir.constant(4 : i32) : i32
+  %3 = llvm.mlir.constant(true) : i1
+  %4 = llvm.load %1 : !llvm.ptr -> i32
+// CHECK:   %[[Gid_01:.*]] = call i32 @__kmpc_global_thread_num(ptr {{.*}})


Suggested change

// CHECK: %[[Gid_01:.*]] = call i32 @__kmpc_global_thread_num(ptr {{.*}})

// CHECK: %[[GID_01:.*]] = call i32 @__kmpc_global_thread_num(ptr {{.*}})

kiranchandramohan · 2024-12-20T12:32:33Z

mlir/test/Target/LLVMIR/openmp-llvm.mlir

+    llvm.call @test_01() : () -> ()
+    omp.terminator
+  }
+// CHECK:   %[[Gid_02:.*]] = call i32 @__kmpc_global_thread_num(ptr {{.*}})


Suggested change

// CHECK: %[[Gid_02:.*]] = call i32 @__kmpc_global_thread_num(ptr {{.*}})

// CHECK: %[[GID_02:.*]] = call i32 @__kmpc_global_thread_num(ptr {{.*}})

kiranchandramohan · 2024-12-20T12:36:39Z

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

+      //
+      Type *Int32Ty = Builder.getInt32Ty();
+      // kmp_task_t* => { ptr }
+      Type *taskPtr = StructType::get(VoidPtr);


Suggested change

Type *taskPtr = StructType::get(VoidPtr);

Type *TaskPtr = StructType::get(VoidPtr);

And for other variables. While this is slightly confusing because MLIR's is somewhat different but we have to stick to the guidelines.
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

Variable names should be nouns (as they represent state). The name should be camel case, and start with an upper case letter (e.g. Leader or Boats).

Sure, thanks for the information.

kiranchandramohan · 2024-12-20T12:52:31Z

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

+      Type *taskStructType = StructType::get(
+          VoidPtr, VoidPtr, Builder.getInt32Ty(), VoidPtr, VoidPtr);
+      Value *priorityData = Builder.CreateInBoundsGEP(
+          taskStructType, taskGEP,
+          {ConstantInt::get(Int32Ty, 0), ConstantInt::get(Int32Ty, 4)});
+      Builder.CreateStore(Priority, priorityData);


Since data2 is another struct ( kmp_cmplrdata), Is this sufficient because priority is the first field of kmp_cmplrdata, or do we have to index into kmp_cmplrdata as well?

typedef union kmp_cmplrdata { kmp_int32 priority; /**< priority specified by user for the task */ kmp_routine_entry_t destructors; /* pointer to function to invoke deconstructors of firstprivate C++ objects */ /* future data */ } kmp_cmplrdata_t; /* sizeof_kmp_task_t passed as arg to kmpc_omp_task call */ /*! */ typedef struct kmp_task { /* GEH: Shouldn't this be aligned somehow? */ void *shareds; /**< pointer to block of pointers to shared vars */ kmp_routine_entry_t routine; /**< pointer to routine to call for executing task */ kmp_int32 part_id; /**< part id for the task */ kmp_cmplrdata_t data1; /* Two known optional additions: destructors and priority */ kmp_cmplrdata_t data2; /* Process destructors first, priority second */ /* future data */ /* private vars */ } kmp_task_t;

Is this sufficient because priority is the first field of kmp_cmplrdata

Yes, because I checked whether the priority value is passed to runtime by printing the value inside the __kmpc_omp_task, and it prints the correct value.

But, after thinking about this, we should indeed index through kmp_cmplrdata_t (which might reduce some bugs for the future.)

Thirumalai-Shaktivel · 2024-12-26T05:01:16Z

Done, @kiranchandramohan, can you please review this again, so that we can merge it?

github-actions · 2024-12-26T05:03:41Z

✅ With the latest revision this PR passed the C/C++ code formatter.

tblah

LGTM, thanks!

Thirumalai-Shaktivel · 2025-01-16T04:31:42Z

Thanks for the review @tblah!

[Flang] [OpenMP] Add LLVM lowering support for PRIORITY in TASK

ce92e4e

Implementation details: The PRIORITY clause is recognized by setting the flags = 32 to the `__kmpc_omp_task_alloc` runtime call. Also, Store the priority-value to the `kmp_task_t` struct

Thirumalai-Shaktivel added flang Flang issues not falling into any other category flang:openmp flang:codegen labels Dec 20, 2024

Thirumalai-Shaktivel requested review from tblah, kiranchandramohan and kparzysz December 20, 2024 10:11

llvmbot added mlir:llvm mlir mlir:openmp clang:openmp OpenMP related changes to Clang labels Dec 20, 2024

Thirumalai-Shaktivel commented Dec 20, 2024

View reviewed changes

fix: Remove the duplicate check

81adf56

kiranchandramohan reviewed Dec 20, 2024

View reviewed changes

Add another GEP to get the actual priority location

4309b80

Thirumalai-Shaktivel force-pushed the llvm/priority_01 branch from 19a2b17 to 4309b80 Compare December 26, 2024 04:58

Fix Variable name to follow the guidelines

287a709

Thirumalai-Shaktivel requested a review from kiranchandramohan December 26, 2024 05:01

Fix formatting

8702618

tblah approved these changes Jan 2, 2025

View reviewed changes

Thirumalai-Shaktivel merged commit 1d890b0 into llvm:main Jan 16, 2025
8 checks passed

Thirumalai-Shaktivel deleted the llvm/priority_01 branch January 16, 2025 04:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Flang] [OpenMP] Add LLVM lowering support for PRIORITY in TASK #120710

[Flang] [OpenMP] Add LLVM lowering support for PRIORITY in TASK #120710

Uh oh!

Thirumalai-Shaktivel commented Dec 20, 2024

Uh oh!

llvmbot commented Dec 20, 2024 •

edited

Loading

Uh oh!

llvmbot commented Dec 20, 2024

Uh oh!

Thirumalai-Shaktivel Dec 20, 2024

Uh oh!

kiranchandramohan left a comment

Uh oh!

kiranchandramohan Dec 20, 2024

Uh oh!

kiranchandramohan Dec 20, 2024

Uh oh!

kiranchandramohan Dec 20, 2024

Uh oh!

Thirumalai-Shaktivel Dec 26, 2024

Uh oh!

kiranchandramohan Dec 20, 2024

Uh oh!

Thirumalai-Shaktivel Dec 26, 2024 •

edited

Loading

Uh oh!

Thirumalai-Shaktivel commented Dec 26, 2024

Uh oh!

github-actions bot commented Dec 26, 2024 •

edited

Loading

Uh oh!

tblah left a comment

Uh oh!

Thirumalai-Shaktivel commented Jan 16, 2025

Uh oh!

Uh oh!

Uh oh!

	// CHECK: %[[Gid_01:.]] = call i32 @__kmpc_global_thread_num(ptr {{.}})
	// CHECK: %[[GID_01:.]] = call i32 @__kmpc_global_thread_num(ptr {{.}})

	// CHECK: %[[Gid_02:.]] = call i32 @__kmpc_global_thread_num(ptr {{.}})
	// CHECK: %[[GID_02:.]] = call i32 @__kmpc_global_thread_num(ptr {{.}})

	Type *taskPtr = StructType::get(VoidPtr);
	Type *TaskPtr = StructType::get(VoidPtr);

[Flang] [OpenMP] Add LLVM lowering support for PRIORITY in TASK #120710

[Flang] [OpenMP] Add LLVM lowering support for PRIORITY in TASK #120710

Uh oh!

Conversation

Thirumalai-Shaktivel commented Dec 20, 2024

Uh oh!

llvmbot commented Dec 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 20, 2024

Uh oh!

Thirumalai-Shaktivel Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan left a comment

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

Thirumalai-Shaktivel Dec 26, 2024

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

Thirumalai-Shaktivel Dec 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Thirumalai-Shaktivel commented Dec 26, 2024

Uh oh!

github-actions bot commented Dec 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

Thirumalai-Shaktivel commented Jan 16, 2025

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Dec 20, 2024 •

edited

Loading

Thirumalai-Shaktivel Dec 26, 2024 •

edited

Loading

github-actions bot commented Dec 26, 2024 •

edited

Loading