[flang][OpenMP] Diagnose invalid reduction modifiers #92406
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Emit diagnostic messages for invalid modifiers in "reduction" clause. Fixes llvm#92397
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-flang-semantics Author: Krzysztof Parzyszek (kparzysz) ChangesEmit diagnostic messages for invalid modifiers in "reduction" clause. Fixes #92397 Full diff: https://github.com/llvm/llvm-project/pull/92406.diff 4 Files Affected:
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
+
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| } | ||
| } else { | ||
| // Catch-all for potential future modifiers to make sure that this | ||
| // function is up-tp-date. |
There was a problem hiding this comment.
Suggested change
| // function is up-tp-date. | |
| // function is up-to-date. |
| static llvm::omp::Directive worksharing[]{ | ||
| llvm::omp::Directive::OMPD_do, | ||
| // llvm::omp::Directive::OMPD_for, C++ only | ||
| llvm::omp::Directive::OMPD_loop, |
There was a problem hiding this comment.
Is loop allowed?
There was a problem hiding this comment.
In the restrictions for loop directive. Page 257 OMP 5.2
If a reduction-modifier is specified in a reduction clause that appears on the directive then the reduction modifier must be default.
There was a problem hiding this comment.
Added a separate check for "loop".
Comment on lines
2415
to
2416
| llvm::omp::Directive::OMPD_single, | ||
| llvm::omp::Directive::OMPD_workshare, |
There was a problem hiding this comment.
Reduction clause is not allowed for these two, but I guess it is alright to have it here.
There was a problem hiding this comment.
Removed with a comment that they are not allowed with "reduction".
|
I couldn't add a testcase for "loop" because the parser doesn't recognize "loop" yet. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Emit diagnostic messages for invalid modifiers in "reduction" clause.
Fixes #92397