From 2aa218c247eef03f4ea922d635b7a9f46d061119 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 22 May 2024 07:23:14 -0500 Subject: [PATCH] [flang][OpenMP] Diagnose invalid reduction modifiers (#92406) 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 | 59 ++++++++++++ flang/lib/Semantics/check-omp-structure.h | 1 + .../OpenMP/invalid-reduction-modifier.f90 | 4 +- .../Semantics/OpenMP/reduction-modifiers.f90 | 89 +++++++++++++++++++ 4 files changed, 150 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 e9637b7bb591f3..5e3a5725c18d24 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -2310,6 +2310,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) { if (CheckReductionOperators(x)) { CheckReductionTypeList(x); } + CheckReductionModifier(x); } bool OmpStructureChecker::CheckReductionOperators( @@ -2394,6 +2395,64 @@ void OmpStructureChecker::CheckReductionTypeList( } } +void OmpStructureChecker::CheckReductionModifier( + const parser::OmpClause::Reduction &x) { + using ReductionModifier = parser::OmpReductionClause::ReductionModifier; + const auto &maybeModifier{std::get>(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 (dirCtx.directive == llvm::omp::Directive::OMPD_loop) { + // [5.2:257:33-34] + // If a reduction-modifier is specified in a reduction clause that + // appears on the directive, then the reduction modifier must be + // default. + context_.Say(GetContext().clauseSource, + "REDUCTION modifier on LOOP directive must be DEFAULT"_err_en_US); + } + 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_scope, + llvm::omp::Directive::OMPD_sections, + // There are more worksharing directives, but they do not apply: + // "for" is C++ only, + // "single" and "workshare" don't allow reduction clause, + // "loop" has different restrictions (checked above). + }; + 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 { + // Catch-all for potential future modifiers to make sure that this + // function is up-to-date. + 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 1f7284307703bf..47705771e8e28f 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 53871276761fa4..b3e87df7086eba 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 00000000000000..cf38200ba0a83e --- /dev/null +++ b/flang/test/Semantics/OpenMP/reduction-modifiers.f90 @@ -0,0 +1,89 @@ +! 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