[flang][openmp] Adds Parser and Semantic Support for Interop Construct, and Init and Use Clauses. #120584
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
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-openmp Author: None (swatheesh-mcw) ChangesAdds Parser and Semantic Support for the below construct and clauses:
Note: Patch is 27.78 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/120584.diff 11 Files Affected:
diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index 3a689c335c81c0..ac70de588c7d9f 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -525,6 +525,14 @@ struct NodeVisitor {
READ_FEATURE(OmpScheduleClause)
READ_FEATURE(OmpScheduleClause::Kind)
READ_FEATURE(OmpScheduleClause::Modifier)
+ READ_FEATURE(InteropType)
+ READ_FEATURE(InteropType::Kind)
+ READ_FEATURE(InteropPreference)
+ READ_FEATURE(OmpInitClause)
+ READ_FEATURE(OmpInitClause::InteropModifier)
+ READ_FEATURE(OmpInitClause::InteropTypes)
+ READ_FEATURE(OmpInitClause::InteropVar)
+ READ_FEATURE(OmpUseClause)
READ_FEATURE(OmpDeviceModifier)
READ_FEATURE(OmpDeviceClause)
READ_FEATURE(OmpDeviceClause::Modifier)
@@ -545,6 +553,7 @@ struct NodeVisitor {
READ_FEATURE(OpenACCConstruct)
READ_FEATURE(OpenACCDeclarativeConstruct)
READ_FEATURE(OpenACCLoopConstruct)
+ READ_FEATURE(OpenMPInteropConstruct)
READ_FEATURE(OpenACCRoutineConstruct)
READ_FEATURE(OpenACCStandaloneDeclarativeConstruct)
READ_FEATURE(OpenACCStandaloneConstruct)
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 7821d40a644a27..5020d481dff29f 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -620,6 +620,14 @@ class ParseTreeDumper {
NODE_ENUM(OmpDeviceModifier, Value)
NODE(parser, OmpDeviceTypeClause)
NODE_ENUM(OmpDeviceTypeClause, DeviceTypeDescription)
+ NODE(parser, InteropType)
+ NODE_ENUM(InteropType, Kind)
+ NODE(parser, InteropPreference)
+ NODE(parser, OmpInitClause)
+ NODE(OmpInitClause, InteropModifier)
+ NODE(OmpInitClause, InteropTypes)
+ NODE(OmpInitClause, InteropVar)
+ NODE(parser, OmpUseClause)
NODE(parser, OmpUpdateClause)
NODE(parser, OmpChunkModifier)
NODE_ENUM(OmpChunkModifier, Value)
@@ -639,6 +647,7 @@ class ParseTreeDumper {
NODE(parser, OpenACCDeclarativeConstruct)
NODE(parser, OpenACCEndConstruct)
NODE(parser, OpenACCLoopConstruct)
+ NODE(parser, OpenMPInteropConstruct)
NODE(parser, OpenACCRoutineConstruct)
NODE(parser, OpenACCStandaloneDeclarativeConstruct)
NODE(parser, OpenACCStandaloneConstruct)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 2ef593b3e50daa..3a0f41a67b01b0 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4158,6 +4158,36 @@ struct OmpUpdateClause {
std::variant<OmpDependenceType, OmpTaskDependenceType> u;
};
+// REF: [5.1:217-220], [5.2:293-294]
+//
+// init-clause -> INIT ([interop-modifier,] [interop-type,]
+// interop-type: interop-var)
+// interop-modifier: prefer_type(preference-list)
+// interop-type: target, targetsync
+// There can be at most only two interop-type.
+struct InteropType {
+ ENUM_CLASS(Kind, Target, TargetSync)
+ WRAPPER_CLASS_BOILERPLATE(InteropType, Kind);
+};
+
+struct InteropPreference {
+ UNION_CLASS_BOILERPLATE(InteropPreference);
+ std::variant<CharLiteralConstant, ScalarIntConstantExpr> u;
+};
+
+struct OmpInitClause {
+ TUPLE_CLASS_BOILERPLATE(OmpInitClause);
+ WRAPPER_CLASS(InteropModifier, std::list<InteropPreference>);
+ WRAPPER_CLASS(InteropTypes, std::list<InteropType>);
+ WRAPPER_CLASS(InteropVar, OmpObject);
+ std::tuple<std::optional<InteropModifier>, InteropTypes, InteropVar> t;
+};
+
+// REF: [5.1:217-220], [5.2:294]
+//
+// 14.1.3 use-clause -> USE (interop-var)
+WRAPPER_CLASS(OmpUseClause, OmpObject);
+
// OpenMP Clauses
struct OmpClause {
UNION_CLASS_BOILERPLATE(OmpClause);
@@ -4528,6 +4558,15 @@ struct OmpSimpleStandaloneDirective {
CharBlock source;
};
+// Ref: [5.1:217-220], [5.2:291-292]
+//
+// interop -> INTEROP clause[ [ [,] clause]...]
+struct OpenMPInteropConstruct {
+ TUPLE_CLASS_BOILERPLATE(OpenMPInteropConstruct);
+ CharBlock source;
+ std::tuple<Verbatim, OmpClauseList> t;
+};
+
struct OpenMPSimpleStandaloneConstruct {
TUPLE_CLASS_BOILERPLATE(OpenMPSimpleStandaloneConstruct);
CharBlock source;
@@ -4539,7 +4578,7 @@ struct OpenMPStandaloneConstruct {
CharBlock source;
std::variant<OpenMPSimpleStandaloneConstruct, OpenMPFlushConstruct,
OpenMPCancelConstruct, OpenMPCancellationPointConstruct,
- OpenMPDepobjConstruct>
+ OpenMPDepobjConstruct, OpenMPInteropConstruct>
u;
};
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index b07e89d201d198..b6328dda8441c9 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2752,6 +2752,13 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
TODO(converter.getCurrentLocation(), "OpenMPDepobjConstruct");
}
+static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
+ semantics::SemanticsContext &semaCtx,
+ lower::pft::Evaluation &eval,
+ const parser::OpenMPInteropConstruct &interopConstruct) {
+ TODO(converter.getCurrentLocation(), "OpenMPInteropConstruct");
+}
+
static void
genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 67385c03f66c80..ac9f7a45dbd77e 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -531,6 +531,22 @@ TYPE_PARSER(
// OpenMPv5.2 12.5.2 detach-clause -> DETACH (event-handle)
TYPE_PARSER(construct<OmpDetachClause>(Parser<OmpObject>{}))
+// InteropTypes
+TYPE_PARSER(construct<InteropType>(
+ "TARGETSYNC" >> pure(InteropType::Kind::TargetSync) ||
+ "TARGET" >> pure(InteropType::Kind::Target)))
+
+// InteropPreference
+TYPE_PARSER(construct<InteropPreference>(
+ construct<InteropPreference>(charLiteralConstant) ||
+ construct<InteropPreference>(scalarIntConstantExpr)))
+
+// init clause
+TYPE_PARSER(construct<OmpInitClause>(
+ maybe(verbatim("PREFER_TYPE"_tok) >>
+ parenthesized(nonemptyList(Parser<InteropPreference>{})) / ","),
+ nonemptyList(Parser<InteropType>{}) / ":", Parser<OmpObject>{}))
+
// 2.8.1 ALIGNED (list: alignment)
TYPE_PARSER(construct<OmpAlignedClause>(Parser<OmpObjectList>{},
maybe(":" >> nonemptyList(Parser<OmpAlignedClause::Modifier>{}))))
@@ -644,6 +660,8 @@ TYPE_PARSER(
"IF" >> construct<OmpClause>(construct<OmpClause::If>(
parenthesized(Parser<OmpIfClause>{}))) ||
"INBRANCH" >> construct<OmpClause>(construct<OmpClause::Inbranch>()) ||
+ "INIT" >> construct<OmpClause>(construct<OmpClause::Init>(
+ parenthesized(Parser<OmpInitClause>{}))) ||
"INCLUSIVE" >> construct<OmpClause>(construct<OmpClause::Inclusive>(
parenthesized(Parser<OmpObjectList>{}))) ||
"IS_DEVICE_PTR" >> construct<OmpClause>(construct<OmpClause::IsDevicePtr>(
@@ -718,6 +736,8 @@ TYPE_PARSER(
parenthesized(scalarIntExpr))) ||
"TO" >> construct<OmpClause>(construct<OmpClause::To>(
parenthesized(Parser<OmpToClause>{}))) ||
+ "USE" >> construct<OmpClause>(construct<OmpClause::Use>(
+ parenthesized(Parser<OmpObject>{}))) ||
"USE_DEVICE_PTR" >> construct<OmpClause>(construct<OmpClause::UseDevicePtr>(
parenthesized(Parser<OmpObjectList>{}))) ||
"USE_DEVICE_ADDR" >>
@@ -877,6 +897,10 @@ TYPE_PARSER(sourced(construct<OmpSimpleStandaloneDirective>(first(
TYPE_PARSER(sourced(construct<OpenMPSimpleStandaloneConstruct>(
Parser<OmpSimpleStandaloneDirective>{}, Parser<OmpClauseList>{})))
+// 14.1 Interop construct
+TYPE_PARSER(sourced(construct<OpenMPInteropConstruct>(
+ verbatim("INTEROP"_tok), sourced(Parser<OmpClauseList>{}))))
+
// Standalone Constructs
TYPE_PARSER(
sourced(construct<OpenMPStandaloneConstruct>(
@@ -885,7 +909,9 @@ TYPE_PARSER(
construct<OpenMPStandaloneConstruct>(Parser<OpenMPCancelConstruct>{}) ||
construct<OpenMPStandaloneConstruct>(
Parser<OpenMPCancellationPointConstruct>{}) ||
- construct<OpenMPStandaloneConstruct>(Parser<OpenMPDepobjConstruct>{})) /
+ construct<OpenMPStandaloneConstruct>(Parser<OpenMPDepobjConstruct>{}) ||
+ construct<OpenMPStandaloneConstruct>(
+ Parser<OpenMPInteropConstruct>{})) /
endOfLine)
// Directives enclosing structured-block
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 0a6af7435b4a22..00957c6c424019 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2097,6 +2097,15 @@ class UnparseVisitor {
Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
Walk(std::get<OmpObjectList>(x.t));
}
+ void Unparse(const OmpInitClause::InteropTypes &x) { Walk(x.v, ","); }
+ void Unparse(const OmpInitClause::InteropModifier &x) { Walk(x.v, ","); }
+ void Unparse(const OmpInitClause &x) {
+ Walk("PREFER_TYPE(",
+ std::get<std::optional<OmpInitClause::InteropModifier>>(x.t), "),");
+ Walk(std::get<OmpInitClause::InteropTypes>(x.t));
+ Put(": ");
+ Walk(std::get<OmpInitClause::InteropVar>(x.t));
+ }
void Unparse(const OmpMapClause &x) {
using Modifier = OmpMapClause::Modifier;
Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
@@ -2638,6 +2647,15 @@ class UnparseVisitor {
Put(")");
Walk(std::get<std::optional<OmpReductionInitializerClause>>(x.t));
}
+
+ void Unparse(const OpenMPInteropConstruct &x) {
+ BeginOpenMP();
+ Word("!$OMP INTEROP");
+ Walk(std::get<OmpClauseList>(x.t));
+ Put("\n");
+ EndOpenMP();
+ }
+
bool Pre(const OpenMPDeclarativeConstruct &x) {
BeginOpenMP();
Word("!$OMP ");
@@ -2923,6 +2941,7 @@ class UnparseVisitor {
OmpDeviceTypeClause, DeviceTypeDescription) // OMP device_type
WALK_NESTED_ENUM(OmpReductionModifier, Value) // OMP reduction-modifier
WALK_NESTED_ENUM(OmpExpectation, Value) // OMP motion-expectation
+ WALK_NESTED_ENUM(InteropType, Kind) // OMP InteropVar
WALK_NESTED_ENUM(OmpCancelType, Type) // OMP cancel-type
WALK_NESTED_ENUM(OmpOrderClause, Ordering) // OMP ordering
WALK_NESTED_ENUM(OmpOrderModifier, Value) // OMP order-modifier
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 95b962f5daf57c..68a041fe6e9eeb 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -4801,6 +4801,91 @@ void OmpStructureChecker::Leave(const parser::DoConstruct &x) {
Base::Leave(x);
}
+void OmpStructureChecker::Enter(const parser::OpenMPInteropConstruct &x) {
+ bool isDependClauseOccured{false};
+ int targetCount{0}, targetSyncCount{0};
+ const auto &dir{std::get<parser::Verbatim>(x.t)};
+ std::list<std::string> ObjectNameList;
+ PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_interop);
+ const auto &clauseList{std::get<parser::OmpClauseList>(x.t)};
+ for (const auto &clause : clauseList.v) {
+ common::visit(
+ common::visitors{
+ [&](const parser::OmpClause::Init &InitClause) {
+ const auto &InteropTypeList{
+ std::get<parser::OmpInitClause::InteropTypes>(
+ InitClause.v.t)};
+ for (auto &InteropTypeVal : InteropTypeList.v) {
+ if (*(parser::Unwrap<parser::InteropType::Kind>(
+ InteropTypeVal)) ==
+ parser::InteropType::Kind::TargetSync) {
+ ++targetSyncCount;
+ } else {
+ ++targetCount;
+ }
+ if (targetCount > 1 || targetSyncCount > 1) {
+ context_.Say(GetContext().directiveSource,
+ "Each interop-type may be specified at most once."_err_en_US);
+ }
+ }
+ const auto &InteropVar{parser::Unwrap<parser::OmpObject>(
+ std::get<parser::OmpInitClause::InteropVar>(InitClause.v.t))};
+ const auto *name{parser::Unwrap<parser::Name>(InteropVar)};
+ const auto ObjectName{name->ToString()};
+ if (ObjectNameList.end() !=
+ std::find(ObjectNameList.begin(), ObjectNameList.end(),
+ ObjectName)) {
+ context_.Say(GetContext().directiveSource,
+ "Each interop-var may be specified for at most one action-clause of each interop construct."_err_en_US);
+ } else {
+ ObjectNameList.push_back(ObjectName);
+ }
+ },
+ [&](const parser::OmpClause::Depend &DependClause) {
+ isDependClauseOccured = true;
+ },
+ [&](const parser::OmpClause::Destroy &DestroyClause) {
+ const auto &InteropVar{
+ parser::Unwrap<parser::OmpObject>(DestroyClause.v)};
+ const auto *name{parser::Unwrap<parser::Name>(InteropVar)};
+ const auto ObjectName{name->ToString()};
+ if (ObjectNameList.end() !=
+ std::find(ObjectNameList.begin(), ObjectNameList.end(),
+ ObjectName)) {
+ context_.Say(GetContext().directiveSource,
+ "Each interop-var may be specified for at most one action-clause of each interop construct."_err_en_US);
+ } else {
+ ObjectNameList.push_back(ObjectName);
+ }
+ },
+ [&](const parser::OmpClause::Use &UseClause) {
+ const auto &InteropVar{
+ parser::Unwrap<parser::OmpObject>(UseClause.v)};
+ const auto *name{parser::Unwrap<parser::Name>(InteropVar)};
+ const auto ObjectName{name->ToString()};
+ if (ObjectNameList.end() !=
+ std::find(ObjectNameList.begin(), ObjectNameList.end(),
+ ObjectName)) {
+ context_.Say(GetContext().directiveSource,
+ "Each interop-var may be specified for at most one action-clause of each interop construct."_err_en_US);
+ } else {
+ ObjectNameList.push_back(ObjectName);
+ }
+ },
+ [&](const auto &) {},
+ },
+ clause.u);
+ }
+ if (isDependClauseOccured && !targetSyncCount) {
+ context_.Say(GetContext().directiveSource,
+ "A depend clause can only appear on the directive if the interop-type includes targetsync"_err_en_US);
+ }
+}
+
+void OmpStructureChecker::Leave(const parser::OpenMPInteropConstruct &) {
+ dirContext_.pop_back();
+}
+
void OmpStructureChecker::CheckAllowedRequiresClause(llvmOmpClause clause) {
CheckAllowedClause(clause);
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 346a7bed9138f0..7b3d9fa3d736b8 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -73,6 +73,8 @@ class OmpStructureChecker
void Enter(const parser::OpenMPConstruct &);
void Leave(const parser::OpenMPConstruct &);
+ void Enter(const parser::OpenMPInteropConstruct &);
+ void Leave(const parser::OpenMPInteropConstruct &);
void Enter(const parser::OpenMPLoopConstruct &);
void Leave(const parser::OpenMPLoopConstruct &);
void Enter(const parser::OmpEndLoopDirective &);
diff --git a/flang/test/Parser/OpenMP/interop-construct.f90 b/flang/test/Parser/OpenMP/interop-construct.f90
new file mode 100644
index 00000000000000..9fd2b3d63d5c5d
--- /dev/null
+++ b/flang/test/Parser/OpenMP/interop-construct.f90
@@ -0,0 +1,204 @@
+! RUN: %flang_fc1 -fdebug-unparse -fopenmp %s | FileCheck --ignore-case --check-prefix="UNPARSE" %s
+! RUN: %flang_fc1 -fdebug-dump-parse-tree-no-sema -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s
+
+subroutine test_interop_01()
+ !$omp interop device(1)
+ print *,'pass'
+end subroutine test_interop_01
+
+!UNPARSE: SUBROUTINE test_interop_01
+!UNPARSE: !$OMP INTEROP DEVICE(1_4)
+!UNPARSE: PRINT *, "pass"
+!UNPARSE: END SUBROUTINE test_interop_01
+
+!PARSE-TREE: | SubroutineStmt
+!PARSE-TREE: | | Name = 'test_interop_01'
+!PARSE-TREE: | SpecificationPart
+!PARSE-TREE: | | ImplicitPart ->
+!PARSE-TREE: | ExecutionPart -> Block
+!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPInteropConstruct
+!PARSE-TREE: | | | Verbatim
+!PARSE-TREE: | | | OmpClauseList -> OmpClause -> Device -> OmpDeviceClause
+!PARSE-TREE: | | | | Scalar -> Integer -> Expr -> LiteralConstant -> IntLiteralConstant = '1'
+!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> PrintStmt
+!PARSE-TREE: | | | Format -> Star
+!PARSE-TREE: | | | OutputItem -> Expr -> LiteralConstant -> CharLiteralConstant
+!PARSE-TREE: | | | | string = 'pass'
+!PARSE-TREE: | EndSubroutineStmt -> Name = 'test_interop_01'
+
+subroutine test_interop_02()
+ use omp_lib
+ integer(omp_interop_kind) :: obj1, obj2, obj3
+ !$omp interop init(targetsync: obj) use(obj1) destroy(obj3)
+ print *,'pass'
+end subroutine test_interop_02
+
+!UNPARSE: SUBROUTINE test_interop_02
+!UNPARSE: USE :: omp_lib
+!UNPARSE: INTEGER(KIND=8_4) obj1, obj2, obj3
+!UNPARSE: !$OMP INTEROP INIT(TARGETSYNC: obj) USE(obj1) DESTROY(obj3)
+!UNPARSE: PRINT *, "pass"
+!UNPARSE: END SUBROUTINE test_interop_02
+
+!PARSE-TREE: | SubroutineStmt
+!PARSE-TREE: | | Name = 'test_interop_02'
+!PARSE-TREE: | SpecificationPart
+!PARSE-TREE: | | UseStmt
+!PARSE-TREE: | | | Name = 'omp_lib'
+!PARSE-TREE: | | ImplicitPart ->
+!PARSE-TREE: | | DeclarationConstruct -> SpecificationConstruct -> TypeDeclarationStmt
+!PARSE-TREE: | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec -> KindSelector -> Scalar -> Integer -> Constant -> Expr -> Designator -> DataRef -> Name = 'omp_interop_kind'
+!PARSE-TREE: | | | EntityDecl
+!PARSE-TREE: | | | | Name = 'obj1'
+!PARSE-TREE: | | | EntityDecl
+!PARSE-TREE: | | | | Name = 'obj2'
+!PARSE-TREE: | | | EntityDecl
+!PARSE-TREE: | | | | Name = 'obj3'
+!PARSE-TREE: | ExecutionPart -> Block
+!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPInteropConstruct
+!PARSE-TREE: | | | Verbatim
+!PARSE-TREE: | | | OmpClauseList -> OmpClause -> Init -> OmpInitClause
+!PARSE-TREE: | | | | InteropTypes -> InteropType -> Kind = TargetSync
+!PARSE-TREE: | | | | InteropVar -> OmpObject -> Designator -> DataRef -> Name = 'obj'
+!PARSE-TREE: | | | OmpClause -> Use -> OmpUseClause -> OmpObject -> Designator -> DataRef -> Name = 'obj1'
+!PARSE-TREE: | | | OmpClause -> Destroy -> OmpDestroyClause -> OmpObject -> Designator -> DataRef -> Name = 'obj3'
+!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> PrintStmt
+!PARSE-TREE: | | | Format -> Star
+!PARSE-TREE: | | | OutputItem -> Expr -> LiteralConstant -> CharLiteralConstant
+!PARSE-TREE: | | | | string = 'pass'
+!PARSE-TREE: | EndSubroutineStmt -> Name = 'test_interop_02'
+
+subroutine test_interop_03()
+ use omp_lib
+ Integer(omp_interop_kind) :: obj
+ !$omp interop init(targetsync: obj) depend(inout: obj)
+ print *,'pass'
+end subroutine test_interop_03
+
+!UNPARSE: SUBROUTINE test_interop_03
+!UNPARSE: USE :: omp_lib
+!UNPARSE: INTEGER(KIND=8_4) obj
+!UNPARSE: !$OMP INTEROP INIT(TARGETSYNC: obj) DEPEND(INOUT: obj)
+!UNPARSE: PRINT *, "pass"
+!UNPARSE: END SUBROUTINE test_interop_03
+
+!PARSE-TREE: | SubroutineStmt
+!PARSE-TREE: | | Name = 'test_interop_03'
+!PARSE-TREE: | SpecificationPart
+!PARSE-TREE: | | UseStmt
+!PARSE-TREE: | | | Name = 'omp_lib'
+!PARSE-TREE: | | ImplicitPart ->
+!PARSE-TREE: | | DeclarationConstruct -> SpecificationConstruct -> TypeDeclarationStmt
+!PARSE-TREE: | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec -> KindSelector -> Scalar -> Integer -> Constant -> Expr -> Designator -> DataRef -> Name = 'omp_interop_kind'
+!PARSE-TREE: | | | EntityDecl
+!PARSE-TREE: | | | | Name = 'obj'
+!PARSE-TREE: | ExecutionPart -> Block
+!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPInteropConstruct
+!...
[truncated]
|
kiranchandramohan
requested review from
kiranchandramohan,
Leporacanthicus and
kparzysz
|
Please follow the common method for implementing modifiers, for example as in #117786.
|
Sure @kparzysz . Will make the requested change. Thanks! |
|
Please add a prefix to your title so it is easy to identify the area it touches
|
swatheesh-mcw
changed the title
Hi @kparzysz, I tried making the
Can you suggest or direct me through an example where I can refer to create parser support for Thanks in advance! |
Hi @kparzysz , I have addressed the review comments on #9e748a6. Please let me know if any changes are required. Thanks! |
I have added another commit #0fdb9c8 to preserve the alphabetical ordering of modifier declaration and definitions. Please let me know if any modifications are required. |
|
Ping for review |
kiranchandramohan
added a commit
to kiranchandramohan/llvm-project
that referenced
this pull request
Mar 10, 2025
Support is added for parsing and semantics. Lowering will emit a TODO error. append_args clause and use of interop inside have some overlap with llvm#120584.
There was a problem hiding this comment.
@swatheesh-mcw Thanks for this patch. Could you rebase?
For error messages, we usually have the OpenMP constructs, clauses or reserved terms in caps. Could you update the error messages and tests appropriately?
Could you also add a Lowering test that checks for the TODO message. eg: https://github.com/llvm/llvm-project/blob/main/flang/test/Lower/OpenMP/Todo/dispatch.f90
kiranchandramohan
added a commit
to kiranchandramohan/llvm-project
that referenced
this pull request
Mar 10, 2025
Support is added for parsing and semantics. Lowering will emit a TODO error. append_args clause and use of interop inside have some overlap with llvm#120584.
|
Sure @kiranchandramohan, will rebase and update the error messages and tests. Thanks! |
swatheesh-mcw
force-pushed
the
swatheesh-interop-latest
branch
from
0fdb9c8 to
2e9e552
Compare
kparzysz
reviewed
Mar 13, 2025
Comment on lines
5599
to
5601
| for (auto it{interopTypeModifier.begin()}, | ||
| end{interopTypeModifier.end()}; | ||
| it != end; ++it) { |
There was a problem hiding this comment.
Use range-for: for (auto &it : interopTypeModifier)
There was a problem hiding this comment.
Addressed the review comment in #2370fb7.
Comment on lines
5619
to
5621
| if (ObjectNameList.end() != | ||
| std::find(ObjectNameList.begin(), ObjectNameList.end(), | ||
| ObjectName)) { |
There was a problem hiding this comment.
Please use llvm::is_contained to check if some element is present in a container (see llvm/include/llvm/ADT/STLExtras.h)
There was a problem hiding this comment.
Addressed the review comment in #2370fb7.
Comment on lines
5602
to
5604
| if (parser::ToUpperCaseLetters( | ||
| parser::OmpInteropType::EnumToString((*it)->v)) == | ||
| "TARGETSYNC") { |
There was a problem hiding this comment.
Don't compare strings, check if v == OmpInteropType::Value::TargetSync instead.
There was a problem hiding this comment.
Addressed the review comment in #2370fb7.
| bool isDependClauseOccured{false}; | ||
| int targetCount{0}, targetSyncCount{0}; | ||
| const auto &dir{std::get<parser::Verbatim>(x.t)}; | ||
| std::list<std::string> ObjectNameList; |
There was a problem hiding this comment.
Don't use strings, use const Symbol * to identify objects. Also, use std::set instead of list, and check the result of the insert operation to see if the element was already in the set.
There was a problem hiding this comment.
Addressed the review comment in #2370fb7.
| for (const auto &clause : clauseList.v) { | ||
| common::visit( | ||
| common::visitors{ | ||
| [&](const parser::OmpClause::Init &InitClause) { |
There was a problem hiding this comment.
Variable names start with a lowercase letter.
There was a problem hiding this comment.
Addressed the review comment in #2370fb7.
| } | ||
| } | ||
| } | ||
| const auto &InteropVar{parser::Unwrap<parser::OmpObject>( |
There was a problem hiding this comment.
Addressed the review comment in #2370fb7.
| ObjectNameList.push_back(ObjectName); | ||
| } | ||
| }, | ||
| [&](const parser::OmpClause::Depend &DependClause) { |
There was a problem hiding this comment.
Lowercase. There are more places with this issue.
There was a problem hiding this comment.
Addressed the review comment in #2370fb7.
kparzysz
reviewed
Mar 13, 2025
flang/lib/Parser/unparse.cpp
Outdated
| void Unparse(const OmpInitClause &x) { | ||
| using Modifier = OmpInitClause::Modifier; | ||
| auto &modifiers{std::get<std::optional<std::list<Modifier>>>(x.t)}; | ||
| bool is_type_start = true; |
There was a problem hiding this comment.
Use camelCase for variable names, here and everywhere else in this function.
There was a problem hiding this comment.
Addressed the review comment in #2370fb7.
Comment on lines
5609
to
5612
| if (targetCount > 1 || targetSyncCount > 1) { | ||
| context_.Say(GetContext().directiveSource, | ||
| "Each interop-type may be specified at most once."_err_en_US); | ||
| } |
There was a problem hiding this comment.
Move this check out of the loop, otherwise you could print this message multiple times. Consider this: init(target, target, targetsync, targetsync: x).
There was a problem hiding this comment.
Addressed the review comment in #2370fb7.
flang/lib/Parser/unparse.cpp
Outdated
| void Unparse(const OmpInitClause &x) { | ||
| using Modifier = OmpInitClause::Modifier; | ||
| auto &modifiers{std::get<std::optional<std::list<Modifier>>>(x.t)}; | ||
| bool isTypeStart = true; |
There was a problem hiding this comment.
Nit: Braced initialization
There was a problem hiding this comment.
Addressed this in #ebe9848.
| if (llvm::is_contained(objectSymbolList, objectSymbol)) { | ||
| context_.Say( | ||
| GetContext().directiveSource, | ||
| "Each interop-var may be specified for at most one action-clause of each interop construct."_err_en_US); |
There was a problem hiding this comment.
Suggested change
| "Each interop-var may be specified for at most one action-clause of each interop construct."_err_en_US); | |
| "Each interop-var may be specified for at most one action-clause of each INTEROP construct."_err_en_US); |
There was a problem hiding this comment.
Addressed this in #ebe9848.
| if (isDependClauseOccured && !targetSyncCount) { | ||
| context_.Say( | ||
| GetContext().directiveSource, | ||
| "A depend clause can only appear on the directive if the interop-type includes targetsync"_err_en_US); |
There was a problem hiding this comment.
Suggested change
| "A depend clause can only appear on the directive if the interop-type includes targetsync"_err_en_US); | |
| "A DEPEND clause can only appear on the directive if the interop-type includes TARGETSYNC"_err_en_US); |
There was a problem hiding this comment.
Addressed this in #ebe9848.
| if (llvm::is_contained(objectSymbolList, objectSymbol)) { | ||
| context_.Say( | ||
| GetContext().directiveSource, | ||
| "Each interop-var may be specified for at most one action-clause of each interop construct."_err_en_US); |
There was a problem hiding this comment.
Suggested change
| "Each interop-var may be specified for at most one action-clause of each interop construct."_err_en_US); | |
| "Each interop-var may be specified for at most one action-clause of each INTEROP construct."_err_en_US); |
There was a problem hiding this comment.
Addressed this in #ebe9848.
flang/lib/Parser/openmp-parsers.cpp
Outdated
| @@ -1165,6 +1190,10 @@ TYPE_PARSER(sourced(construct<OmpSimpleStandaloneDirective>(first( | |||
| TYPE_PARSER(sourced(construct<OpenMPSimpleStandaloneConstruct>( | |||
| Parser<OmpSimpleStandaloneDirective>{}, Parser<OmpClauseList>{}))) | |||
|
|
|||
| // 14.1 Interop construct | |||
There was a problem hiding this comment.
Prefix with the standard version number.
There was a problem hiding this comment.
Addressed this in #ebe9848.
|
Could you rebase to address the conflicts in |
- Interop Construct - Init Clause - Use Clause Note: The other clauses supported by Interop Construct such as Destroy, Use, Depend and Device are added already.
… interop construct.
swatheesh-mcw
force-pushed
the
swatheesh-interop-latest
branch
from
ebe9848 to
d7d40c3
Compare
|
Hi @kiranchandramohan, I have rebased and updated a comment in |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Please fix the clang-format issue. Let me know if you want me to submit this after the format fix. |
|
Hi @kiranchandramohan, sorry there is some issue with my clang-format configuration in my code editor. Just pushed the commit after modifying the |
kparzysz
reviewed
Mar 18, 2025
| @@ -0,0 +1,8 @@ | |||
| ! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s | |||
There was a problem hiding this comment.
Please add !REQUIRES: openmp_runtime at the beginning of the file.
There was a problem hiding this comment.
Addressed the comment in #40b5b8d.
| @@ -0,0 +1,203 @@ | |||
| ! RUN: %flang_fc1 -fdebug-unparse -fopenmp-version=52 -fopenmp %s | FileCheck --ignore-case --check-prefix="UNPARSE" %s | |||
There was a problem hiding this comment.
Please add !REQUIRES: openmp_runtime at the beginning of the file.
There was a problem hiding this comment.
Addressed the comment in #40b5b8d.
kiranchandramohan
approved these changes
Mar 19, 2025
|
I will merge now. Please let us know if there are any CI failures. |
|
@swatheesh-mcw Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/22946 Here is the relevant piece of the build log for the reference |
kiranchandramohan
added a commit
that referenced
this pull request
Mar 19, 2025
…Construct, and Init and Use Clauses." (#132005) Reverts #120584 Reverting due to CI failure https://lab.llvm.org/buildbot/#/builders/157/builds/22946
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Mar 19, 2025
…or Interop Construct, and Init and Use Clauses." (#132005) Reverts llvm/llvm-project#120584 Reverting due to CI failure https://lab.llvm.org/buildbot/#/builders/157/builds/22946
|
Hi @kiranchandramohan, I could see that the LLVM builds are failing due to failure in written testcases. Is there something I need to do? Thanks. |
|
@swatheesh-mcw I had to revert the patch in (#132005) because of the failure. I think you have to add Can you revert #132005 and make the change suggested above? |
|
Sure @kiranchandramohan, I have created a new pull request #132343 to revert #132005. I will update the lower testcase there. Thanks! |
kiranchandramohan
added a commit
to kiranchandramohan/llvm-project
that referenced
this pull request
May 2, 2025
Support is added for parsing and semantics. Lowering will emit a TODO error. append_args clause and use of interop inside have some overlap with llvm#120584.
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.
Adds Parser and Semantic Support for the below construct and clauses:
Note:
The other clauses supported by Interop Construct such as Destroy, Use, Depend and Device are added already.