[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:56:47 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/4] [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/4] 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/4] 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);
}
}
>From 903ffaf25625e63d1a35506b21bd5ce1de820184 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 16 May 2024 09:56:28 -0500
Subject: [PATCH 4/4] More clang-format
---
flang/lib/Semantics/check-omp-structure.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index f20a06dd9f7d5..5c057c5149fc8 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2441,7 +2441,7 @@ void OmpStructureChecker::CheckReductionModifier(
// 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);
+ "Unexpected modifier on REDUCTION clause"_err_en_US);
}
}
More information about the flang-commits
mailing list