Skip to content

[Flang][OpenMP] Add LLVM translation support for UNTIED clause in Task #115283

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

Conversation

Thirumalai-Shaktivel
Copy link
Member

Implementation details:
The UNTIED clause is recognized by setting the flag=0 for the default case or performing logical OR to flag if other clauses are specified, and this flag is passed as an argument to the __kmpc_omp_task_alloc runtime call.

Implementation details:
The UNTIED clause is recognized by setting the flag=0 for the default
case or performing logical OR to flag if other clauses are specified,
and this flag is passed as an argument to the `__kmpc_omp_task_alloc`
runtime call.
@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

@llvm/pr-subscribers-mlir-openmp
@llvm/pr-subscribers-mlir-llvm
@llvm/pr-subscribers-flang-fir-hlfir
@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-flang-codegen

@llvm/pr-subscribers-flang-openmp

Author: Thirumalai Shaktivel (Thirumalai-Shaktivel)

Changes

Implementation details:
The UNTIED clause is recognized by setting the flag=0 for the default case or performing logical OR to flag if other clauses are specified, and this flag is passed as an argument to the __kmpc_omp_task_alloc runtime call.


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

4 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+1)
  • (removed) flang/test/Lower/OpenMP/Todo/task_untied.f90 (-13)
  • (modified) flang/test/Lower/OpenMP/task.f90 (+13)
  • (modified) mlir/test/Target/LLVMIR/openmp-llvm.mlir (+11)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 4f9e2347308aa1..597871fb41f6aa 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2844,6 +2844,7 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
         !std::holds_alternative<clause::UseDevicePtr>(clause.u) &&
         !std::holds_alternative<clause::InReduction>(clause.u) &&
         !std::holds_alternative<clause::Mergeable>(clause.u) &&
+        !std::holds_alternative<clause::Untied>(clause.u) &&
         !std::holds_alternative<clause::TaskReduction>(clause.u)) {
       TODO(clauseLocation, "OpenMP Block construct clause");
     }
diff --git a/flang/test/Lower/OpenMP/Todo/task_untied.f90 b/flang/test/Lower/OpenMP/Todo/task_untied.f90
deleted file mode 100644
index 19621c7aac16d6..00000000000000
--- a/flang/test/Lower/OpenMP/Todo/task_untied.f90
+++ /dev/null
@@ -1,13 +0,0 @@
-! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-
-!===============================================================================
-! `untied` clause
-!===============================================================================
-
-! CHECK: not yet implemented: OpenMP Block construct clause
-subroutine omp_task_untied()
-  !$omp task untied
-  call foo()
-  !$omp end task
-end subroutine omp_task_untied
diff --git a/flang/test/Lower/OpenMP/task.f90 b/flang/test/Lower/OpenMP/task.f90
index 4f00f261fe57df..bf0f3574258076 100644
--- a/flang/test/Lower/OpenMP/task.f90
+++ b/flang/test/Lower/OpenMP/task.f90
@@ -245,3 +245,16 @@ subroutine task_multiple_clauses()
   !CHECK: omp.terminator
   !$omp end task
 end subroutine task_multiple_clauses
+
+!===============================================================================
+! `untied` clause
+!===============================================================================
+
+!CHECK-LABEL: func.func @_QPomp_task_untied() {
+subroutine omp_task_untied()
+  !CHECK: omp.task untied {
+  !$omp task untied
+    call foo()
+  !CHECK: omp.terminator
+  !$omp end task
+end subroutine omp_task_untied
diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
index cdf94b1ceae11b..6eb9fbf0421311 100644
--- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -3003,6 +3003,17 @@ module attributes {omp.is_target_device = true} {
 
 // -----
 
+llvm.func @omp_task_untied() {
+  // The third argument is 0: which signifies the united task
+  // CHECK: {{.*}} = call ptr @__kmpc_omp_task_alloc(ptr @1, i32 %{{.*}}, i32 0, i64 40, i64 0, ptr @{{.*}})
+  omp.task untied {
+    omp.terminator
+  }
+  llvm.return
+}
+
+// -----
+
 llvm.func external @foo_before() -> ()
 llvm.func external @foo() -> ()
 llvm.func external @foo_after() -> ()

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

Please could you add a test showing what happens (TODO or correct behavior) if untied is used with taskloop.

Otherwise the changes look good.

@tblah
Copy link
Contributor

tblah commented Nov 7, 2024

See also the failing mlir test Target/LLVMIR/openmp-llvm.mlir

@Thirumalai-Shaktivel
Copy link
Member Author

Yea, sure. I will check it out and report back.

@Thirumalai-Shaktivel
Copy link
Member Author

Please could you add a test showing what happens (TODO or correct behavior) if untied is used with taskloop.

Taskloop doesn't have the OpenMP lowering support (it throws "not yet implemented: omp.taskloop").
Still, I have added a test and check to catch it.

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

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

Thanks for the update. LGTM

@Thirumalai-Shaktivel Thirumalai-Shaktivel merged commit 919aead into llvm:main Dec 20, 2024
8 checks passed
@Thirumalai-Shaktivel Thirumalai-Shaktivel deleted the llvm/task_untied_01 branch December 20, 2024 11:06
@ceseo
Copy link
Contributor

ceseo commented Dec 23, 2024

This seems to be breaking one of the test-suite gfortran tests:

FAIL: test-suite::gfortran-regression-compile-regression__gomp__pr44085_f90.test

@omjavaid
Copy link
Contributor

I have temporarily reverted this change as it was breaking various LLVM bots due to failure of following LLVM testsuite GFortran tests: llvm-test-suite/Fortran/gfortran/regression/gomp/pr44085.f90
Please take a look and re-apply after fix. Thanks!

@Thirumalai-Shaktivel Thirumalai-Shaktivel restored the llvm/task_untied_01 branch December 24, 2024 06:52
Thirumalai-Shaktivel added a commit that referenced this pull request Jan 21, 2025
Implementation details:
The UNTIED clause is recognized by setting the flag=0 for the default
case or performing logical OR to flag if other clauses are specified,
and this flag is passed as an argument to the `__kmpc_omp_task_alloc`
runtime call.


Resubmitting the PR with fix for the failure, as it was reverted here:
927a70d
and previously merged here: #115283
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Jan 21, 2025
…k (#121052)

Implementation details:
The UNTIED clause is recognized by setting the flag=0 for the default
case or performing logical OR to flag if other clauses are specified,
and this flag is passed as an argument to the `__kmpc_omp_task_alloc`
runtime call.

Resubmitting the PR with fix for the failure, as it was reverted here:
927a70d
and previously merged here: llvm/llvm-project#115283
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants