[flang-commits] [flang] [flang][OpenMP]Add symbls omp_in, omp_out and omp_priv in DECLARE RED… (PR #129908)
Mats Petersson via flang-commits
flang-commits at lists.llvm.org
Thu Mar 6 04:11:24 PST 2025
https://github.com/Leporacanthicus updated https://github.com/llvm/llvm-project/pull/129908
>From eaff8c50ea718b129da4a44dbe488e629abb739c Mon Sep 17 00:00:00 2001
From: Mats Petersson <mats.petersson at arm.com>
Date: Thu, 27 Feb 2025 15:01:44 +0000
Subject: [PATCH 1/3] [flang][OpenMP]Add symbls omp_in, omp_out and omp_priv in
DECLARE REDUCTION
This patch allows better parsing of the reduction and initializer
components, including supporting derived types in both those places.
There is more work needed here, but this is a definite improvement
in what can be handled through parser and semantics.
Note that declare reduction is still not supported in lowering,
so any attempt to compile DECLARE REDUCTION code will end with a
TODO aka "Not yet implemented" abort in the compiler.
Note that this version of the code does not cover declaring multiple
reductions using the same name with different types. This is
will be fixed in a future patch. [This was also the case before
this change].
One existing test modified to actually compile (as it didn't in
the original form).
---
flang/include/flang/Parser/dump-parse-tree.h | 1 -
flang/include/flang/Parser/parse-tree.h | 4 +--
flang/lib/Parser/openmp-parsers.cpp | 3 +-
flang/lib/Parser/unparse.cpp | 11 +++---
flang/lib/Semantics/resolve-names.cpp | 34 ++++++++++++++++---
.../OpenMP/declare-reduction-unparse.f90 | 4 +--
.../Parser/OpenMP/metadirective-dirspec.f90 | 29 +++++++++-------
7 files changed, 57 insertions(+), 29 deletions(-)
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 3b3c6bdc448d7..72c6ecf9a6cf5 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -639,7 +639,6 @@ class ParseTreeDumper {
NODE(parser, OmpTaskReductionClause)
NODE(OmpTaskReductionClause, Modifier)
NODE(parser, OmpInitializerProc)
- NODE(parser, OmpInitializerExpr)
NODE(parser, OmpInitializerClause)
NODE(parser, OmpReductionIdentifier)
NODE(parser, OmpAllocateClause)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index f11859bb09ddb..7727746535441 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4252,12 +4252,10 @@ struct OmpInitializerProc {
TUPLE_CLASS_BOILERPLATE(OmpInitializerProc);
std::tuple<ProcedureDesignator, std::list<ActualArgSpec>> t;
};
-WRAPPER_CLASS(OmpInitializerExpr, Expr);
-
// Initialization for declare reduction construct
struct OmpInitializerClause {
UNION_CLASS_BOILERPLATE(OmpInitializerClause);
- std::variant<OmpInitializerProc, OmpInitializerExpr> u;
+ std::variant<OmpInitializerProc, AssignmentStmt> u;
};
// Ref: [4.5:199-201], [5.0:288-290], [5.1:321-322], [5.2:115-117]
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 43cd2ea1eb0e6..bbce8b1c7b392 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1176,12 +1176,11 @@ TYPE_PARSER(construct<OmpBlockDirective>(first(
TYPE_PARSER(sourced(construct<OmpBeginBlockDirective>(
sourced(Parser<OmpBlockDirective>{}), Parser<OmpClauseList>{})))
-TYPE_PARSER(construct<OmpInitializerExpr>("OMP_PRIV =" >> expr))
TYPE_PARSER(construct<OmpInitializerProc>(Parser<ProcedureDesignator>{},
parenthesized(many(maybe(","_tok) >> Parser<ActualArgSpec>{}))))
TYPE_PARSER(construct<OmpInitializerClause>(
- construct<OmpInitializerClause>(Parser<OmpInitializerExpr>{}) ||
+ construct<OmpInitializerClause>(assignmentStmt) ||
construct<OmpInitializerClause>(Parser<OmpInitializerProc>{})))
// 2.16 Declare Reduction Construct
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 1df17b6d7382b..2c7d0ad92585c 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2705,11 +2705,14 @@ class UnparseVisitor {
Walk(std::get<std::list<ActualArgSpec>>(x.t));
Put(")");
}
- void Unparse(const OmpInitializerExpr &x) {
- Word("OMP_PRIV = ");
- Walk(x.v);
+ void Unparse(const OmpInitializerClause &x) {
+ // Don't let the visitor go to the normal AssignmentStmt Unparse function,
+ // it adds an extra newline that we don't want.
+ if (const auto *assignment = std::get_if<AssignmentStmt>(&x.u))
+ Walk(assignment->t, "=");
+ else
+ Walk(x.u);
}
- void Unparse(const OmpInitializerClause &x) { Walk(x.u); }
void Unparse(const OpenMPDeclareReductionConstruct &x) {
BeginOpenMP();
Word("!$OMP DECLARE REDUCTION ");
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 4f80cdca0f4bb..a0fe9ae446231 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1656,6 +1656,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
const parser::OmpClauseList &clauses);
void ProcessReductionSpecifier(const parser::OmpReductionSpecifier &spec,
const std::optional<parser::OmpClauseList> &clauses);
+ void CreateTempSymbol(const parser::CharBlock &name, const DeclTypeSpec &dts);
};
bool OmpVisitor::NeedsScope(const parser::OpenMPBlockConstruct &x) {
@@ -1748,6 +1749,14 @@ void OmpVisitor::ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec,
PopScope();
}
+void OmpVisitor::CreateTempSymbol(
+ const parser::CharBlock &name, const DeclTypeSpec &dts) {
+ ObjectEntityDetails details;
+ details.set_type(dts);
+
+ MakeSymbol(name, Attrs{}, std::move(details));
+}
+
void OmpVisitor::ProcessReductionSpecifier(
const parser::OmpReductionSpecifier &spec,
const std::optional<parser::OmpClauseList> &clauses) {
@@ -1761,11 +1770,26 @@ void OmpVisitor::ProcessReductionSpecifier(
// 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<parser::OmpTypeNameList>(spec.t));
- Walk(std::get<std::optional<parser::OmpReductionCombiner>>(spec.t));
- Walk(clauses);
- PopScope();
+ auto &typeList = std::get<parser::OmpTypeNameList>(spec.t);
+ for (auto &t : typeList.v) {
+ PushScope(Scope::Kind::OtherConstruct, nullptr);
+ BeginDeclTypeSpec();
+ // We need to walk t.u because Walk(t) does it's own BeginDeclTypeSpec.
+ Walk(t.u);
+
+ auto *typeSpec = GetDeclTypeSpec();
+ assert(typeSpec && "We should have a type here");
+ const parser::CharBlock ompVarNames[] = {
+ {"omp_in", 6}, {"omp_out", 7}, {"omp_priv", 8}};
+
+ for (auto &nm : ompVarNames) {
+ CreateTempSymbol(nm, *typeSpec);
+ }
+ EndDeclTypeSpec();
+ Walk(std::get<std::optional<parser::OmpReductionCombiner>>(spec.t));
+ Walk(clauses);
+ PopScope();
+ }
}
bool OmpVisitor::Pre(const parser::OmpDirectiveSpecification &x) {
diff --git a/flang/test/Parser/OpenMP/declare-reduction-unparse.f90 b/flang/test/Parser/OpenMP/declare-reduction-unparse.f90
index f0ab99f5cfb5f..7b1b569d87f78 100644
--- a/flang/test/Parser/OpenMP/declare-reduction-unparse.f90
+++ b/flang/test/Parser/OpenMP/declare-reduction-unparse.f90
@@ -44,7 +44,7 @@ end function func
program main
integer :: my_var
!CHECK: !$OMP DECLARE REDUCTION (my_add_red:INTEGER: omp_out=omp_out+omp_in
-!CHECK-NEXT: ) INITIALIZER(OMP_PRIV = 0_4)
+!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
@@ -58,4 +58,4 @@ end program main
!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: OmpInitializerClause -> OmpInitializerExpr -> Expr = '0_4'
+!PARSE-TREE: OmpInitializerClause -> AssignmentStmt = 'omp_priv=0_4'
diff --git a/flang/test/Parser/OpenMP/metadirective-dirspec.f90 b/flang/test/Parser/OpenMP/metadirective-dirspec.f90
index 73520c41fe77d..872cf5d46d34a 100644
--- a/flang/test/Parser/OpenMP/metadirective-dirspec.f90
+++ b/flang/test/Parser/OpenMP/metadirective-dirspec.f90
@@ -92,10 +92,10 @@ subroutine f03
integer :: x
endtype
type :: tt2
- real :: a
+ real :: x
endtype
!$omp metadirective when(user={condition(.true.)}: &
- !$omp & declare reduction(+ : tt1, tt2 : omp_out = omp_in + omp_out))
+ !$omp & declare reduction(+ : tt1, tt2 : omp_out%x = omp_in%x + omp_out%x))
end
!UNPARSE: SUBROUTINE f03
@@ -103,9 +103,9 @@ subroutine f03
!UNPARSE: INTEGER :: x
!UNPARSE: END TYPE
!UNPARSE: TYPE :: tt2
-!UNPARSE: REAL :: a
+!UNPARSE: REAL :: x
!UNPARSE: END TYPE
-!UNPARSE: !$OMP METADIRECTIVE WHEN(USER={CONDITION(.true._4)}: DECLARE REDUCTION(+:tt1,tt2: omp_out=omp_in+omp_out
+!UNPARSE: !$OMP METADIRECTIVE WHEN(USER={CONDITION(.true._4)}: DECLARE REDUCTION(+:tt1,tt2: omp_out%x=omp_in%x+omp_out%x
!UNPARSE: ))
!UNPARSE: END SUBROUTINE
@@ -127,15 +127,20 @@ subroutine f03
!PARSE-TREE: | | | | | Name = 'tt1'
!PARSE-TREE: | | | | OmpTypeSpecifier -> TypeSpec -> DerivedTypeSpec
!PARSE-TREE: | | | | | Name = 'tt2'
-!PARSE-TREE: | | | | OmpReductionCombiner -> AssignmentStmt = 'omp_out=omp_in+omp_out'
-!PARSE-TREE: | | | | | Variable = 'omp_out'
-!PARSE-TREE: | | | | | | Designator -> DataRef -> Name = 'omp_out'
-!PARSE-TREE: | | | | | Expr = 'omp_in+omp_out'
+!PARSE-TREE: | | | | OmpReductionCombiner -> AssignmentStmt = 'omp_out%x=omp_in%x+omp_out%x'
+!PARSE-TREE: | | | | | | Designator -> DataRef -> StructureComponent
+!PARSE-TREE: | | | | | | | DataRef -> Name = 'omp_out'
+!PARSE-TREE: | | | | | | | Name = 'x'
+!PARSE-TREE: | | | | | Expr = 'omp_in%x+omp_out%x'
!PARSE-TREE: | | | | | | Add
-!PARSE-TREE: | | | | | | | Expr = 'omp_in'
-!PARSE-TREE: | | | | | | | | Designator -> DataRef -> Name = 'omp_in'
-!PARSE-TREE: | | | | | | | Expr = 'omp_out'
-!PARSE-TREE: | | | | | | | | Designator -> DataRef -> Name = 'omp_out'
+!PARSE-TREE: | | | | | | | Expr = 'omp_in%x'
+!PARSE-TREE: | | | | | | | | Designator -> DataRef -> StructureComponent
+!PARSE-TREE: | | | | | | | | | DataRef -> Name = 'omp_in'
+!PARSE-TREE: | | | | | | | | | Name = 'x'
+!PARSE-TREE: | | | | | | | Expr = 'omp_out%x'
+!PARSE-TREE: | | | | | | | | Designator -> DataRef -> StructureComponent
+!PARSE-TREE: | | | | | | | | | DataRef -> Name = 'omp_out'
+!PARSE-TREE: | | | | | | | | | Name = 'x'
!PARSE-TREE: | | | OmpClauseList ->
subroutine f04
>From df74037c79a85d49ebb83dd7b5c517bf63811db9 Mon Sep 17 00:00:00 2001
From: Mats Petersson <mats.petersson at arm.com>
Date: Thu, 6 Mar 2025 10:50:41 +0000
Subject: [PATCH 2/3] Review comment fixes
---
flang/lib/Parser/unparse.cpp | 5 +++--
flang/lib/Semantics/resolve-names.cpp | 8 ++++----
flang/test/Semantics/OpenMP/declare-reduction.f90 | 9 +++++++--
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 2c7d0ad92585c..667da7533d3a8 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2708,10 +2708,11 @@ class UnparseVisitor {
void Unparse(const OmpInitializerClause &x) {
// Don't let the visitor go to the normal AssignmentStmt Unparse function,
// it adds an extra newline that we don't want.
- if (const auto *assignment = std::get_if<AssignmentStmt>(&x.u))
+ if (const auto *assignment{std::get_if<AssignmentStmt>(&x.u)}) {
Walk(assignment->t, "=");
- else
+ } else {
Walk(x.u);
+ }
}
void Unparse(const OpenMPDeclareReductionConstruct &x) {
BeginOpenMP();
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index a0fe9ae446231..e4a380348ae25 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1751,7 +1751,7 @@ void OmpVisitor::ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec,
void OmpVisitor::CreateTempSymbol(
const parser::CharBlock &name, const DeclTypeSpec &dts) {
- ObjectEntityDetails details;
+ ObjectEntityDetails details{};
details.set_type(dts);
MakeSymbol(name, Attrs{}, std::move(details));
@@ -1770,16 +1770,16 @@ void OmpVisitor::ProcessReductionSpecifier(
// 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.
- auto &typeList = std::get<parser::OmpTypeNameList>(spec.t);
+ auto &typeList{std::get<parser::OmpTypeNameList>(spec.t)};
for (auto &t : typeList.v) {
PushScope(Scope::Kind::OtherConstruct, nullptr);
BeginDeclTypeSpec();
// We need to walk t.u because Walk(t) does it's own BeginDeclTypeSpec.
Walk(t.u);
- auto *typeSpec = GetDeclTypeSpec();
+ const DeclTypeSpec *typeSpec{GetDeclTypeSpec()};
assert(typeSpec && "We should have a type here");
- const parser::CharBlock ompVarNames[] = {
+ const parser::CharBlock ompVarNames[]{
{"omp_in", 6}, {"omp_out", 7}, {"omp_priv", 8}};
for (auto &nm : ompVarNames) {
diff --git a/flang/test/Semantics/OpenMP/declare-reduction.f90 b/flang/test/Semantics/OpenMP/declare-reduction.f90
index e61af0430575f..5bb2132b8eaa2 100644
--- a/flang/test/Semantics/OpenMP/declare-reduction.f90
+++ b/flang/test/Semantics/OpenMP/declare-reduction.f90
@@ -18,7 +18,10 @@ end subroutine initme
end interface
!$omp declare reduction(red_add:integer(4):omp_out=omp_out+omp_in) initializer(initme(omp_priv,0))
!CHECK: red_add: Misc ConstructName
-!CHECK: Subprogram scope: initme
+!CHECK: Subprogram scope: initme
+!CHECK: omp_in size=4 offset=0: ObjectEntity type: INTEGER(4)
+!CHECK: omp_out size=4 offset=4: ObjectEntity type: INTEGER(4)
+!CHECK: omp_priv size=4 offset=8: ObjectEntity type: INTEGER(4)
!$omp simd reduction(red_add:res)
do i=1,n
res=res+x(i)
@@ -32,6 +35,8 @@ program main
!$omp declare reduction (my_add_red : integer : omp_out = omp_out + omp_in) initializer (omp_priv=0)
!CHECK: my_add_red: Misc ConstructName
+!CHECK: omp_in size=4 offset=0: ObjectEntity type: INTEGER(4)
+!CHECK: omp_out size=4 offset=4: ObjectEntity type: INTEGER(4)
+!CHECK: omp_priv size=4 offset=8: ObjectEntity type: INTEGER(4)
end program main
-
>From 2708306c87bc805b5b5dfbdf8a965dde7e27bd18 Mon Sep 17 00:00:00 2001
From: Mats Petersson <mats.petersson at arm.com>
Date: Thu, 6 Mar 2025 12:09:59 +0000
Subject: [PATCH 3/3] Move function inline
---
flang/lib/Semantics/resolve-names.cpp | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index e4a380348ae25..3e182c9289947 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1656,7 +1656,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
const parser::OmpClauseList &clauses);
void ProcessReductionSpecifier(const parser::OmpReductionSpecifier &spec,
const std::optional<parser::OmpClauseList> &clauses);
- void CreateTempSymbol(const parser::CharBlock &name, const DeclTypeSpec &dts);
};
bool OmpVisitor::NeedsScope(const parser::OpenMPBlockConstruct &x) {
@@ -1749,14 +1748,6 @@ void OmpVisitor::ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec,
PopScope();
}
-void OmpVisitor::CreateTempSymbol(
- const parser::CharBlock &name, const DeclTypeSpec &dts) {
- ObjectEntityDetails details{};
- details.set_type(dts);
-
- MakeSymbol(name, Attrs{}, std::move(details));
-}
-
void OmpVisitor::ProcessReductionSpecifier(
const parser::OmpReductionSpecifier &spec,
const std::optional<parser::OmpClauseList> &clauses) {
@@ -1783,7 +1774,9 @@ void OmpVisitor::ProcessReductionSpecifier(
{"omp_in", 6}, {"omp_out", 7}, {"omp_priv", 8}};
for (auto &nm : ompVarNames) {
- CreateTempSymbol(nm, *typeSpec);
+ ObjectEntityDetails details{};
+ details.set_type(*typeSpec);
+ MakeSymbol(nm, Attrs{}, std::move(details));
}
EndDeclTypeSpec();
Walk(std::get<std::optional<parser::OmpReductionCombiner>>(spec.t));
More information about the flang-commits
mailing list