From 28a107ce182b5426e50de6398a4bc5d0a4815dbe Mon Sep 17 00:00:00 2001 From: Mats Petersson Date: Tue, 11 Feb 2025 15:07:33 +0000 Subject: [PATCH 1/3] [flang][OpenMP]Improve support for DECLARE REDUCTION Part of the DECLARE REDUCTION was already supported by the parser, but the semantics to add the reduction identifier wasn't implemented. The semantics would not accept the name given by the reduction, so a few lines added to support that. Some tests were in place but not quite working, so fixed those up too. Adding new tests for unparsing and parse-tree, as well as checking the symbolic name being generated. Lowering of DECLARE REDUCTION is not supported in this patch, and a test that it hits the relevant TODO is in this patch (most of this was already existing, but not actually testing the TODO message). --- flang/include/flang/Parser/parse-tree.h | 4 ++-- flang/lib/Parser/openmp-parsers.cpp | 8 +++---- flang/lib/Parser/unparse.cpp | 5 ++--- flang/lib/Semantics/check-omp-structure.cpp | 4 ++++ flang/lib/Semantics/resolve-directives.cpp | 9 ++++++++ flang/lib/Semantics/resolve-names.cpp | 19 +++++++++++++++- .../OpenMP/Todo/omp-declare-reduction.f90 | 4 ++-- .../OpenMP/declare-reduction-unparse.f90 | 22 +++++++++++++++++++ .../OpenMP/declarative-directive01.f90 | 20 ++++++++--------- .../Semantics/OpenMP/declare-reduction.f90 | 11 ++++++++++ 10 files changed, 83 insertions(+), 23 deletions(-) create mode 100644 flang/test/Parser/OpenMP/declare-reduction-unparse.f90 create mode 100644 flang/test/Semantics/OpenMP/declare-reduction.f90 diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index c3a02fca5ade8..6ba43f6688c25 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -4553,8 +4553,8 @@ WRAPPER_CLASS(OmpReductionInitializerClause, Expr); struct OpenMPDeclareReductionConstruct { TUPLE_CLASS_BOILERPLATE(OpenMPDeclareReductionConstruct); CharBlock source; - std::tuple, - OmpReductionCombiner, std::optional> + std::tuple, + std::optional> t; }; diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 2b6c77c08cc58..b39b8737b70c0 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -170,8 +170,8 @@ TYPE_PARSER(sourced( // TYPE_PARSER(construct(nonemptyList(Parser{}))) TYPE_PARSER( // - construct(Parser{}) || - construct(Parser{})) + construct(Parser{}) || + construct(Parser{})) TYPE_PARSER(construct( // Parser{}, @@ -1148,9 +1148,7 @@ TYPE_PARSER(construct( // 2.16 Declare Reduction Construct TYPE_PARSER(sourced(construct( verbatim("DECLARE REDUCTION"_tok), - "(" >> Parser{} / ":", - nonemptyList(Parser{}) / ":", - Parser{} / ")", + "(" >> indirect(Parser{}) / ")", maybe(Parser{})))) // declare-target with list diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index cd91fbe4ea5eb..3d00979d7b7a6 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2690,11 +2690,10 @@ class UnparseVisitor { BeginOpenMP(); Word("!$OMP DECLARE REDUCTION "); Put("("); - Walk(std::get(x.t)), Put(" : "); - Walk(std::get>(x.t), ","), Put(" : "); - Walk(std::get(x.t)); + Walk(std::get>(x.t)); Put(")"); Walk(std::get>(x.t)); + Put("\n"); EndOpenMP(); } diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index fd2893998205c..9d7b60cdecbd0 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -3180,6 +3180,10 @@ bool OmpStructureChecker::CheckReductionOperator( const SourceName &realName{name->symbol->GetUltimate().name()}; valid = llvm::is_contained({"max", "min", "iand", "ior", "ieor"}, realName); + if (!valid) { + auto *misc{name->symbol->detailsIf()}; + valid = misc && misc->kind() == MiscDetails::Kind::ConstructName; + } } if (!valid) { context_.Say(source, diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 91a1b3061e1f9..94b653c152c5b 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -446,6 +446,9 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { bool Pre(const parser::OpenMPDeclareMapperConstruct &); void Post(const parser::OpenMPDeclareMapperConstruct &) { PopContext(); } + bool Pre(const parser::OpenMPDeclareReductionConstruct &); + void Post(const parser::OpenMPDeclareReductionConstruct &) { PopContext(); } + bool Pre(const parser::OpenMPThreadprivate &); void Post(const parser::OpenMPThreadprivate &) { PopContext(); } @@ -1976,6 +1979,12 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) { return true; } +bool OmpAttributeVisitor::Pre( + const parser::OpenMPDeclareReductionConstruct &x) { + PushContext(x.source, llvm::omp::Directive::OMPD_declare_reduction); + return true; +} + bool OmpAttributeVisitor::Pre(const parser::OpenMPThreadprivate &x) { PushContext(x.source, llvm::omp::Directive::OMPD_threadprivate); const auto &list{std::get(x.t)}; diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index e64abe6b50e78..ff793658f1e06 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1482,6 +1482,15 @@ class OmpVisitor : public virtual DeclarationVisitor { return false; } + bool Pre(const parser::OpenMPDeclareReductionConstruct &x) { + AddOmpSourceRange(x.source); + parser::OmpClauseList emptyList{std::list{}}; + ProcessReductionSpecifier( + std::get>(x.t).value(), + emptyList); + Walk(std::get>(x.t)); + return false; + } bool Pre(const parser::OmpMapClause &); void Post(const parser::OmpBeginLoopDirective &) { @@ -1732,11 +1741,19 @@ void OmpVisitor::ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec, void OmpVisitor::ProcessReductionSpecifier( const parser::OmpReductionSpecifier &spec, const parser::OmpClauseList &clauses) { + BeginDeclTypeSpec(); + const auto &id{std::get(spec.t)}; + if (auto procDes{std::get_if(&id.u)}) { + if (auto *name{std::get_if(&procDes->u)}) { + name->symbol = + &MakeSymbol(*name, MiscDetails{MiscDetails::Kind::ConstructName}); + } + } + EndDeclTypeSpec(); // Creating a new scope in case the combiner expression (or clauses) use // reerved identifiers, like "omp_in". This is a temporary solution until // we deal with these in a more thorough way. PushScope(Scope::Kind::OtherConstruct, nullptr); - Walk(std::get(spec.t)); Walk(std::get(spec.t)); Walk(std::get>(spec.t)); Walk(clauses); diff --git a/flang/test/Lower/OpenMP/Todo/omp-declare-reduction.f90 b/flang/test/Lower/OpenMP/Todo/omp-declare-reduction.f90 index 7a7d28db8d6f5..db50c9ac8ee9d 100644 --- a/flang/test/Lower/OpenMP/Todo/omp-declare-reduction.f90 +++ b/flang/test/Lower/OpenMP/Todo/omp-declare-reduction.f90 @@ -1,10 +1,10 @@ ! This test checks lowering of OpenMP declare reduction Directive. -// RUN: not flang -fc1 -emit-fir -fopenmp %s 2>&1 | FileCheck %s +! RUN: not flang -fc1 -emit-fir -fopenmp %s 2>&1 | FileCheck %s subroutine declare_red() integer :: my_var - // CHECK: not yet implemented: OpenMPDeclareReductionConstruct + !CHECK: not yet implemented: OpenMPDeclareReductionConstruct !$omp declare reduction (my_red : integer : omp_out = omp_in) initializer (omp_priv = 0) my_var = 0 end subroutine declare_red diff --git a/flang/test/Parser/OpenMP/declare-reduction-unparse.f90 b/flang/test/Parser/OpenMP/declare-reduction-unparse.f90 new file mode 100644 index 0000000000000..0d77b4f3cc030 --- /dev/null +++ b/flang/test/Parser/OpenMP/declare-reduction-unparse.f90 @@ -0,0 +1,22 @@ +! RUN: %flang_fc1 -fdebug-unparse -fopenmp %s | FileCheck --ignore-case %s +! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s +!CHECK-LABEL: program main +program main + use omp_lib + integer :: my_var + !CHECK: !$OMP DECLARE REDUCTION (my_add_red:INTEGER: omp_out=omp_out+omp_in + !CHECK-NEXT: ) INITIALIZER(OMP_PRIV = 0_4) + + !$omp declare reduction (my_add_red : integer : omp_out = omp_out + omp_in) initializer (omp_priv=0) + my_var = 0 + !$omp parallel reduction (my_add_red : my_var) num_threads(4) + my_var = omp_get_thread_num() + 1 + !$omp end parallel + print *, "sum of thread numbers is ", my_var +end program main + +!PARSE-TREE: OpenMPDeclareReductionConstruct +!PARSE-TREE: OmpReductionIdentifier -> ProcedureDesignator -> Name = 'my_add_red' +!PARSE-TREE: DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec +!PARSE-TREE: OmpReductionCombiner -> AssignmentStmt = 'omp_out=omp_out+omp_in' +!PARSE-TREE: OmpReductionInitializerClause -> Expr = '0_4' diff --git a/flang/test/Semantics/OpenMP/declarative-directive01.f90 b/flang/test/Semantics/OpenMP/declarative-directive01.f90 index 17dc50b70e542..85c1eddb788d1 100644 --- a/flang/test/Semantics/OpenMP/declarative-directive01.f90 +++ b/flang/test/Semantics/OpenMP/declarative-directive01.f90 @@ -88,15 +88,15 @@ end module m2 ! 2.16 declare-reduction -! subroutine declare_red_1() -! use omp_lib -! integer :: my_var -! !$omp declare reduction (my_add_red : integer : omp_out = omp_out + omp_in) initializer (omp_priv=0) -! my_var = 0 -! !$omp parallel reduction (my_add_red : my_var) num_threads(4) -! my_var = omp_get_thread_num() + 1 -! !$omp end parallel -! print *, "sum of thread numbers is ", my_var -! end subroutine declare_red_1 +subroutine declare_red_1() + use omp_lib + integer :: my_var + !$omp declare reduction (my_add_red : integer : omp_out = omp_out + omp_in) initializer (omp_priv=0) + my_var = 0 + !$omp parallel reduction (my_add_red : my_var) num_threads(4) + my_var = omp_get_thread_num() + 1 + !$omp end parallel + print *, "sum of thread numbers is ", my_var +end subroutine declare_red_1 end diff --git a/flang/test/Semantics/OpenMP/declare-reduction.f90 b/flang/test/Semantics/OpenMP/declare-reduction.f90 new file mode 100644 index 0000000000000..8fee79dfc0b7b --- /dev/null +++ b/flang/test/Semantics/OpenMP/declare-reduction.f90 @@ -0,0 +1,11 @@ +! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s + +program main +!CHECK-LABEL: MainProgram scope: main + + !$omp declare reduction (my_add_red : integer : omp_out = omp_out + omp_in) initializer (omp_priv=0) + +!CHECK: my_add_red: Misc ConstructName + +end program main + From bfccd95be8adcc1c0c041597c4f45afa0ed3df5f Mon Sep 17 00:00:00 2001 From: Mats Petersson Date: Tue, 18 Feb 2025 17:46:54 +0000 Subject: [PATCH 2/3] Fix broken tests --- flang/test/Parser/OpenMP/declare-reduction-unparse.f90 | 1 - flang/test/Semantics/OpenMP/declarative-directive01.f90 | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/flang/test/Parser/OpenMP/declare-reduction-unparse.f90 b/flang/test/Parser/OpenMP/declare-reduction-unparse.f90 index 0d77b4f3cc030..a2a3ef9f630ab 100644 --- a/flang/test/Parser/OpenMP/declare-reduction-unparse.f90 +++ b/flang/test/Parser/OpenMP/declare-reduction-unparse.f90 @@ -2,7 +2,6 @@ ! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s !CHECK-LABEL: program main program main - use omp_lib integer :: my_var !CHECK: !$OMP DECLARE REDUCTION (my_add_red:INTEGER: omp_out=omp_out+omp_in !CHECK-NEXT: ) INITIALIZER(OMP_PRIV = 0_4) diff --git a/flang/test/Semantics/OpenMP/declarative-directive01.f90 b/flang/test/Semantics/OpenMP/declarative-directive01.f90 index 85c1eddb788d1..bd148b87e1e4e 100644 --- a/flang/test/Semantics/OpenMP/declarative-directive01.f90 +++ b/flang/test/Semantics/OpenMP/declarative-directive01.f90 @@ -89,12 +89,11 @@ end module m2 ! 2.16 declare-reduction subroutine declare_red_1() - use omp_lib integer :: my_var !$omp declare reduction (my_add_red : integer : omp_out = omp_out + omp_in) initializer (omp_priv=0) my_var = 0 !$omp parallel reduction (my_add_red : my_var) num_threads(4) - my_var = omp_get_thread_num() + 1 + my_var = 1 !$omp end parallel print *, "sum of thread numbers is ", my_var end subroutine declare_red_1 From 9b089d82f6bd0600ae7047c7fba4a70847e36bdc Mon Sep 17 00:00:00 2001 From: Mats Petersson Date: Wed, 19 Feb 2025 10:38:29 +0000 Subject: [PATCH 3/3] Remove out of date comment --- flang/test/Semantics/OpenMP/declarative-directive01.f90 | 3 --- 1 file changed, 3 deletions(-) diff --git a/flang/test/Semantics/OpenMP/declarative-directive01.f90 b/flang/test/Semantics/OpenMP/declarative-directive01.f90 index bd148b87e1e4e..e8bf605565fad 100644 --- a/flang/test/Semantics/OpenMP/declarative-directive01.f90 +++ b/flang/test/Semantics/OpenMP/declarative-directive01.f90 @@ -2,9 +2,6 @@ ! Check OpenMP declarative directives -!TODO: all internal errors -! enable declare-reduction example after name resolution - ! 2.4 requires subroutine requires_1(a)