[flang][OpenMP]Add parsing support for MAP(MAPPER(name) ...) #116274
Merged
Conversation
This file contains hidden or 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
This prepares for using the DECLARE MAPPER construct. A check in lowering will say "Not implemented" when trying to use a mapper as some code is required to tie the mapper to the declared one. Senantics check for the symbol generated.
llvmbot
added
flang
Member
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-fir-hlfir Author: Mats Petersson (Leporacanthicus) ChangesThis prepares for using the DECLARE MAPPER construct. A check in lowering will say "Not implemented" when trying to use a mapper as some code is required to tie the mapper to the declared one. Senantics check for the symbol generated. Full diff: https://github.com/llvm/llvm-project/pull/116274.diff 9 Files Affected:
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 5886e384b986b6..520b7ceeb14496 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -546,6 +546,7 @@ class ParseTreeDumper {
NODE_ENUM(OmpLinearModifier, Type)
NODE(parser, OmpLoopDirective)
NODE(parser, OmpMapClause)
+ NODE(parser, OmpMapperIdentifier)
NODE_ENUM(OmpMapClause, TypeModifier)
NODE_ENUM(OmpMapClause, Type)
static std::string GetNodeName(const llvm::omp::Clause &x) {
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 10d7840775e88d..c7edaa21419693 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3703,8 +3703,11 @@ struct OmpLinearClause {
std::variant<WithModifier, WithoutModifier> u;
};
+WRAPPER_CLASS(OmpMapperIdentifier, std::optional<Name>);
+
// 2.15.5.1 map ->
-// MAP ([[map-type-modifier-list [,]] [iterator-modifier [,]] map-type : ]
+// MAP ([MAPPER(mapper-identifier)] [[map-type-modifier-list [,]]
+// [iterator-modifier [,]] map-type : ]
// variable-name-list)
// map-type-modifier-list -> map-type-modifier [,] [...]
// map-type-modifier -> ALWAYS | CLOSE | PRESENT | OMPX_HOLD
@@ -3718,7 +3721,8 @@ struct OmpMapClause {
// The checks for satisfying those constraints are deferred to semantics.
// In OpenMP 5.2 the non-comma syntax has been deprecated: keep the
// information about separator presence to emit a diagnostic if needed.
- std::tuple<std::optional<std::list<TypeModifier>>,
+ std::tuple<OmpMapperIdentifier, // Mapper name
+ std::optional<std::list<TypeModifier>>,
std::optional<std::list<OmpIteratorModifier>>, // unique
std::optional<std::list<Type>>, // unique
OmpObjectList,
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 3dedd4864bafc5..88aa62a62352c4 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -10,6 +10,7 @@
#include "flang/Common/idioms.h"
#include "flang/Evaluate/expression.h"
+#include "flang/Optimizer/Builder/Todo.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
#include "flang/Semantics/symbol.h"
@@ -974,6 +975,10 @@ Map make(const parser::OmpClause::Map &inp,
std::get<std::optional<std::list<parser::OmpIteratorModifier>>>(inp.v.t);
auto &t2 = std::get<std::optional<std::list<wrapped::Type>>>(inp.v.t);
auto &t3 = std::get<parser::OmpObjectList>(inp.v.t);
+ auto &t4 = std::get<parser::OmpMapperIdentifier>(inp.v.t);
+
+ if (t4.v)
+ TODO_NOLOC("OmpMapClause(MAPPER(...))");
// These should have been diagnosed already.
assert((!t1 || t1->size() == 1) && "Only one iterator modifier is allowed");
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index c2c730edacc02a..8259f501dca845 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -131,7 +131,6 @@ template <typename Separator> struct MotionModifiers {
constexpr MotionModifiers(const MotionModifiers &) = default;
constexpr MotionModifiers(MotionModifiers &&) = default;
- // Parsing of mappers if not implemented yet.
using ExpParser = Parser<OmpFromClause::Expectation>;
using IterParser = Parser<OmpIteratorModifier>;
using ModParser = ConcatSeparated<Separator, ExpParser, IterParser>;
@@ -250,21 +249,26 @@ TYPE_PARSER(
"TOFROM" >> pure(OmpMapClause::Type::Tofrom)))
template <bool CommasEverywhere>
-static inline OmpMapClause makeMapClause(
+static inline OmpMapClause makeMapClause(OmpMapperIdentifier &&mm,
std::tuple<std::optional<std::list<OmpMapClause::TypeModifier>>,
std::optional<std::list<OmpIteratorModifier>>,
std::optional<std::list<OmpMapClause::Type>>> &&mods,
OmpObjectList &&objs) {
auto &&[tm, it, ty] = std::move(mods);
- return OmpMapClause{std::move(tm), std::move(it), std::move(ty),
- std::move(objs), CommasEverywhere};
+ return OmpMapClause{std::move(mm), std::move(tm), std::move(it),
+ std::move(ty), std::move(objs), CommasEverywhere};
}
+TYPE_PARSER(construct<OmpMapperIdentifier>(
+ maybe("MAPPER"_tok >> parenthesized(name) / ","_tok)))
+
TYPE_PARSER(construct<OmpMapClause>(
- applyFunction<OmpMapClause>(
- makeMapClause<true>, MapModifiers(","_tok), Parser<OmpObjectList>{}) ||
+ applyFunction<OmpMapClause>(makeMapClause<true>,
+ Parser<OmpMapperIdentifier>{}, MapModifiers(","_tok),
+ Parser<OmpObjectList>{}) ||
applyFunction<OmpMapClause>(makeMapClause<false>,
- MapModifiers(maybe(","_tok)), Parser<OmpObjectList>{})))
+ Parser<OmpMapperIdentifier>{}, MapModifiers(maybe(","_tok)),
+ Parser<OmpObjectList>{})))
// [OpenMP 5.0]
// 2.19.7.2 defaultmap(implicit-behavior[:variable-category])
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 158d3a1f14e4fe..a4de6d23f9f199 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2095,12 +2095,22 @@ class UnparseVisitor {
std::get<std::optional<std::list<OmpMapClause::TypeModifier>>>(x.t);
auto &iter = std::get<std::optional<std::list<OmpIteratorModifier>>>(x.t);
auto &type = std::get<std::optional<std::list<OmpMapClause::Type>>>(x.t);
+ auto &mapper = std::get<OmpMapperIdentifier>(x.t);
// For a given list of items, if the item has a value, then walk it.
// Print commas between items that have values.
// Return 'true' if something did get printed, otherwise 'false'.
bool needComma{false};
+ if (mapper.v) {
+ Word("MAPPER(");
+ Walk(*mapper.v);
+ Put(")");
+ needComma = true;
+ }
if (typeMod) {
+ if (needComma) {
+ Put(", ");
+ }
Walk(*typeMod);
needComma = true;
}
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 9b0e204ac4e918..9ebb90587e9e79 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1471,6 +1471,8 @@ class OmpVisitor : public virtual DeclarationVisitor {
bool Pre(const parser::OpenMPDeclareMapperConstruct &);
+ bool Pre(const parser::OmpMapClause &);
+
void Post(const parser::OmpBeginLoopDirective &) {
messageHandler().set_currStmtSource(std::nullopt);
}
@@ -1639,6 +1641,14 @@ bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) {
return false;
}
+bool OmpVisitor::Pre(const parser::OmpMapClause &x) {
+ const auto &mid{std::get<parser::OmpMapperIdentifier>(x.t)};
+ if (const auto &mapperName{mid.v})
+ mapperName->symbol =
+ &MakeSymbol(*mapperName, MiscDetails{MiscDetails::Kind::ConstructName});
+ return true;
+}
+
// Walk the parse tree and resolve names to symbols.
class ResolveNamesVisitor : public virtual ScopeHandler,
public ModuleVisitor,
diff --git a/flang/test/Lower/OpenMP/Todo/map-mapper.f90 b/flang/test/Lower/OpenMP/Todo/map-mapper.f90
new file mode 100644
index 00000000000000..30831337beff2b
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/map-mapper.f90
@@ -0,0 +1,11 @@
+! RUN: not %flang_fc1 -emit-fir -fopenmp -fopenmp-version=50 %s 2>&1 | FileCheck %s
+program p
+ integer, parameter :: n = 256
+ real(8) :: a(256)
+ !$omp target map(mapper(xx), from:a)
+!CHECK: not yet implemented: OmpMapClause(MAPPER(...))
+ do i=1,n
+ a(i) = 4.2
+ end do
+ !$omp end target
+end program p
diff --git a/flang/test/Parser/OpenMP/map-modifiers.f90 b/flang/test/Parser/OpenMP/map-modifiers.f90
index 0c95f21c5e6a53..2b4073f2cafcd7 100644
--- a/flang/test/Parser/OpenMP/map-modifiers.f90
+++ b/flang/test/Parser/OpenMP/map-modifiers.f90
@@ -316,3 +316,28 @@ subroutine f90(x, y)
!PARSE-TREE: | | | | Designator -> DataRef -> Name = 'k'
!PARSE-TREE: | | bool = 'true'
+subroutine f100(x, y)
+ integer :: x(10)
+ integer :: y
+ integer, parameter :: p = 23
+ !$omp target map(mapper(xx), from: x)
+ x = x + 1
+ !$omp end target
+end
+
+!UNPARSE: SUBROUTINE f100 (x, y)
+!UNPARSE: INTEGER x(10_4)
+!UNPARSE: INTEGER y
+!UNPARSE: INTEGER, PARAMETER :: p = 23_4
+!UNPARSE: !$OMP TARGET MAP(MAPPER(XX), FROM: X)
+!UNPARSE: x=x+1_4
+!UNPARSE: !$OMP END TARGET
+!UNPARSE: END SUBROUTINE
+
+!PARSE-TREE: OmpBeginBlockDirective
+!PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
+!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
+!PARSE-TREE: | | OmpMapperIdentifier -> Name = 'xx'
+!PARSE-TREE: | | Type = From
+!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
+
diff --git a/flang/test/Semantics/OpenMP/map-clause-symbols.f90 b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
new file mode 100644
index 00000000000000..8f984fcd2fa7e2
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s
+program main
+!CHECK-LABEL: MainProgram scope: main
+ integer, parameter :: n = 256
+ real(8) :: a(256)
+ !$omp target map(mapper(xx), from:a)
+ do i=1,n
+ a(i) = 4.2
+ end do
+ !$omp end target
+!CHECK: OtherConstruct scope: size=0 alignment=1 sourceRange=74 bytes
+!CHECK: OtherClause scope: size=0 alignment=1 sourceRange=0 bytes
+!CHECK: xx: Misc ConstructName
+end program main
|
Member
|
@llvm/pr-subscribers-flang-parser Author: Mats Petersson (Leporacanthicus) ChangesThis prepares for using the DECLARE MAPPER construct. A check in lowering will say "Not implemented" when trying to use a mapper as some code is required to tie the mapper to the declared one. Senantics check for the symbol generated. Full diff: https://github.com/llvm/llvm-project/pull/116274.diff 9 Files Affected:
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 5886e384b986b6..520b7ceeb14496 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -546,6 +546,7 @@ class ParseTreeDumper {
NODE_ENUM(OmpLinearModifier, Type)
NODE(parser, OmpLoopDirective)
NODE(parser, OmpMapClause)
+ NODE(parser, OmpMapperIdentifier)
NODE_ENUM(OmpMapClause, TypeModifier)
NODE_ENUM(OmpMapClause, Type)
static std::string GetNodeName(const llvm::omp::Clause &x) {
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 10d7840775e88d..c7edaa21419693 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3703,8 +3703,11 @@ struct OmpLinearClause {
std::variant<WithModifier, WithoutModifier> u;
};
+WRAPPER_CLASS(OmpMapperIdentifier, std::optional<Name>);
+
// 2.15.5.1 map ->
-// MAP ([[map-type-modifier-list [,]] [iterator-modifier [,]] map-type : ]
+// MAP ([MAPPER(mapper-identifier)] [[map-type-modifier-list [,]]
+// [iterator-modifier [,]] map-type : ]
// variable-name-list)
// map-type-modifier-list -> map-type-modifier [,] [...]
// map-type-modifier -> ALWAYS | CLOSE | PRESENT | OMPX_HOLD
@@ -3718,7 +3721,8 @@ struct OmpMapClause {
// The checks for satisfying those constraints are deferred to semantics.
// In OpenMP 5.2 the non-comma syntax has been deprecated: keep the
// information about separator presence to emit a diagnostic if needed.
- std::tuple<std::optional<std::list<TypeModifier>>,
+ std::tuple<OmpMapperIdentifier, // Mapper name
+ std::optional<std::list<TypeModifier>>,
std::optional<std::list<OmpIteratorModifier>>, // unique
std::optional<std::list<Type>>, // unique
OmpObjectList,
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 3dedd4864bafc5..88aa62a62352c4 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -10,6 +10,7 @@
#include "flang/Common/idioms.h"
#include "flang/Evaluate/expression.h"
+#include "flang/Optimizer/Builder/Todo.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
#include "flang/Semantics/symbol.h"
@@ -974,6 +975,10 @@ Map make(const parser::OmpClause::Map &inp,
std::get<std::optional<std::list<parser::OmpIteratorModifier>>>(inp.v.t);
auto &t2 = std::get<std::optional<std::list<wrapped::Type>>>(inp.v.t);
auto &t3 = std::get<parser::OmpObjectList>(inp.v.t);
+ auto &t4 = std::get<parser::OmpMapperIdentifier>(inp.v.t);
+
+ if (t4.v)
+ TODO_NOLOC("OmpMapClause(MAPPER(...))");
// These should have been diagnosed already.
assert((!t1 || t1->size() == 1) && "Only one iterator modifier is allowed");
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index c2c730edacc02a..8259f501dca845 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -131,7 +131,6 @@ template <typename Separator> struct MotionModifiers {
constexpr MotionModifiers(const MotionModifiers &) = default;
constexpr MotionModifiers(MotionModifiers &&) = default;
- // Parsing of mappers if not implemented yet.
using ExpParser = Parser<OmpFromClause::Expectation>;
using IterParser = Parser<OmpIteratorModifier>;
using ModParser = ConcatSeparated<Separator, ExpParser, IterParser>;
@@ -250,21 +249,26 @@ TYPE_PARSER(
"TOFROM" >> pure(OmpMapClause::Type::Tofrom)))
template <bool CommasEverywhere>
-static inline OmpMapClause makeMapClause(
+static inline OmpMapClause makeMapClause(OmpMapperIdentifier &&mm,
std::tuple<std::optional<std::list<OmpMapClause::TypeModifier>>,
std::optional<std::list<OmpIteratorModifier>>,
std::optional<std::list<OmpMapClause::Type>>> &&mods,
OmpObjectList &&objs) {
auto &&[tm, it, ty] = std::move(mods);
- return OmpMapClause{std::move(tm), std::move(it), std::move(ty),
- std::move(objs), CommasEverywhere};
+ return OmpMapClause{std::move(mm), std::move(tm), std::move(it),
+ std::move(ty), std::move(objs), CommasEverywhere};
}
+TYPE_PARSER(construct<OmpMapperIdentifier>(
+ maybe("MAPPER"_tok >> parenthesized(name) / ","_tok)))
+
TYPE_PARSER(construct<OmpMapClause>(
- applyFunction<OmpMapClause>(
- makeMapClause<true>, MapModifiers(","_tok), Parser<OmpObjectList>{}) ||
+ applyFunction<OmpMapClause>(makeMapClause<true>,
+ Parser<OmpMapperIdentifier>{}, MapModifiers(","_tok),
+ Parser<OmpObjectList>{}) ||
applyFunction<OmpMapClause>(makeMapClause<false>,
- MapModifiers(maybe(","_tok)), Parser<OmpObjectList>{})))
+ Parser<OmpMapperIdentifier>{}, MapModifiers(maybe(","_tok)),
+ Parser<OmpObjectList>{})))
// [OpenMP 5.0]
// 2.19.7.2 defaultmap(implicit-behavior[:variable-category])
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 158d3a1f14e4fe..a4de6d23f9f199 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2095,12 +2095,22 @@ class UnparseVisitor {
std::get<std::optional<std::list<OmpMapClause::TypeModifier>>>(x.t);
auto &iter = std::get<std::optional<std::list<OmpIteratorModifier>>>(x.t);
auto &type = std::get<std::optional<std::list<OmpMapClause::Type>>>(x.t);
+ auto &mapper = std::get<OmpMapperIdentifier>(x.t);
// For a given list of items, if the item has a value, then walk it.
// Print commas between items that have values.
// Return 'true' if something did get printed, otherwise 'false'.
bool needComma{false};
+ if (mapper.v) {
+ Word("MAPPER(");
+ Walk(*mapper.v);
+ Put(")");
+ needComma = true;
+ }
if (typeMod) {
+ if (needComma) {
+ Put(", ");
+ }
Walk(*typeMod);
needComma = true;
}
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 9b0e204ac4e918..9ebb90587e9e79 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1471,6 +1471,8 @@ class OmpVisitor : public virtual DeclarationVisitor {
bool Pre(const parser::OpenMPDeclareMapperConstruct &);
+ bool Pre(const parser::OmpMapClause &);
+
void Post(const parser::OmpBeginLoopDirective &) {
messageHandler().set_currStmtSource(std::nullopt);
}
@@ -1639,6 +1641,14 @@ bool OmpVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) {
return false;
}
+bool OmpVisitor::Pre(const parser::OmpMapClause &x) {
+ const auto &mid{std::get<parser::OmpMapperIdentifier>(x.t)};
+ if (const auto &mapperName{mid.v})
+ mapperName->symbol =
+ &MakeSymbol(*mapperName, MiscDetails{MiscDetails::Kind::ConstructName});
+ return true;
+}
+
// Walk the parse tree and resolve names to symbols.
class ResolveNamesVisitor : public virtual ScopeHandler,
public ModuleVisitor,
diff --git a/flang/test/Lower/OpenMP/Todo/map-mapper.f90 b/flang/test/Lower/OpenMP/Todo/map-mapper.f90
new file mode 100644
index 00000000000000..30831337beff2b
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/map-mapper.f90
@@ -0,0 +1,11 @@
+! RUN: not %flang_fc1 -emit-fir -fopenmp -fopenmp-version=50 %s 2>&1 | FileCheck %s
+program p
+ integer, parameter :: n = 256
+ real(8) :: a(256)
+ !$omp target map(mapper(xx), from:a)
+!CHECK: not yet implemented: OmpMapClause(MAPPER(...))
+ do i=1,n
+ a(i) = 4.2
+ end do
+ !$omp end target
+end program p
diff --git a/flang/test/Parser/OpenMP/map-modifiers.f90 b/flang/test/Parser/OpenMP/map-modifiers.f90
index 0c95f21c5e6a53..2b4073f2cafcd7 100644
--- a/flang/test/Parser/OpenMP/map-modifiers.f90
+++ b/flang/test/Parser/OpenMP/map-modifiers.f90
@@ -316,3 +316,28 @@ subroutine f90(x, y)
!PARSE-TREE: | | | | Designator -> DataRef -> Name = 'k'
!PARSE-TREE: | | bool = 'true'
+subroutine f100(x, y)
+ integer :: x(10)
+ integer :: y
+ integer, parameter :: p = 23
+ !$omp target map(mapper(xx), from: x)
+ x = x + 1
+ !$omp end target
+end
+
+!UNPARSE: SUBROUTINE f100 (x, y)
+!UNPARSE: INTEGER x(10_4)
+!UNPARSE: INTEGER y
+!UNPARSE: INTEGER, PARAMETER :: p = 23_4
+!UNPARSE: !$OMP TARGET MAP(MAPPER(XX), FROM: X)
+!UNPARSE: x=x+1_4
+!UNPARSE: !$OMP END TARGET
+!UNPARSE: END SUBROUTINE
+
+!PARSE-TREE: OmpBeginBlockDirective
+!PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
+!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
+!PARSE-TREE: | | OmpMapperIdentifier -> Name = 'xx'
+!PARSE-TREE: | | Type = From
+!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
+
diff --git a/flang/test/Semantics/OpenMP/map-clause-symbols.f90 b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
new file mode 100644
index 00000000000000..8f984fcd2fa7e2
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s
+program main
+!CHECK-LABEL: MainProgram scope: main
+ integer, parameter :: n = 256
+ real(8) :: a(256)
+ !$omp target map(mapper(xx), from:a)
+ do i=1,n
+ a(i) = 4.2
+ end do
+ !$omp end target
+!CHECK: OtherConstruct scope: size=0 alignment=1 sourceRange=74 bytes
+!CHECK: OtherClause scope: size=0 alignment=1 sourceRange=0 bytes
+!CHECK: xx: Misc ConstructName
+end program main
|
tblah
reviewed
Nov 15, 2024
flang/lib/Lower/OpenMP/Clauses.cpp
Outdated
| auto &t4 = std::get<parser::OmpMapperIdentifier>(inp.v.t); | ||
|
|
||
| if (t4.v) | ||
| TODO_NOLOC("OmpMapClause(MAPPER(...))"); |
Contributor
There was a problem hiding this comment.
Please could you write something easier for users to parse. Maybe something like "support for user-defined mappers".
Contributor
Author
There was a problem hiding this comment.
Fair point! I've updated the message to contain both a user-friendly explanation and the developer-relevant clause.
| // In OpenMP 5.2 the non-comma syntax has been deprecated: keep the | ||
| // information about separator presence to emit a diagnostic if needed. | ||
| std::tuple<std::optional<std::list<TypeModifier>>, | ||
| std::tuple<OmpMapperIdentifier, // Mapper name |
Contributor
There was a problem hiding this comment.
It reads a bit strangely to me that the optional mapper identifier looks non-optional (if one doesn't look up at the type definition). Could the std::optional be moved outside of the type so it is visible here?
Don't worry about it if it becomes a headache to do - this is very nitpicky
Contributor
Author
There was a problem hiding this comment.
So, this was my solution to making the maybe work. Since we need to parse the "MAPPER" and then parenthesized(name) and a comma. If I move the optional around, the maybe parsing doesn't work.
Unless someone can let me know how to make something maybe parse to produce the wrapper thing optional.
Everything I've tried (and that's a lot - I couldn't list them all, but std::optional<Name> in the std::tuple was one of them.
The TypeModifier and IteratorModifer and Type gets away with it, because they don't have a keyword with a parenthesized "argument". so they can just do a list (which is allowed to be empty).
I did try again just now, but I can't come up with something that compiles...
Contributor
There was a problem hiding this comment.
The way this was intended to work is that
- You add
std::optional<std::list<YourModifier>>to the tuple in here. - Add the mapper parser to the
ModParserlist in MapModifiers in openmp-parsers.cpp. - Unpack it in
makeMapClauseand pass it to theOmpMapClauseconstructor.
Then in check-omp-structure.cpp you can verify that there is at most one mapper modifier in the list, and print a meaningful message.
Having said that, I'm working on modifier verification right now, which will change how they are organized and parsed, so it's not a big deal if you leave it as is, since I'll have to change it anyway. (One problem with the current code that I'm trying to address is that the modifiers can be specified in any order, which the current parsers don't allow.)
Leporacanthicus
commented
Nov 15, 2024
Contributor
Author
Leporacanthicus
left a comment
Leporacanthicus
left a comment
There was a problem hiding this comment.
Thanks for the comments. I've replied and fixed as I think appropriate.
| // In OpenMP 5.2 the non-comma syntax has been deprecated: keep the | ||
| // information about separator presence to emit a diagnostic if needed. | ||
| std::tuple<std::optional<std::list<TypeModifier>>, | ||
| std::tuple<OmpMapperIdentifier, // Mapper name |
Contributor
Author
There was a problem hiding this comment.
So, this was my solution to making the maybe work. Since we need to parse the "MAPPER" and then parenthesized(name) and a comma. If I move the optional around, the maybe parsing doesn't work.
Unless someone can let me know how to make something maybe parse to produce the wrapper thing optional.
Everything I've tried (and that's a lot - I couldn't list them all, but std::optional<Name> in the std::tuple was one of them.
The TypeModifier and IteratorModifer and Type gets away with it, because they don't have a keyword with a parenthesized "argument". so they can just do a list (which is allowed to be empty).
I did try again just now, but I can't come up with something that compiles...
flang/lib/Lower/OpenMP/Clauses.cpp
Outdated
| auto &t4 = std::get<parser::OmpMapperIdentifier>(inp.v.t); | ||
|
|
||
| if (t4.v) | ||
| TODO_NOLOC("OmpMapClause(MAPPER(...))"); |
Contributor
Author
There was a problem hiding this comment.
Fair point! I've updated the message to contain both a user-friendly explanation and the developer-relevant clause.
kparzysz
reviewed
Nov 15, 2024
| // In OpenMP 5.2 the non-comma syntax has been deprecated: keep the | ||
| // information about separator presence to emit a diagnostic if needed. | ||
| std::tuple<std::optional<std::list<TypeModifier>>, | ||
| std::tuple<OmpMapperIdentifier, // Mapper name |
Contributor
There was a problem hiding this comment.
The way this was intended to work is that
- You add
std::optional<std::list<YourModifier>>to the tuple in here. - Add the mapper parser to the
ModParserlist in MapModifiers in openmp-parsers.cpp. - Unpack it in
makeMapClauseand pass it to theOmpMapClauseconstructor.
Then in check-omp-structure.cpp you can verify that there is at most one mapper modifier in the list, and print a meaningful message.
Having said that, I'm working on modifier verification right now, which will change how they are organized and parsed, so it's not a big deal if you leave it as is, since I'll have to change it anyway. (One problem with the current code that I'm trying to address is that the modifiers can be specified in any order, which the current parsers don't allow.)
| const auto &mid{std::get<parser::OmpMapperIdentifier>(x.t)}; | ||
| if (const auto &mapperName{mid.v}) | ||
| mapperName->symbol = | ||
| &MakeSymbol(*mapperName, MiscDetails{MiscDetails::Kind::ConstructName}); |
Contributor
There was a problem hiding this comment.
IIUC, the MAPPER modifier should refer to a preexisting object, so this should be a name lookup, not creating a new symbol. The DECLARE MAPPER should declare the identifier (and create the symbol). If the lookup fails, you could create a dummy, and report it as an error later on.
Contributor
Author
There was a problem hiding this comment.
I've added a FindSymbol and using that. Have to still make a symbol for the testing purposes right now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This prepares for using the DECLARE MAPPER construct.
A check in lowering will say "Not implemented" when trying to use a mapper as some code is required to tie the mapper to the declared one.
Senantics check for the symbol generated.