[flang-commits] [flang] [llvm] [flang][OpenMP] Split DEFAULT into DEFAULT(dsa) and DEFAULT(variant) (PR #212128)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Mon Jul 27 06:56:32 PDT 2026
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/212128
>From 1196889292e746c68ab3895508cb8eea05c17d15 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 2 Jul 2026 06:51:02 -0500
Subject: [PATCH 1/4] [flang][OpenMP] Split DEFAULT into DEFAULT(dsa) and
DEFAULT(variant)
The 5.0 and 5.1 specs used DEFAULT clause for what is now OTHERWISE.
Separate these two claues to be able to specify their properties
independently.
---
flang/include/flang/Parser/dump-parse-tree.h | 1 +
flang/include/flang/Parser/parse-tree.h | 20 +++++++------
flang/lib/Lower/OpenMP/Clauses.cpp | 24 +++++----------
flang/lib/Lower/OpenMP/OpenMP.cpp | 10 +++----
flang/lib/Parser/openmp-parsers.cpp | 16 ++++++----
flang/lib/Semantics/check-omp-variant.cpp | 3 +-
flang/lib/Semantics/resolve-directives.cpp | 30 +++++++++----------
.../test/Parser/OpenMP/metadirective-v50.f90 | 2 +-
.../llvm/Frontend/Directive/DirectiveBase.td | 6 ++++
llvm/include/llvm/Frontend/OpenMP/OMP.td | 8 ++++-
llvm/include/llvm/TableGen/DirectiveEmitter.h | 2 +-
11 files changed, 63 insertions(+), 59 deletions(-)
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 29797b7d088a7..6750127945b51 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -602,6 +602,7 @@ class ParseTreeDumper {
NODE(parser, OmpDefaultClause)
NODE_ENUM(OmpDefaultClause, DataSharingAttribute)
NODE(parser, OmpDefaultmapClause)
+ NODE(parser, OmpDefaultVariantClause)
NODE(OmpDefaultmapClause, Modifier)
NODE_ENUM(OmpDefaultmapClause, ImplicitBehavior)
NODE(parser, OmpDeleteModifier)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 283d7f4aa7c26..0862800601c29 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4523,17 +4523,9 @@ struct OmpApplyClause {
// data-sharing-attribute ->
// SHARED | NONE | // since 4.5
// PRIVATE | FIRSTPRIVATE // since 5.0
-//
-// When used in METADIRECTIVE:
-// default-clause ->
-// DEFAULT(directive-specification) // since 5.0, until 5.1
-// See also otherwise-clause.
struct OmpDefaultClause {
ENUM_CLASS(DataSharingAttribute, Private, Firstprivate, Shared, None)
- UNION_CLASS_BOILERPLATE(OmpDefaultClause);
- std::variant<DataSharingAttribute,
- common::Indirection<OmpDirectiveSpecification>>
- u;
+ WRAPPER_CLASS_BOILERPLATE(OmpDefaultClause, DataSharingAttribute);
};
// Ref: [4.5:103-107], [5.0:324-325], [5.1:357-358], [5.2:161-162]
@@ -4554,6 +4546,16 @@ struct OmpDefaultmapClause {
std::tuple<ImplicitBehavior, MODIFIERS()> t;
};
+// Ref: [5.0:56-57], [5.1:60-62]
+//
+// default-clause ->
+// DEFAULT(directive-specification) // since 5.0, until 5.1
+// See also otherwise-clause.
+struct OmpDefaultVariantClause {
+ WRAPPER_CLASS_BOILERPLATE(
+ OmpDefaultVariantClause, common::Indirection<OmpDirectiveSpecification>);
+};
+
// Ref: [4.5:169-172], [5.0:255-259], [5.1:288-292], [5.2:91-93]
//
// iteration-offset ->
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 96dcfa9ee9fbe..28a8f8f60fc48 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -654,8 +654,8 @@ Copyprivate make(const parser::OmpClause::Copyprivate &inp,
// has been superseded by the OTHERWISE clause.
// Disambiguate this in this representation: for the DSA case, create Default,
// and in the other case create Otherwise.
-Default makeDefault(const parser::OmpClause::Default &inp,
- semantics::SemanticsContext &semaCtx) {
+Default make(const parser::OmpClause::Default &inp,
+ semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpDefaultClause
using wrapped = parser::OmpDefaultClause;
@@ -669,11 +669,10 @@ Default makeDefault(const parser::OmpClause::Default &inp,
// clang-format on
);
- auto dsa = std::get<wrapped::DataSharingAttribute>(inp.v.u);
- return Default{/*DataSharingAttribute=*/convert(dsa)};
+ return Default{/*DataSharingAttribute=*/convert(inp.v.v)};
}
-Otherwise makeOtherwise(const parser::OmpClause::Default &inp,
+Otherwise makeOtherwise(const parser::OmpClause::DefaultVariant &inp,
semantics::SemanticsContext &semaCtx) {
return Otherwise{};
}
@@ -1806,18 +1805,9 @@ Clause makeClause(const parser::OmpClause &cls,
semantics::SemanticsContext &semaCtx) {
return Fortran::common::visit( //
common::visitors{
- [&](const parser::OmpClause::Default &s) {
- using DSA = parser::OmpDefaultClause::DataSharingAttribute;
- using ODS = common::Indirection<parser::OmpDirectiveSpecification>;
- if (std::holds_alternative<DSA>(s.v.u)) {
- return makeClause(llvm::omp::Clause::OMPC_default,
- clause::makeDefault(s, semaCtx), cls.source);
- } else if (std::holds_alternative<ODS>(s.v.u)) {
- return makeClause(llvm::omp::Clause::OMPC_otherwise,
- clause::makeOtherwise(s, semaCtx), cls.source);
- } else {
- llvm_unreachable("Unexpected alternative");
- }
+ [&](const parser::OmpClause::DefaultVariant &s) {
+ return makeClause(llvm::omp::Clause::OMPC_default_variant,
+ clause::makeOtherwise(s, semaCtx), cls.source);
},
[&](const parser::OmpClause::Depend &s) {
using TaskDep = parser::OmpDependClause::TaskDep;
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 9807023935784..5acdd8f01fa1c 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -6019,12 +6019,10 @@ static void genMetadirective(lower::AbstractConverter &converter,
std::get_if<parser::OmpClause::Otherwise>(&clause.u)) {
if (otherwiseClause->v && otherwiseClause->v->v)
fallback = getFallbackVariant(otherwiseClause->v->v->value());
- } else if (const auto *defaultClause =
- std::get_if<parser::OmpClause::Default>(&clause.u)) {
- if (const auto *dirSpecPtr = std::get_if<
- common::Indirection<parser::OmpDirectiveSpecification>>(
- &defaultClause->v.u))
- fallback = getFallbackVariant(dirSpecPtr->value());
+ } else if (const auto *defaultVariantClause =
+ std::get_if<parser::OmpClause::DefaultVariant>(&clause.u)) {
+ const auto &dirSpec = defaultVariantClause->v.v;
+ fallback = getFallbackVariant(dirSpec.value());
}
}
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index f960ad3454b2d..11d0aa9e1359b 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1215,10 +1215,10 @@ TYPE_PARSER(construct<OmpDefaultClause::DataSharingAttribute>(
"NONE" >> pure(OmpDefaultClause::DataSharingAttribute::None)))
TYPE_PARSER(construct<OmpDefaultClause>(
- construct<OmpDefaultClause>(
- Parser<OmpDefaultClause::DataSharingAttribute>{}) ||
- construct<OmpDefaultClause>(
- indirect(OmpDirectiveSpecificationParser(/*allowCommas=*/false)))))
+ Parser<OmpDefaultClause::DataSharingAttribute>{}))
+
+TYPE_PARSER(construct<OmpDefaultVariantClause>(
+ indirect(OmpDirectiveSpecificationParser(/*allowCommas=*/false))))
TYPE_PARSER(construct<OmpDynGroupprivateClause>(
maybe(nonemptyList(Parser<OmpDynGroupprivateClause::Modifier>{}) / ":"),
@@ -1521,8 +1521,12 @@ TYPE_PARSER( //
parenthesized(Parser<OmpObjectList>{}))) ||
"COPYPRIVATE" >> construct<OmpClause>(construct<OmpClause::Copyprivate>(
(parenthesized(Parser<OmpObjectList>{})))) ||
- "DEFAULT"_id >> construct<OmpClause>(construct<OmpClause::Default>(
- parenthesized(Parser<OmpDefaultClause>{}))) ||
+ "DEFAULT"_id >>
+ // Default or DefaultVariant depending on the argument.
+ (construct<OmpClause>(construct<OmpClause::Default>(
+ parenthesized(Parser<OmpDefaultClause>{}))) ||
+ construct<OmpClause>(construct<OmpClause::DefaultVariant>(
+ parenthesized(Parser<OmpDefaultVariantClause>{})))) ||
"DEFAULTMAP" >> construct<OmpClause>(construct<OmpClause::Defaultmap>(
parenthesized(Parser<OmpDefaultmapClause>{}))) ||
"DEPEND" >> construct<OmpClause>(construct<OmpClause::Depend>(
diff --git a/flang/lib/Semantics/check-omp-variant.cpp b/flang/lib/Semantics/check-omp-variant.cpp
index f49b2df9f3bfa..0f8daf84d25ec 100644
--- a/flang/lib/Semantics/check-omp-variant.cpp
+++ b/flang/lib/Semantics/check-omp-variant.cpp
@@ -54,8 +54,7 @@ bool HasDefaultNone(const parser::OmpDirectiveSpecification &spec) {
return false;
}
const auto &defaultClause{std::get<parser::OmpClause::Default>(clause->u)};
- const auto *dsa{std::get_if<DataSharingAttribute>(&defaultClause.v.u)};
- return dsa && *dsa == DataSharingAttribute::None;
+ return defaultClause.v.v == DataSharingAttribute::None;
}
bool HasNestedPrivateDSA(const Symbol &symbol, const Scope &scope) {
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 917d559ccbe50..8b550b1c1d70a 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2511,22 +2511,20 @@ void OmpAttributeVisitor::Post(const parser::OmpDefaultClause &x) {
// The DEFAULT clause may also be used on METADIRECTIVE. In that case
// there is nothing to do.
using DataSharingAttribute = parser::OmpDefaultClause::DataSharingAttribute;
- if (auto *dsa{std::get_if<DataSharingAttribute>(&x.u)}) {
- if (!dirContext_.empty()) {
- switch (*dsa) {
- case DataSharingAttribute::Private:
- SetContextDefaultDSA(Symbol::Flag::OmpPrivate);
- break;
- case DataSharingAttribute::Firstprivate:
- SetContextDefaultDSA(Symbol::Flag::OmpFirstPrivate);
- break;
- case DataSharingAttribute::Shared:
- SetContextDefaultDSA(Symbol::Flag::OmpShared);
- break;
- case DataSharingAttribute::None:
- SetContextDefaultDSA(Symbol::Flag::OmpNone);
- break;
- }
+ if (!dirContext_.empty()) {
+ switch (x.v) {
+ case DataSharingAttribute::Private:
+ SetContextDefaultDSA(Symbol::Flag::OmpPrivate);
+ break;
+ case DataSharingAttribute::Firstprivate:
+ SetContextDefaultDSA(Symbol::Flag::OmpFirstPrivate);
+ break;
+ case DataSharingAttribute::Shared:
+ SetContextDefaultDSA(Symbol::Flag::OmpShared);
+ break;
+ case DataSharingAttribute::None:
+ SetContextDefaultDSA(Symbol::Flag::OmpNone);
+ break;
}
}
}
diff --git a/flang/test/Parser/OpenMP/metadirective-v50.f90 b/flang/test/Parser/OpenMP/metadirective-v50.f90
index 6fef3376470a6..20670594382f8 100644
--- a/flang/test/Parser/OpenMP/metadirective-v50.f90
+++ b/flang/test/Parser/OpenMP/metadirective-v50.f90
@@ -26,6 +26,6 @@ subroutine f01
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpClauseList ->
-!PARSE-TREE: | OmpClause -> Default -> OmpDefaultClause -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpClause -> DefaultVariant -> OmpDefaultVariantClause -> OmpDirectiveSpecification
!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | OmpClauseList ->
diff --git a/llvm/include/llvm/Frontend/Directive/DirectiveBase.td b/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
index 56d5457481bc3..8c4fb1325400b 100644
--- a/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
+++ b/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
@@ -103,6 +103,9 @@ class Clause<list<Spelling> ss> {
// Spellings of the clause.
list<Spelling> spellings = ss;
+ // Optional name for the generated enum (if different from spelling).
+ string name = ?;
+
// Optional class holding value of the clause in clang AST.
string clangClass = "";
@@ -227,6 +230,9 @@ class Directive<list<Spelling> ss> {
// Spellings of the directive.
list<Spelling> spellings = ss;
+ // Optional name for the generated enum (if different from spelling).
+ string name = ?;
+
// Clauses cannot appear twice in the three allowed lists below. Also, since
// required implies allowed, the same clause cannot appear in both the
// allowedClauses and requiredClauses lists.
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index 6c616f55aacfb..dc2af1e62cfa4 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -147,6 +147,8 @@ def OMPC_Counts : Clause<[Spelling<"counts">]> {
let clangClass = "OMPCountsClause";
}
def OMPC_Default : Clause<[Spelling<"default">]> {
+ // This is for default DSA.
+ // For the "otherwise" clause, use OMPC_DefaultVariant.
let clangClass = "OMPDefaultClause";
let flangClass = "OmpDefaultClause";
}
@@ -154,6 +156,10 @@ def OMPC_DefaultMap : Clause<[Spelling<"defaultmap">]> {
let clangClass = "OMPDefaultmapClause";
let flangClass = "OmpDefaultmapClause";
}
+def OMPC_DefaultVariant : Clause<[Spelling<"default">]> {
+ let name = "default_variant";
+ let flangClass = "OmpDefaultVariantClause";
+}
def OMPC_Depend : Clause<[Spelling<"depend">]> {
let clangClass = "OMPDependClause";
let flangClass = "OmpDependClause";
@@ -1056,7 +1062,7 @@ def OMP_Metadirective : Directive<[Spelling<"metadirective">]> {
VersionedClause<OMPC_When>,
];
let allowedOnceClauses = [
- VersionedClause<OMPC_Default, 50, 51>,
+ VersionedClause<OMPC_DefaultVariant, 50, 51>,
VersionedClause<OMPC_Otherwise, 52>,
];
let association = AS_None;
diff --git a/llvm/include/llvm/TableGen/DirectiveEmitter.h b/llvm/include/llvm/TableGen/DirectiveEmitter.h
index d111f666ff31a..00b2cec453aef 100644
--- a/llvm/include/llvm/TableGen/DirectiveEmitter.h
+++ b/llvm/include/llvm/TableGen/DirectiveEmitter.h
@@ -308,7 +308,7 @@ class Clause : public BaseRecord {
// ex: async -> Async
// num_threads -> NumThreads
std::string getFormattedParserClassName() const {
- StringRef Name = getSpellingForIdentifier();
+ std::string Name = getFormattedName();
return BaseRecord::getUpperCamelName(Name, "_");
}
>From 9dab3510397947d5041808695bec1c8c41058725 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Sun, 26 Jul 2026 10:53:57 -0500
Subject: [PATCH 2/4] format
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 5acdd8f01fa1c..b4fa431d42223 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -6022,7 +6022,7 @@ static void genMetadirective(lower::AbstractConverter &converter,
} else if (const auto *defaultVariantClause =
std::get_if<parser::OmpClause::DefaultVariant>(&clause.u)) {
const auto &dirSpec = defaultVariantClause->v.v;
- fallback = getFallbackVariant(dirSpec.value());
+ fallback = getFallbackVariant(dirSpec.value());
}
}
>From a9a73232f4350dba0de92a5ac32f1348a3cd0976 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 27 Jul 2026 08:54:13 -0500
Subject: [PATCH 3/4] Update comment
---
flang/lib/Lower/OpenMP/Clauses.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 28a8f8f60fc48..f8cbbd17a55b9 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -649,11 +649,9 @@ Copyprivate make(const parser::OmpClause::Copyprivate &inp,
return Copyprivate{/*List=*/makeObjects(inp.v, semaCtx)};
}
-// The Default clause is overloaded in OpenMP 5.0 and 5.1: it can be either
-// a data-sharing clause, or a METADIRECTIVE clause. In the latter case, it
-// has been superseded by the OTHERWISE clause.
-// Disambiguate this in this representation: for the DSA case, create Default,
-// and in the other case create Otherwise.
+// The DEFAULT clause is OpenMP 5.0 and 5.1 that represents the default
+// directive variant in METADIRECTIVE is represented by the DefaultVariant
+// class.
Default make(const parser::OmpClause::Default &inp,
semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpDefaultClause
@@ -672,6 +670,8 @@ Default make(const parser::OmpClause::Default &inp,
return Default{/*DataSharingAttribute=*/convert(inp.v.v)};
}
+// Lower the DefaultVariant (specific to OpenMP 5.0 and 5.1) directly to
+// OTHERWISE (which replaced it since 5.2).
Otherwise makeOtherwise(const parser::OmpClause::DefaultVariant &inp,
semantics::SemanticsContext &semaCtx) {
return Otherwise{};
>From 1ef1bb2ca38f5927c28cd7e9ad4cc5913f59a0a1 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 27 Jul 2026 08:54:31 -0500
Subject: [PATCH 4/4] Add comment explaning name conflicts
---
llvm/utils/TableGen/Basic/DirectiveEmitter.cpp | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp b/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
index 93d83f909bfd2..390cb37cb3c94 100644
--- a/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
@@ -460,6 +460,15 @@ static void generateGetKind(ArrayRef<const Record *> Records, raw_ostream &OS,
directive::VersionRange All;
+ // When a given spelling maps to more than one enum kind, this function
+ // will return one of them, but it's unspecified which one.
+ // This can happen whem a directive/clause uses the same spelling as
+ // another directive/clause, e.g. when it varies depending on the version:
+ // OMPC_foo : {"foo", v1.0}, {"bar", v2.0}
+ // OMPC_bar : {"bar", v1.0}, {"baz", v2.0}
+ // or when the same spelling can be used to mean different things:
+ // OMPC_do_one_thing : {"doit"}
+ // OMPC_do_something_else : {"doit"}
for (const Record *R : Records) {
BaseRecord Rec(R);
std::string Ident = ImplicitAsUnknown && R->getValueAsBit("isImplicit")
More information about the flang-commits
mailing list