[Flang][OpenMP] Mark mergeable and untied clauses as unsupported

[kiranchandramohan]

These two clauses are not supported in the OpenMP Dialect to LLVM conversion. Mark as TODO till support is added.

[llvmbot]

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Kiran Chandramohan (kiranchandramohan)

Changes

These two clauses are not supported in the OpenMP Dialect to LLVM conversion. Mark as TODO till support is added.

---

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

5 Files Affected:

• (modified) flang/lib/Lower/OpenMP.cpp (-2)
• (modified) flang/test/Lower/OpenMP/FIR/task.f90 (+2-30)
• (added) flang/test/Lower/OpenMP/Todo/task_mergeable.f90 (+13)
• (added) flang/test/Lower/OpenMP/Todo/task_untied.f90 (+13)
• (modified) flang/test/Lower/OpenMP/task.f90 (+2-30)

diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 4a73ee87579c71a..5297aefe3df9569 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -2868,8 +2868,6 @@ genOMP(Fortran::lower::AbstractConverter &converter,
         !std::get_if<Fortran::parser::OmpClause::Allocate>(&clause.u) &&
         !std::get_if<Fortran::parser::OmpClause::Default>(&clause.u) &&
         !std::get_if<Fortran::parser::OmpClause::Final>(&clause.u) &&
-        !std::get_if<Fortran::parser::OmpClause::Untied>(&clause.u) &&
-        !std::get_if<Fortran::parser::OmpClause::Mergeable>(&clause.u) &&
         !std::get_if<Fortran::parser::OmpClause::Priority>(&clause.u) &&
         !std::get_if<Fortran::parser::OmpClause::Reduction>(&clause.u) &&
         !std::get_if<Fortran::parser::OmpClause::Depend>(&clause.u) &&
diff --git a/flang/test/Lower/OpenMP/FIR/task.f90 b/flang/test/Lower/OpenMP/FIR/task.f90
index d7419bd1100e690..99a9e3a6b1e35aa 100644
--- a/flang/test/Lower/OpenMP/FIR/task.f90
+++ b/flang/test/Lower/OpenMP/FIR/task.f90
@@ -40,34 +40,6 @@ subroutine omp_task_final(bar)
   !$omp end task
 end subroutine omp_task_final
 
-!===============================================================================
-! `untied` clause
-!===============================================================================
-
-!CHECK-LABEL: func @_QPomp_task_untied() {
-subroutine omp_task_untied()
-  !CHECK: omp.task untied {
-  !$omp task untied
-  !CHECK: fir.call @_QPfoo() {{.*}}: () -> ()
-  call foo()
-  !CHECK: omp.terminator
-  !$omp end task
-end subroutine omp_task_untied
-
-!===============================================================================
-! `mergeable` clause
-!===============================================================================
-
-!CHECK-LABEL: func @_QPomp_task_mergeable() {
-subroutine omp_task_mergeable()
-  !CHECK: omp.task mergeable {
-  !$omp task mergeable
-  !CHECK: fir.call @_QPfoo() {{.*}}: () -> ()
-  call foo()
-  !CHECK: omp.terminator
-  !$omp end task
-end subroutine omp_task_mergeable
-
 !===============================================================================
 ! `priority` clause
 !===============================================================================
@@ -245,8 +217,8 @@ subroutine task_multiple_clauses()
   integer :: x, y, z
   logical :: buzz
 
-  !CHECK: omp.task if(%{{.+}}) final(%{{.+}}) untied mergeable priority(%{{.+}}) allocate(%{{.+}} : i32 -> %{{.+}} : !fir.ref<i32>) {
-  !$omp task if(buzz) final(buzz) untied mergeable priority(z) allocate(omp_high_bw_mem_alloc: x) private(x) firstprivate(y)
+  !CHECK: omp.task if(%{{.+}}) final(%{{.+}}) priority(%{{.+}}) allocate(%{{.+}} : i32 -> %{{.+}} : !fir.ref<i32>) {
+  !$omp task if(buzz) final(buzz) priority(z) allocate(omp_high_bw_mem_alloc: x) private(x) firstprivate(y)
 
   !CHECK: %[[x_priv:.+]] = fir.alloca i32
   !CHECK: %[[y_priv:.+]] = fir.alloca i32
diff --git a/flang/test/Lower/OpenMP/Todo/task_mergeable.f90 b/flang/test/Lower/OpenMP/Todo/task_mergeable.f90
new file mode 100644
index 000000000000000..13145d92ccf902e
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/task_mergeable.f90
@@ -0,0 +1,13 @@
+! 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
+
+!===============================================================================
+! `mergeable` clause
+!===============================================================================
+
+! CHECK: not yet implemented: OpenMP Block construct clause
+subroutine omp_task_mergeable()
+  !$omp task mergeable
+  call foo()
+  !$omp end task
+end subroutine omp_task_mergeable
diff --git a/flang/test/Lower/OpenMP/Todo/task_untied.f90 b/flang/test/Lower/OpenMP/Todo/task_untied.f90
new file mode 100644
index 000000000000000..19621c7aac16d6e
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/task_untied.f90
@@ -0,0 +1,13 @@
+! 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 99b1740ca75a81c..7d7a79af3185f52 100644
--- a/flang/test/Lower/OpenMP/task.f90
+++ b/flang/test/Lower/OpenMP/task.f90
@@ -40,34 +40,6 @@ subroutine omp_task_final(bar)
   !$omp end task
 end subroutine omp_task_final
 
-!===============================================================================
-! `untied` clause
-!===============================================================================
-
-!CHECK-LABEL: func @_QPomp_task_untied() {
-subroutine omp_task_untied()
-  !CHECK: omp.task untied {
-  !$omp task untied
-  !CHECK: fir.call @_QPfoo() {{.*}}: () -> ()
-  call foo()
-  !CHECK: omp.terminator
-  !$omp end task
-end subroutine omp_task_untied
-
-!===============================================================================
-! `mergeable` clause
-!===============================================================================
-
-!CHECK-LABEL: func @_QPomp_task_mergeable() {
-subroutine omp_task_mergeable()
-  !CHECK: omp.task mergeable {
-  !$omp task mergeable
-  !CHECK: fir.call @_QPfoo() {{.*}}: () -> ()
-  call foo()
-  !CHECK: omp.terminator
-  !$omp end task
-end subroutine omp_task_mergeable
-
 !===============================================================================
 ! `priority` clause
 !===============================================================================
@@ -253,8 +225,8 @@ subroutine task_multiple_clauses()
   integer :: x, y, z
   logical :: buzz
 
-  !CHECK: omp.task if(%{{.+}}) final(%{{.+}}) untied mergeable priority(%{{.+}}) allocate(%{{.+}} : i32 -> %{{.+}} : !fir.ref<i32>) {
-  !$omp task if(buzz) final(buzz) untied mergeable priority(z) allocate(omp_high_bw_mem_alloc: x) private(x) firstprivate(y)
+  !CHECK: omp.task if(%{{.+}}) final(%{{.+}}) priority(%{{.+}}) allocate(%{{.+}} : i32 -> %{{.+}} : !fir.ref<i32>) {
+  !$omp task if(buzz) final(buzz) priority(z) allocate(omp_high_bw_mem_alloc: x) private(x) firstprivate(y)
 
 !CHECK: %[[X_PRIV_ALLOCA:.+]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFtask_multiple_clausesEx"}
 !CHECK: %[[X_PRIV:.+]]:2 = hlfir.declare %[[X_PRIV_ALLOCA]] {uniq_name = "_QFtask_multiple_clausesEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)

[shraiysh]

MLIR reports an error of the kind unhandled clauses for translation to LLVM IR when lowering. Although it is a bit ugly because of the crash dump. If it is straightforward, can we change that to report a clean "todo" message without the backtrace just for OpenMP Dialect whenever a clause fails? I am just worried that these (and more similar) changes will have to land again once the translation is supported. If that change in MLIR is too complicated, this patch LGTM.

[kiranchandramohan]

Thanks @shraiysh. That is a good point. MLIR emitError messages during translation, would be detected as a failure by the Driver and then it does the default handling which is to issue a crash dump with segfault. In most cases (unlike OpenMP) an error in this stage (MLIR to LLVM translation) would mean something really bad has happened. To handle this appropriately, we will have to add an emitTODO into MLIR and then catch it appropriately in the Driver.
Since there is more than one component (MLIR, Flang Driver) involved, I think this might need a discussion. I would like to go forward with this patch for now

[shraiysh]

LGTM in that case.