[flang-commits] [flang] [flang][OpenMP] Add semantic checks for two DECLARE VARIANT restrictions (PR #209528)
Abid Qadeer via flang-commits
flang-commits at lists.llvm.org
Wed Jul 15 04:18:54 PDT 2026
https://github.com/abidh updated https://github.com/llvm/llvm-project/pull/209528
>From 5cacaf963106f2d143ffa1012387b5148335a8d5 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 14 Jul 2026 16:19:09 +0100
Subject: [PATCH 1/2] [flang][OpenMP] Add semantic checks for two DECLARE
VARIANT restrictions
Diagnose two DECLARE VARIANT restrictions from the OpenMP specification
(5.2 [7.5], 6.0 [9.6]) that were previously accepted without error:
- If a procedure is determined to be a function variant through more than
one DECLARE VARIANT directive, the construct selector set of their
context selectors must be the same.
- A procedure determined to be a function variant may not be specified as
a base function in another DECLARE VARIANT directive.
Assisted-by: Cursor
---
flang/lib/Semantics/check-omp-structure.h | 6 ++
flang/lib/Semantics/check-omp-variant.cpp | 100 +++++++++++++++---
.../OpenMP/declare-variant-restriction.f90 | 86 +++++++++++++++
3 files changed, 176 insertions(+), 16 deletions(-)
create mode 100644 flang/test/Semantics/OpenMP/declare-variant-restriction.f90
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 3d4cdd08d07ca..3f1a561d59c54 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -19,6 +19,7 @@
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/openmp-directive-sets.h"
#include "flang/Semantics/semantics.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/iterator_range.h"
using OmpClauseSet =
@@ -458,6 +459,11 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
int directiveNest_[LastType + 1] = {0};
std::set<std::pair<const Symbol *, const Symbol *>> declareVariantPairs_;
+ std::map<const Symbol *,
+ std::pair<llvm::SmallVector<llvm::omp::Directive, 4>, parser::CharBlock>>
+ declareVariantConstructSets_;
+ // Tracks the procedures that appear as a base in DECLARE VARIANT.
+ std::set<const Symbol *> declareVariantBases_;
int allocateDirectiveLevel_{0};
parser::CharBlock visitedAtomicSource_;
diff --git a/flang/lib/Semantics/check-omp-variant.cpp b/flang/lib/Semantics/check-omp-variant.cpp
index aeb5f1046d589..a72b01689cd17 100644
--- a/flang/lib/Semantics/check-omp-variant.cpp
+++ b/flang/lib/Semantics/check-omp-variant.cpp
@@ -878,6 +878,33 @@ static bool CheckVariantAccessibility(SemanticsContext &context,
return false;
}
+// Collect the construct selector set of `sel` into `constructs` as an ordered
+// list of leaf construct directives. Combined and composite constructs are
+// decomposed into their leaves (e.g. `target teams` -> target, teams), so two
+// match selectors have the same construct selector set iff the collected lists
+// are equal. An absent construct selector yields an empty list.
+static void CollectConstructSelectorSet(
+ const parser::traits::OmpContextSelectorSpecification &sel,
+ llvm::SmallVectorImpl<llvm::omp::Directive> &constructs) {
+ using SetName = parser::OmpTraitSetSelectorName;
+ using TraitName = parser::OmpTraitSelectorName;
+ for (const parser::OmpTraitSetSelector &traitSet : sel.v) {
+ if (std::get<SetName>(traitSet.t).v != SetName::Value::Construct) {
+ continue;
+ }
+ for (const parser::OmpTraitSelector &trait :
+ std::get<std::list<parser::OmpTraitSelector>>(traitSet.t)) {
+ const auto &traitName{std::get<TraitName>(trait.t)};
+ if (const auto *dir{std::get_if<llvm::omp::Directive>(&traitName.u)}) {
+ for (llvm::omp::Directive leaf :
+ llvm::omp::getLeafConstructsOrSelf(*dir)) {
+ constructs.push_back(leaf);
+ }
+ }
+ }
+ }
+}
+
void OmpStructureChecker::CheckOmpDeclareVariantDirective(
const parser::OmpDeclareVariantDirective &x) {
const parser::OmpDirectiveSpecification &spec{x.v};
@@ -965,23 +992,64 @@ void OmpStructureChecker::CheckOmpDeclareVariantDirective(
if (base == variant) {
context_.Say(arg.source,
"The variant procedure must differ from the base procedure"_err_en_US);
- } else if (!declareVariantPairs_.emplace(base, variant).second) {
- context_.Say(arg.source,
- "Variant '%s' was already specified for '%s' in another DECLARE VARIANT directive"_err_en_US,
- variant->name(), base->name());
- } else if (CheckVariantAccessibility(
- context_, *base, *variant, arg.source)) {
- if (!hasArgModifiers) {
- // adjust_args/append_args perform the "transformation for its OpenMP
- // context", so the variant interface intentionally differs from the
- // base; skip the same-interface check until they are supported.
- CheckDeclareVariantInterface(context_, *base, *variant, arg.source);
+ } else {
+ // OpenMP 5.2 [7.5] / 6.0 [9.6]: a procedure determined to be a function
+ // variant may not be a base function in another DECLARE VARIANT
+ // directive, and vice versa. A variant is recorded as a key in
+ // declareVariantConstructSets_ below, so that map serves as the set of
+ // procedures that appear as a variant.
+ if (declareVariantConstructSets_.find(base) !=
+ declareVariantConstructSets_.end()) {
+ context_.Say(arg.source,
+ "The base procedure '%s' is also specified as a variant procedure in another DECLARE VARIANT directive"_err_en_US,
+ base->name());
+ }
+ if (declareVariantBases_.find(variant) != declareVariantBases_.end()) {
+ context_.Say(arg.source,
+ "The variant procedure '%s' is also specified as a base procedure in another DECLARE VARIANT directive"_err_en_US,
+ variant->name());
}
- // Record the variant on its base procedure.
- if (base->has<SubprogramDetails>()) {
- auto &details{const_cast<Symbol &>(*base).get<SubprogramDetails>()};
- details.addOmpDeclareVariant(
- OmpDeclareVariantEntry{*variant, matchSelector});
+ declareVariantBases_.insert(base);
+
+ if (!declareVariantPairs_.emplace(base, variant).second) {
+ context_.Say(arg.source,
+ "Variant '%s' was already specified for '%s' in another DECLARE VARIANT directive"_err_en_US,
+ variant->name(), base->name());
+ } else {
+ // OpenMP 5.2 [7.5] / 6.0 [9.6]: if a procedure is determined to be a
+ // function variant through more than one DECLARE VARIANT directive, the
+ // construct selector set of their context selectors must be the same.
+ llvm::SmallVector<llvm::omp::Directive, 4> constructs;
+ CollectConstructSelectorSet(*matchSelector, constructs);
+ auto it{declareVariantConstructSets_.find(variant)};
+ if (it == declareVariantConstructSets_.end()) {
+ declareVariantConstructSets_.emplace(
+ variant, std::make_pair(std::move(constructs), arg.source));
+ } else if (it->second.first != constructs) {
+ context_
+ .Say(arg.source,
+ "Variant procedure '%s' must have the same 'construct' selector set in all DECLARE VARIANT directives"_err_en_US,
+ variant->name())
+ .Attach(it->second.second,
+ "Previous DECLARE VARIANT directive for '%s'"_en_US,
+ variant->name());
+ }
+
+ if (CheckVariantAccessibility(context_, *base, *variant, arg.source)) {
+ if (!hasArgModifiers) {
+ // adjust_args/append_args perform the "transformation for its
+ // OpenMP context", so the variant interface intentionally differs
+ // from the base; skip the same-interface check until they are
+ // supported.
+ CheckDeclareVariantInterface(context_, *base, *variant, arg.source);
+ }
+ // Record the variant on its base procedure.
+ if (base->has<SubprogramDetails>()) {
+ auto &details{const_cast<Symbol &>(*base).get<SubprogramDetails>()};
+ details.addOmpDeclareVariant(
+ OmpDeclareVariantEntry{*variant, matchSelector});
+ }
+ }
}
}
}
diff --git a/flang/test/Semantics/OpenMP/declare-variant-restriction.f90 b/flang/test/Semantics/OpenMP/declare-variant-restriction.f90
new file mode 100644
index 0000000000000..a565f3a97abd2
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-variant-restriction.f90
@@ -0,0 +1,86 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=51
+
+! Same variant (p3) used with different 'construct' selector sets: not conforming.
+subroutine r1_diff_sets
+ !$omp declare variant (p1:p3) match (construct={parallel})
+ !ERROR: Variant procedure 'p3' must have the same 'construct' selector set in all DECLARE VARIANT directives
+ !$omp declare variant (p2:p3) match (construct={do})
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ end subroutine
+ subroutine p3
+ end subroutine
+end subroutine
+
+! Same variant (p3) used with identical 'construct' selector sets: allowed.
+subroutine r1_same_sets
+ !$omp declare variant (p1:p3) match (construct={parallel})
+ !$omp declare variant (p2:p3) match (construct={parallel})
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ end subroutine
+ subroutine p3
+ end subroutine
+end subroutine
+
+! A procedure (p2) that is a variant may not later be used as a base function.
+subroutine r3_variant_then_base
+ !$omp declare variant (p1:p2) match (construct={parallel})
+ !ERROR: The base procedure 'p2' is also specified as a variant procedure in another DECLARE VARIANT directive
+ !$omp declare variant (p2:p3) match (construct={parallel})
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ end subroutine
+ subroutine p3
+ end subroutine
+end subroutine
+
+! The same conflict is detected when the base use appears first: p1 is a base,
+! then used as a variant.
+subroutine r3_base_then_variant
+ !$omp declare variant (p1:p2) match (construct={parallel})
+ !ERROR: The variant procedure 'p1' is also specified as a base procedure in another DECLARE VARIANT directive
+ !$omp declare variant (p3:p1) match (construct={parallel})
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ end subroutine
+ subroutine p3
+ end subroutine
+end subroutine
+
+! Cross-function usage
+module m_cross_function
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ !$omp declare variant (p1) match (user={condition(.false.)},construct={do})
+ end subroutine
+ subroutine p3
+ !ERROR: Variant procedure 'p1' must have the same 'construct' selector set in all DECLARE VARIANT directives
+ !$omp declare variant (p1) match (user={condition(.true.)})
+ end subroutine
+end module
+
+! Two directives for the same variant (p1) that both omit the 'construct'
+! selector have the same (empty) construct selector set: allowed. Non-construct
+! selectors (here 'user') do not affect the comparison.
+module m_no_construct_ok
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ !$omp declare variant (p1) match (user={condition(.true.)})
+ end subroutine
+ subroutine p3
+ !$omp declare variant (p1) match (user={condition(.false.)})
+ end subroutine
+end module
>From c05b1eb88b06e2bee76f35cce02c3f0a617063bf Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Wed, 15 Jul 2026 12:13:46 +0100
Subject: [PATCH 2/2] Handle review comments.
The simd needs sepaate handling.
---
flang/lib/Semantics/check-omp-variant.cpp | 6 ++++
.../OpenMP/declare-variant-restriction.f90 | 28 +++++++++++++++++++
2 files changed, 34 insertions(+)
diff --git a/flang/lib/Semantics/check-omp-variant.cpp b/flang/lib/Semantics/check-omp-variant.cpp
index a72b01689cd17..228d439d40433 100644
--- a/flang/lib/Semantics/check-omp-variant.cpp
+++ b/flang/lib/Semantics/check-omp-variant.cpp
@@ -900,6 +900,12 @@ static void CollectConstructSelectorSet(
llvm::omp::getLeafConstructsOrSelf(*dir)) {
constructs.push_back(leaf);
}
+ } else if (const auto *value{std::get_if<TraitName::Value>(&traitName.u)};
+ value && *value == TraitName::Value::Simd) {
+ // In a construct selector, `simd` is represented as Value::Simd (it can
+ // carry simd-specific properties), not as a Directive; treat it as
+ // OMPD_simd so it participates in the comparison.
+ constructs.push_back(llvm::omp::Directive::OMPD_simd);
}
}
}
diff --git a/flang/test/Semantics/OpenMP/declare-variant-restriction.f90 b/flang/test/Semantics/OpenMP/declare-variant-restriction.f90
index a565f3a97abd2..704271f087dcf 100644
--- a/flang/test/Semantics/OpenMP/declare-variant-restriction.f90
+++ b/flang/test/Semantics/OpenMP/declare-variant-restriction.f90
@@ -27,6 +27,34 @@ subroutine p3
end subroutine
end subroutine
+! 'simd' (represented specially in the parse tree) appears in only one of the
+! construct selector sets, so the sets differ: not conforming.
+subroutine r1_simd_diff
+ !$omp declare variant (p1:p3) match (construct={parallel})
+ !ERROR: Variant procedure 'p3' must have the same 'construct' selector set in all DECLARE VARIANT directives
+ !$omp declare variant (p2:p3) match (construct={parallel, simd})
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ end subroutine
+ subroutine p3
+ end subroutine
+end subroutine
+
+! 'simd' appears in both construct selector sets: the sets are the same.
+subroutine r1_simd_same
+ !$omp declare variant (p1:p3) match (construct={parallel, simd})
+ !$omp declare variant (p2:p3) match (construct={parallel, simd})
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ end subroutine
+ subroutine p3
+ end subroutine
+end subroutine
+
! A procedure (p2) that is a variant may not later be used as a base function.
subroutine r3_variant_then_base
!$omp declare variant (p1:p2) match (construct={parallel})
More information about the flang-commits
mailing list