[flang-commits] [flang] [flang][OpenMP] Diagnose invalid reduction modifiers (PR #92406)

Krzysztof Parzyszek via flang-commits flang-commits at lists.llvm.org
Thu May 16 07:52:39 PDT 2024


https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/92406

>From 2854e272346baad7d5e9a12a8c8243a7c5b210b3 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 16 May 2024 09:40:51 -0500
Subject: [PATCH 1/3] [flang][OpenMP] Diagnose invalid reduction modifiers

Emit diagnostic messages for invalid modifiers in "reduction" clause.

Fixes https://github.com/llvm/llvm-project/issues/92397
---
 flang/lib/Semantics/check-omp-structure.cpp   | 50 ++++++++++
 flang/lib/Semantics/check-omp-structure.h     |  1 +
 .../OpenMP/invalid-reduction-modifier.f90     |  4 +-
 .../Semantics/OpenMP/reduction-modifiers.f90  | 91 +++++++++++++++++++
 4 files changed, 143 insertions(+), 3 deletions(-)
 create mode 100644 flang/test/Semantics/OpenMP/reduction-modifiers.f90

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 2493eb3ed3676..4377f093d062d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2309,6 +2309,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) {
   if (CheckReductionOperators(x)) {
     CheckReductionTypeList(x);
   }
+  CheckReductionModifier(x);
 }
 
 bool OmpStructureChecker::CheckReductionOperators(
@@ -2393,6 +2394,55 @@ void OmpStructureChecker::CheckReductionTypeList(
   }
 }
 
+void OmpStructureChecker::CheckReductionModifier(
+    const parser::OmpClause::Reduction &x) {
+  using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
+  const auto &maybeModifier{std::get<std::optional<ReductionModifier>>(x.v.t)};
+  if (!maybeModifier || *maybeModifier == ReductionModifier::Default) {
+    // No modifier, or the default one is always ok.
+    return;
+  }
+  ReductionModifier modifier{*maybeModifier};
+  const DirectiveContext &dirCtx{GetContext()};
+  if (modifier == ReductionModifier::Task) {
+    // "Task" is only allowed on worksharing or "parallel" directive.
+    static llvm::omp::Directive worksharing[]{
+        llvm::omp::Directive::OMPD_do,
+        // llvm::omp::Directive::OMPD_for, C++ only
+        llvm::omp::Directive::OMPD_loop,
+        llvm::omp::Directive::OMPD_scope,
+        llvm::omp::Directive::OMPD_sections,
+        llvm::omp::Directive::OMPD_single,
+        llvm::omp::Directive::OMPD_workshare,
+    };
+    if (dirCtx.directive != llvm::omp::Directive::OMPD_parallel &&
+        !llvm::is_contained(worksharing, dirCtx.directive)) {
+      context_.Say(GetContext().clauseSource,
+                   "Modifier 'TASK' on REDUCTION clause is only allowed with "
+                   "PARALLEL or worksharing directive"_err_en_US);
+    }
+  } else if (modifier == ReductionModifier::Inscan) {
+    // Inscan is only allowed on worksharing-loop, worksharing-loop simd,
+    // or "simd" directive.
+    // The worksharing-loop directives are OMPD_do and OMPD_for. Only the
+    // former is allowed in Fortran.
+    switch (dirCtx.directive) {
+    case llvm::omp::Directive::OMPD_do:      // worksharing-loop
+    case llvm::omp::Directive::OMPD_do_simd: // worksharing-loop simd
+    case llvm::omp::Directive::OMPD_simd:    // "simd"
+      break;
+    default:
+      context_.Say(GetContext().clauseSource,
+                   "Modifier 'INSCAN' on REDUCTION clause is only allowed with "
+                   "worksharing-loop, worksharing-loop simd, "
+                   "or SIMD directive"_err_en_US);
+    }
+  } else {
+    context_.Say(GetContext().clauseSource, "Unexpected modifier on REDUCTION "
+                                            "clause"_err_en_US);
+  }
+}
+
 void OmpStructureChecker::CheckIntentInPointerAndDefinable(
     const parser::OmpObjectList &objectList, const llvm::omp::Clause clause) {
   for (const auto &ompObject : objectList.v) {
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 1f7284307703b..47705771e8e28 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -205,6 +205,7 @@ class OmpStructureChecker
   bool CheckIntrinsicOperator(
       const parser::DefinedOperator::IntrinsicOperator &);
   void CheckReductionTypeList(const parser::OmpClause::Reduction &);
+  void CheckReductionModifier(const parser::OmpClause::Reduction &);
   void CheckMasterNesting(const parser::OpenMPBlockConstruct &x);
   void ChecksOnOrderedAsBlock();
   void CheckBarrierNesting(const parser::OpenMPSimpleStandaloneConstruct &x);
diff --git a/flang/test/Lower/OpenMP/invalid-reduction-modifier.f90 b/flang/test/Lower/OpenMP/invalid-reduction-modifier.f90
index 53871276761fa..b3e87df7086eb 100644
--- a/flang/test/Lower/OpenMP/invalid-reduction-modifier.f90
+++ b/flang/test/Lower/OpenMP/invalid-reduction-modifier.f90
@@ -1,6 +1,4 @@
-!Remove the --crash below once we can diagnose the issue more gracefully.
-!REQUIRES: asserts
-!RUN: not --crash %flang_fc1 -fopenmp -emit-hlfir -o - %s
+!RUN: not %flang_fc1 -fopenmp -emit-hlfir -o - %s
 
 ! Check that we reject the "task" reduction modifier on the "simd" directive.
 
diff --git a/flang/test/Semantics/OpenMP/reduction-modifiers.f90 b/flang/test/Semantics/OpenMP/reduction-modifiers.f90
new file mode 100644
index 0000000000000..3e6a856f60310
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/reduction-modifiers.f90
@@ -0,0 +1,91 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp -fopenmp-version=52
+
+subroutine mod_task1(x)
+  integer, intent(inout) :: x
+
+  !Correct: "parallel" directive.
+  !$omp parallel reduction(task, +:x)
+  do i = 1, 100
+    x = foo(i)
+  enddo
+  !$omp end parallel
+end
+
+subroutine mod_task2(x)
+  integer, intent(inout) :: x
+
+  !Correct: worksharing directive.
+  !$omp sections reduction(task, +:x)
+  do i = 1, 100
+    x = foo(i)
+  enddo
+  !$omp end sections
+end
+
+
+subroutine mod_task3(x)
+  integer, intent(inout) :: x
+
+  !ERROR: Modifier 'TASK' on REDUCTION clause is only allowed with PARALLEL or worksharing directive
+  !$omp simd reduction(task, +:x)
+  do i = 1, 100
+    x = foo(i)
+  enddo
+  !$omp end simd
+end
+
+subroutine mod_inscan1(x)
+  integer, intent(inout) :: x
+
+  !Correct: worksharing-loop directive
+  !$omp do reduction(inscan, +:x)
+  do i = 1, 100
+    x = foo(i)
+  enddo
+  !$omp end do
+end
+
+subroutine mod_inscan2(x)
+  integer, intent(inout) :: x
+
+  !Correct: worksharing-loop simd directive
+  !$omp do simd reduction(inscan, +:x)
+  do i = 1, 100
+    x = foo(i)
+  enddo
+  !$omp end do simd
+end
+
+subroutine mod_inscan3(x)
+  integer, intent(inout) :: x
+
+  !Correct: "simd" directive
+  !$omp simd reduction(inscan, +:x)
+  do i = 1, 100
+    x = foo(i)
+  enddo
+  !$omp end simd
+end
+
+subroutine mod_inscan4(x)
+  integer, intent(inout) :: x
+
+  !ERROR: Modifier 'INSCAN' on REDUCTION clause is only allowed with worksharing-loop, worksharing-loop simd, or SIMD directive
+  !$omp parallel reduction(inscan, +:x)
+  do i = 1, 100
+    x = foo(i)
+  enddo
+  !$omp end parallel
+end
+
+subroutine mod_inscan5(x)
+  integer, intent(inout) :: x
+
+  !ERROR: Modifier 'INSCAN' on REDUCTION clause is only allowed with worksharing-loop, worksharing-loop simd, or SIMD directive
+  !$omp sections reduction(inscan, +:x)
+  do i = 1, 100
+    x = foo(i)
+  enddo
+  !$omp end sections
+end
+

>From 4dc8ed103b60bff578cca8bc0b42eca5b9715175 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 16 May 2024 09:45:15 -0500
Subject: [PATCH 2/3] Minor formatting changes

---
 flang/lib/Semantics/check-omp-structure.cpp         | 2 +-
 flang/test/Semantics/OpenMP/reduction-modifiers.f90 | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 4377f093d062d..ba4e9b548412d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2422,7 +2422,7 @@ void OmpStructureChecker::CheckReductionModifier(
                    "PARALLEL or worksharing directive"_err_en_US);
     }
   } else if (modifier == ReductionModifier::Inscan) {
-    // Inscan is only allowed on worksharing-loop, worksharing-loop simd,
+    // "Inscan" is only allowed on worksharing-loop, worksharing-loop simd,
     // or "simd" directive.
     // The worksharing-loop directives are OMPD_do and OMPD_for. Only the
     // former is allowed in Fortran.
diff --git a/flang/test/Semantics/OpenMP/reduction-modifiers.f90 b/flang/test/Semantics/OpenMP/reduction-modifiers.f90
index 3e6a856f60310..cf38200ba0a83 100644
--- a/flang/test/Semantics/OpenMP/reduction-modifiers.f90
+++ b/flang/test/Semantics/OpenMP/reduction-modifiers.f90
@@ -22,7 +22,6 @@ subroutine mod_task2(x)
   !$omp end sections
 end
 
-
 subroutine mod_task3(x)
   integer, intent(inout) :: x
 
@@ -88,4 +87,3 @@ subroutine mod_inscan5(x)
   enddo
   !$omp end sections
 end
-

>From 74ad5a22883cecae65226c6d70a4dd34b15ace33 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 16 May 2024 09:52:27 -0500
Subject: [PATCH 3/3] clang-format

---
 flang/lib/Semantics/check-omp-structure.cpp | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index ba4e9b548412d..f20a06dd9f7d5 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2418,8 +2418,8 @@ void OmpStructureChecker::CheckReductionModifier(
     if (dirCtx.directive != llvm::omp::Directive::OMPD_parallel &&
         !llvm::is_contained(worksharing, dirCtx.directive)) {
       context_.Say(GetContext().clauseSource,
-                   "Modifier 'TASK' on REDUCTION clause is only allowed with "
-                   "PARALLEL or worksharing directive"_err_en_US);
+          "Modifier 'TASK' on REDUCTION clause is only allowed with "
+          "PARALLEL or worksharing directive"_err_en_US);
     }
   } else if (modifier == ReductionModifier::Inscan) {
     // "Inscan" is only allowed on worksharing-loop, worksharing-loop simd,
@@ -2427,19 +2427,21 @@ void OmpStructureChecker::CheckReductionModifier(
     // The worksharing-loop directives are OMPD_do and OMPD_for. Only the
     // former is allowed in Fortran.
     switch (dirCtx.directive) {
-    case llvm::omp::Directive::OMPD_do:      // worksharing-loop
+    case llvm::omp::Directive::OMPD_do: // worksharing-loop
     case llvm::omp::Directive::OMPD_do_simd: // worksharing-loop simd
-    case llvm::omp::Directive::OMPD_simd:    // "simd"
+    case llvm::omp::Directive::OMPD_simd: // "simd"
       break;
     default:
       context_.Say(GetContext().clauseSource,
-                   "Modifier 'INSCAN' on REDUCTION clause is only allowed with "
-                   "worksharing-loop, worksharing-loop simd, "
-                   "or SIMD directive"_err_en_US);
+          "Modifier 'INSCAN' on REDUCTION clause is only allowed with "
+          "worksharing-loop, worksharing-loop simd, "
+          "or SIMD directive"_err_en_US);
     }
   } else {
-    context_.Say(GetContext().clauseSource, "Unexpected modifier on REDUCTION "
-                                            "clause"_err_en_US);
+    // Catch-all for potential future modifiers to make sure that this
+    // function is up-tp-date.
+    context_.Say(GetContext().clauseSource,
+          "Unexpected modifier on REDUCTION clause"_err_en_US);
   }
 }
 



More information about the flang-commits mailing list