-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[flang][OpenMP] Frontend support for DEVICE_SAFESYNC #163560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
REQUIRES clauses apply to the compilation unit, which the OpenMP spec defines as the program unit in Fortran. Don't set REQUIRES flags on all containing scopes, only on the containng program unit, where flags coming from different directives are gathered. If we wanted to set the flags on subprograms, we would need to first accummulate all of them, then propagate them down to all subprograms. That is not done as it is not necessary (the containing program unit is always available).
For each program unit, collect the set of requirements from REQUIRES directives in the source, and modules used by the program unit, and add them to the details of the program unit symbol. The requirements in the symbol details as now stored as clauses. Since requirements need to be emitted in the module files as OpenMP directives, this makes the clause emission straightforward via getOpenMPClauseName. Each program unit, including modules, the corresponding symbol will have the transitive closure of the requirements for everything contained or used in that program unit.
OpenMP 6.0 added an optional logical parameter to the requirement clauses (except ATOMIC_DEFAULT_MEM_ORDER) to indicate whether the clause should take effect or not. The parameter defaults to true if not specified. The parameter value is a compile-time constant expression, but it may require folding to get the final value. Since name resolution happens before folding, the argument expression needs to be analyzed by hand. The determination of the value needs to happen during name resolution because the requirement directives need to be available through module files (and the module reader doesn't to semantic checks beyonn name resolution).
Add parsing and semantic checks for DEVICE_SAFESYNC clause. No lowering.
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-semantics Author: Krzysztof Parzyszek (kparzysz) ChangesAdd parsing and semantic checks for DEVICE_SAFESYNC clause. No lowering. Full diff: https://github.com/llvm/llvm-project/pull/163560.diff 9 Files Affected:
diff --git a/flang/include/flang/Lower/OpenMP/Clauses.h b/flang/include/flang/Lower/OpenMP/Clauses.h
index 273e61166fd9d..74924661d9a03 100644
--- a/flang/include/flang/Lower/OpenMP/Clauses.h
+++ b/flang/include/flang/Lower/OpenMP/Clauses.h
@@ -214,6 +214,7 @@ using Depend = tomp::clause::DependT<TypeTy, IdTy, ExprTy>;
using Destroy = tomp::clause::DestroyT<TypeTy, IdTy, ExprTy>;
using Detach = tomp::clause::DetachT<TypeTy, IdTy, ExprTy>;
using Device = tomp::clause::DeviceT<TypeTy, IdTy, ExprTy>;
+using DeviceSafesync = tomp::clause::DeviceSafesyncT<TypeTy, IdTy, ExprTy>;
using DeviceType = tomp::clause::DeviceTypeT<TypeTy, IdTy, ExprTy>;
using DistSchedule = tomp::clause::DistScheduleT<TypeTy, IdTy, ExprTy>;
using Doacross = tomp::clause::DoacrossT<TypeTy, IdTy, ExprTy>;
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 8854dc2267c8e..7377a3a4e3ca1 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -557,6 +557,7 @@ class ParseTreeDumper {
NODE(OmpDeviceClause, Modifier)
NODE(parser, OmpDeviceModifier)
NODE_ENUM(OmpDeviceModifier, Value)
+ NODE(parser, OmpDeviceSafesyncClause)
NODE(parser, OmpDeviceTypeClause)
NODE_ENUM(OmpDeviceTypeClause, DeviceTypeDescription)
NODE(parser, OmpDirectiveName)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 18b583669421a..079c0d642c32b 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4406,6 +4406,14 @@ struct OmpDeviceClause {
std::tuple<MODIFIERS(), ScalarIntExpr> t;
};
+// Ref: [6.0:356-362]
+//
+// device-safesync-clause ->
+// DEVICE_SAFESYNC [(scalar-logical-const-expr)] // since 6.0
+struct OmpDeviceSafesyncClause {
+ WRAPPER_CLASS_BOILERPLATE(OmpDeviceSafesyncClause, ScalarLogicalConstantExpr);
+};
+
// Ref: [5.0:180-185], [5.1:210-216], [5.2:275]
//
// device-type-clause ->
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 60e569efa964a..c11a3dda7dc41 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -740,6 +740,18 @@ Device make(const parser::OmpClause::Device &inp,
/*DeviceDescription=*/makeExpr(t1, semaCtx)}};
}
+DeviceSafesync make(const parser::OmpClause::DeviceSafesync &inp,
+ semantics::SemanticsContext &semaCtx) {
+ // inp.v -> std::optional<parser::OmpDeviceSafesyncClause>
+ auto &&maybeRequired = maybeApply(
+ [&](const parser::OmpDeviceSafesyncClause &c) {
+ return makeExpr(c.v, semaCtx);
+ },
+ inp.v);
+
+ return DeviceSafesync{/*Required=*/std::move(maybeRequired)};
+}
+
DeviceType make(const parser::OmpClause::DeviceType &inp,
semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpDeviceTypeClause
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 0a70930a0db33..63216384a6582 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1150,6 +1150,9 @@ TYPE_PARSER( //
construct<OmpDestroyClause>(Parser<OmpObject>{}))))) ||
"DEVICE" >> construct<OmpClause>(construct<OmpClause::Device>(
parenthesized(Parser<OmpDeviceClause>{}))) ||
+ "DEVICE_SAFESYNC" >>
+ construct<OmpClause>(construct<OmpClause::DeviceSafesync>(
+ maybe(parenthesized(scalarLogicalConstantExpr)))) ||
"DEVICE_TYPE" >> construct<OmpClause>(construct<OmpClause::DeviceType>(
parenthesized(Parser<OmpDeviceTypeClause>{}))) ||
"DIST_SCHEDULE" >>
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 1c27df18fba20..7b7dfb87763a5 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1535,6 +1535,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPRequiresConstruct &x) {
[&](auto &&s) {
using TypeS = llvm::remove_cvref_t<decltype(s)>;
if constexpr ( //
+ std::is_same_v<TypeS, parser::OmpClause::DeviceSafesync> ||
std::is_same_v<TypeS, parser::OmpClause::DynamicAllocators> ||
std::is_same_v<TypeS, parser::OmpClause::ReverseOffload> ||
std::is_same_v<TypeS, parser::OmpClause::SelfMaps> ||
@@ -5155,6 +5156,10 @@ void OmpStructureChecker::Enter(
CheckAllowedRequiresClause(llvm::omp::Clause::OMPC_atomic_default_mem_order);
}
+void OmpStructureChecker::Enter(const parser::OmpClause::DeviceSafesync &x) {
+ CheckAllowedRequiresClause(llvm::omp::Clause::OMPC_device_safesync);
+}
+
void OmpStructureChecker::Enter(const parser::OmpClause::DynamicAllocators &x) {
CheckAllowedRequiresClause(llvm::omp::Clause::OMPC_dynamic_allocators);
}
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 7067ed3d99286..db061bdce18ea 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -586,6 +586,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
[&](auto &&s) {
using TypeS = llvm::remove_cvref_t<decltype(s)>;
if constexpr ( //
+ std::is_same_v<TypeS, OmpClause::DeviceSafesync> ||
std::is_same_v<TypeS, OmpClause::DynamicAllocators> ||
std::is_same_v<TypeS, OmpClause::ReverseOffload> ||
std::is_same_v<TypeS, OmpClause::SelfMaps> ||
diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
index 9153dd1c56b7f..09cbb7f9fcf4e 100644
--- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
+++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
@@ -541,6 +541,14 @@ struct DeviceT {
std::tuple<OPT(DeviceModifier), DeviceDescription> t;
};
+// [6.0:362]
+template <typename T, typename I, typename E> //
+struct DeviceSafesyncT {
+ using Requires = E;
+ using WrapperTrait = std::true_type;
+ OPT(Requires) v;
+};
+
// V5.2: [13.1] `device_type` clause
template <typename T, typename I, typename E> //
struct DeviceTypeT {
@@ -1331,14 +1339,15 @@ using WrapperClausesT = std::variant<
AtomicDefaultMemOrderT<T, I, E>, AtT<T, I, E>, BindT<T, I, E>,
CollapseT<T, I, E>, ContainsT<T, I, E>, CopyinT<T, I, E>,
CopyprivateT<T, I, E>, DefaultT<T, I, E>, DestroyT<T, I, E>,
- DetachT<T, I, E>, DeviceTypeT<T, I, E>, DynamicAllocatorsT<T, I, E>,
- EnterT<T, I, E>, ExclusiveT<T, I, E>, FailT<T, I, E>, FilterT<T, I, E>,
- FinalT<T, I, E>, FirstprivateT<T, I, E>, HasDeviceAddrT<T, I, E>,
- HintT<T, I, E>, HoldsT<T, I, E>, InclusiveT<T, I, E>, IndirectT<T, I, E>,
- InitializerT<T, I, E>, IsDevicePtrT<T, I, E>, LinkT<T, I, E>,
- MessageT<T, I, E>, NocontextT<T, I, E>, NontemporalT<T, I, E>,
- NovariantsT<T, I, E>, NumTeamsT<T, I, E>, NumThreadsT<T, I, E>,
- OrderedT<T, I, E>, PartialT<T, I, E>, PriorityT<T, I, E>, PrivateT<T, I, E>,
+ DetachT<T, I, E>, DeviceSafesyncT<T, I, E>, DeviceTypeT<T, I, E>,
+ DynamicAllocatorsT<T, I, E>, EnterT<T, I, E>, ExclusiveT<T, I, E>,
+ FailT<T, I, E>, FilterT<T, I, E>, FinalT<T, I, E>, FirstprivateT<T, I, E>,
+ HasDeviceAddrT<T, I, E>, HintT<T, I, E>, HoldsT<T, I, E>,
+ InclusiveT<T, I, E>, IndirectT<T, I, E>, InitializerT<T, I, E>,
+ IsDevicePtrT<T, I, E>, LinkT<T, I, E>, MessageT<T, I, E>,
+ NocontextT<T, I, E>, NontemporalT<T, I, E>, NovariantsT<T, I, E>,
+ NumTeamsT<T, I, E>, NumThreadsT<T, I, E>, OrderedT<T, I, E>,
+ PartialT<T, I, E>, PriorityT<T, I, E>, PrivateT<T, I, E>,
ProcBindT<T, I, E>, ReverseOffloadT<T, I, E>, SafelenT<T, I, E>,
SelfMapsT<T, I, E>, SeverityT<T, I, E>, SharedT<T, I, E>, SimdlenT<T, I, E>,
SizesT<T, I, E>, PermutationT<T, I, E>, ThreadLimitT<T, I, E>,
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index edcf7a92c2a84..00c43d084b216 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -163,6 +163,10 @@ def OMPC_Device : Clause<[Spelling<"device">]> {
let clangClass = "OMPDeviceClause";
let flangClass = "OmpDeviceClause";
}
+def OMPC_DeviceSafesync : Clause<[Spelling<"device_safesync">]> {
+ let flangClass = "OmpDeviceSafesyncClause";
+ let isValueOptional = true;
+}
def OMPC_DeviceType : Clause<[Spelling<"device_type">]> {
let flangClass = "OmpDeviceTypeClause";
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but this needs lit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any lowering tests for this?
This code doesn't do lowering, so no. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thanks
Add parsing and semantic checks for DEVICE_SAFESYNC clause. No lowering.