Skip to content

[Flang][OpenMP]Add tests for TODOs and small changes to improve messages #111562

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 2 commits into from
Oct 11, 2024

Conversation

Leporacanthicus
Copy link
Contributor

The bulk of this change are new tests to check that we get a "Not yet implemneted: some stuff here" message when using some not yet supported OpenMP functionality.

For some of these cases, this also means adding additional clauses to a filter list in OpenMP.cpp - this changes nothing [to the best of my understanding] other than allowing the clause to get to the point where it can be rejected in a TODO with a more clear message. One of the TOOD filters were missing Mergeable clause, so this was also added and the existing test updated for the new more specific error message.

There is no functional change intended here.

The bulk of this change are new tests to check that we get a
"Not yet implemneted: *some stuff here*" message when using some not yet
supported OpenMP functionality.

For some of these cases, this also means adding additional clauses to
a filter list in OpenMP.cpp - this changes nothing [to the best of
my understanding] other than allowing the clause to get to the point
where it can be rejected in a TODO with a more clear message. One of
the TOOD filters were missing Mergeable clause, so this was also added
and the existing test updated for the new more specific error message.

There is no functional change intended here.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels Oct 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 8, 2024

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

@llvm/pr-subscribers-flang-openmp

Author: Mats Petersson (Leporacanthicus)

Changes

The bulk of this change are new tests to check that we get a "Not yet implemneted: some stuff here" message when using some not yet supported OpenMP functionality.

For some of these cases, this also means adding additional clauses to a filter list in OpenMP.cpp - this changes nothing [to the best of my understanding] other than allowing the clause to get to the point where it can be rejected in a TODO with a more clear message. One of the TOOD filters were missing Mergeable clause, so this was also added and the existing test updated for the new more specific error message.

There is no functional change intended here.


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

10 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+6-3)
  • (added) flang/test/Lower/OpenMP/Todo/reduction-inscan.f90 (+14)
  • (added) flang/test/Lower/OpenMP/Todo/reduction-task.f90 (+12)
  • (added) flang/test/Lower/OpenMP/Todo/target-inreduction.f90 (+15)
  • (added) flang/test/Lower/OpenMP/Todo/task-inreduction.f90 (+15)
  • (modified) flang/test/Lower/OpenMP/Todo/task_mergeable.f90 (+1-1)
  • (added) flang/test/Lower/OpenMP/Todo/taskgroup-task-reduction.f90 (+10)
  • (added) flang/test/Lower/OpenMP/Todo/taskloop.f90 (+13)
  • (added) flang/test/Lower/OpenMP/Todo/taskwait-depend.f90 (+10)
  • (added) flang/test/Lower/OpenMP/Todo/taskwait-nowait.f90 (+8)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 60c83586e468b6..cbfe1e7235c103 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1255,8 +1255,8 @@ static void genTaskClauses(lower::AbstractConverter &converter,
   cp.processUntied(clauseOps);
   // TODO Support delayed privatization.
 
-  cp.processTODO<clause::Affinity, clause::Detach, clause::InReduction>(
-      loc, llvm::omp::Directive::OMPD_task);
+  cp.processTODO<clause::Affinity, clause::Detach, clause::InReduction,
+                 clause::Mergeable>(loc, llvm::omp::Directive::OMPD_task);
 }
 
 static void genTaskgroupClauses(lower::AbstractConverter &converter,
@@ -2764,7 +2764,10 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
         !std::holds_alternative<clause::ThreadLimit>(clause.u) &&
         !std::holds_alternative<clause::Threads>(clause.u) &&
         !std::holds_alternative<clause::UseDeviceAddr>(clause.u) &&
-        !std::holds_alternative<clause::UseDevicePtr>(clause.u)) {
+        !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::TaskReduction>(clause.u)) {
       TODO(clauseLocation, "OpenMP Block construct clause");
     }
   }
diff --git a/flang/test/Lower/OpenMP/Todo/reduction-inscan.f90 b/flang/test/Lower/OpenMP/Todo/reduction-inscan.f90
new file mode 100644
index 00000000000000..c5f196fe09693a
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/reduction-inscan.f90
@@ -0,0 +1,14 @@
+! 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
+
+! CHECK: not yet implemented: Reduction modifiers are not supported
+subroutine reduction_inscan()
+  integer :: i,j
+  i = 0
+
+  !$omp do reduction(inscan, +:i)
+  do j=1,10
+     i = i + 1
+  end do
+  !$omp end do
+end subroutine reduction_inscan
diff --git a/flang/test/Lower/OpenMP/Todo/reduction-task.f90 b/flang/test/Lower/OpenMP/Todo/reduction-task.f90
new file mode 100644
index 00000000000000..6707f65e1a4cc3
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/reduction-task.f90
@@ -0,0 +1,12 @@
+! 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
+
+! CHECK: not yet implemented: Reduction modifiers are not supported
+subroutine reduction_task()
+  integer :: i
+  i = 0
+
+  !$omp parallel reduction(task, +:i)
+  i = i + 1
+  !$omp end parallel 
+end subroutine reduction_task
diff --git a/flang/test/Lower/OpenMP/Todo/target-inreduction.f90 b/flang/test/Lower/OpenMP/Todo/target-inreduction.f90
new file mode 100644
index 00000000000000..0751c10b04f5ce
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/target-inreduction.f90
@@ -0,0 +1,15 @@
+! 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: Unhandled clause IN_REDUCTION in TARGET construct
+subroutine omp_targer_inreduction()
+  integer i
+  i = 0
+  !$omp target in_reduction(+:i)
+  i = i + 1
+  !$omp end target
+end subroutine omp_targer_inreduction
diff --git a/flang/test/Lower/OpenMP/Todo/task-inreduction.f90 b/flang/test/Lower/OpenMP/Todo/task-inreduction.f90
new file mode 100644
index 00000000000000..aeed680a6dba7c
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/task-inreduction.f90
@@ -0,0 +1,15 @@
+! 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: Unhandled clause IN_REDUCTION in TASK construct
+subroutine omp_task_in_reduction()
+  integer i
+  i = 0
+  !$omp task in_reduction(+:i)
+  i = i + 1
+  !$omp end task
+end subroutine omp_task_in_reduction
diff --git a/flang/test/Lower/OpenMP/Todo/task_mergeable.f90 b/flang/test/Lower/OpenMP/Todo/task_mergeable.f90
index 13145d92ccf902..ddc27487abfe9c 100644
--- a/flang/test/Lower/OpenMP/Todo/task_mergeable.f90
+++ b/flang/test/Lower/OpenMP/Todo/task_mergeable.f90
@@ -5,7 +5,7 @@
 ! `mergeable` clause
 !===============================================================================
 
-! CHECK: not yet implemented: OpenMP Block construct clause
+! CHECK: not yet implemented: Unhandled clause MERGEABLE in TASK construct
 subroutine omp_task_mergeable()
   !$omp task mergeable
   call foo()
diff --git a/flang/test/Lower/OpenMP/Todo/taskgroup-task-reduction.f90 b/flang/test/Lower/OpenMP/Todo/taskgroup-task-reduction.f90
new file mode 100644
index 00000000000000..1cb471d784d766
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/taskgroup-task-reduction.f90
@@ -0,0 +1,10 @@
+! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s -fopenmp-version=50 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s -fopenmp-version=50 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: Unhandled clause TASK_REDUCTION in TASKGROUP construct
+subroutine omp_taskgroup_task_reduction
+  integer :: res
+  !$omp taskgroup task_reduction(+:res)
+  res = res + 1
+  !$omp end taskgroup
+end subroutine omp_taskgroup_task_reduction
diff --git a/flang/test/Lower/OpenMP/Todo/taskloop.f90 b/flang/test/Lower/OpenMP/Todo/taskloop.f90
new file mode 100644
index 00000000000000..aca050584cbbe3
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/taskloop.f90
@@ -0,0 +1,13 @@
+! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s -fopenmp-version=50 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s -fopenmp-version=50 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: Taskloop construct
+subroutine omp_taskloop
+  integer :: res, i
+  !$omp taskloop
+  do i = 1, 10
+     res = res + 1
+  end do
+  !$omp end taskloop
+end subroutine omp_taskloop
+
diff --git a/flang/test/Lower/OpenMP/Todo/taskwait-depend.f90 b/flang/test/Lower/OpenMP/Todo/taskwait-depend.f90
new file mode 100644
index 00000000000000..d1f953be8802fa
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/taskwait-depend.f90
@@ -0,0 +1,10 @@
+! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s -fopenmp-version=50 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s -fopenmp-version=50 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: Unhandled clause DEPEND in TASKWAIT construct
+subroutine omp_tw_depend
+  integer :: res
+  !$omp taskwait depend(out: res)
+  res = res + 1
+end subroutine omp_tw_depend
+
diff --git a/flang/test/Lower/OpenMP/Todo/taskwait-nowait.f90 b/flang/test/Lower/OpenMP/Todo/taskwait-nowait.f90
new file mode 100644
index 00000000000000..21e8609b08ba37
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/taskwait-nowait.f90
@@ -0,0 +1,8 @@
+! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s -fopenmp-version=51 2>&1 | FileCheck %s
+! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s -fopenmp-version=51 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: Unhandled clause NOWAIT in TASKWAIT construct
+subroutine omp_tw_nowait
+  !$omp taskwait nowait
+end subroutine omp_tw_nowait
+

Copy link
Contributor

@kiranchandramohan kiranchandramohan 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 adding the TODOs.

The CI is failing. Please fix before submitting.

!===============================================================================

! CHECK: not yet implemented: Unhandled clause IN_REDUCTION in TARGET construct
subroutine omp_targer_inreduction()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
subroutine omp_targer_inreduction()
subroutine omp_target_inreduction()

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.

LGTM modulo Kiran's comments. Thanks!

@Leporacanthicus Leporacanthicus merged commit 0163ac1 into llvm:main Oct 11, 2024
9 checks passed
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…ges (llvm#111562)

The bulk of this change are new tests to check that we get a "Not yet
implemneted: *some stuff here*" message when using some not yet
supported OpenMP functionality.

For some of these cases, this also means adding additional clauses to a
filter list in OpenMP.cpp - this changes nothing [to the best of my
understanding] other than allowing the clause to get to the point where
it can be rejected in a TODO with a more clear message. One of the TOOD
filters were missing Mergeable clause, so this was also added and the
existing test updated for the new more specific error message.

There is no functional change intended here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants