[flang-commits] [flang] e5a5c5d - [flang][OpenMP] Add semantic checks for two DECLARE VARIANT restrictions (#209528)
via flang-commits
flang-commits at lists.llvm.org
Tue Jul 21 07:12:10 PDT 2026
Author: Abid Qadeer
Date: 2026-07-21T15:12:05+01:00
New Revision: e5a5c5dfdb4ac6b004ab31df25c30f4cc8772afe
URL: https://github.com/llvm/llvm-project/commit/e5a5c5dfdb4ac6b004ab31df25c30f4cc8772afe
DIFF: https://github.com/llvm/llvm-project/commit/e5a5c5dfdb4ac6b004ab31df25c30f4cc8772afe.diff
LOG: [flang][OpenMP] Add semantic checks for two DECLARE VARIANT restrictions (#209528)
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
Added:
flang/test/Semantics/OpenMP/declare-variant-restriction.f90
Modified:
flang/lib/Semantics/check-omp-structure.h
flang/lib/Semantics/check-omp-variant.cpp
Removed:
################################################################################
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 7248ca79f061e..c0d82615805d8 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 =
@@ -467,6 +468,17 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
int directiveNest_[LastType + 1] = {0};
std::set<std::pair<const Symbol *, const Symbol *>> declareVariantPairs_;
+ // For each variant procedure: its construct-selector-set as an ordered list
+ // of elements (a leaf construct directive plus a normalized rendering of any
+ // properties), and the source of the DECLARE VARIANT directive that first
+ // established the set.
+ std::map<const Symbol *,
+ std::pair<
+ llvm::SmallVector<std::pair<llvm::omp::Directive, std::string>, 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 994a49163914d..19ed3cd0e114c 100644
--- a/flang/lib/Semantics/check-omp-variant.cpp
+++ b/flang/lib/Semantics/check-omp-variant.cpp
@@ -26,8 +26,10 @@
#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/Frontend/OpenMP/OMP.h"
+#include <algorithm>
#include <list>
#include <map>
#include <optional>
@@ -853,6 +855,81 @@ static bool CheckVariantAccessibility(SemanticsContext &context,
return false;
}
+// Normalize the text spanned by `source` for structural comparison: drop
+// whitespace and fold to lower case (Fortran is case-insensitive). This lets
+// e.g. `simdlen(4)` and `SIMDLEN( 4 )` compare equal while `simdlen(4)` and
+// `simdlen(8)` do not.
+static std::string NormalizeSelectorText(parser::CharBlock source) {
+ std::string text{source.ToString()};
+ llvm::erase_if(text, parser::IsWhiteSpace);
+ return parser::ToLowerCaseLetters(text);
+}
+
+// Render the properties of a trait selector into a canonical string (in a
+// construct selector set only `simd` currently carries properties, e.g.
+// simdlen, inbranch/notinbranch). The property list is unordered, so the
+// individual properties are sorted; two selectors have the same properties iff
+// these strings are equal.
+static std::string CanonicalizeTraitProperties(
+ const parser::OmpTraitSelector &trait) {
+ const auto &maybeProperties{
+ std::get<std::optional<parser::OmpTraitSelector::Properties>>(trait.t)};
+ if (!maybeProperties) {
+ return {};
+ }
+ llvm::SmallVector<std::string> items;
+ for (const parser::OmpTraitProperty &property :
+ std::get<std::list<parser::OmpTraitProperty>>(maybeProperties->t)) {
+ items.push_back(NormalizeSelectorText(property.source));
+ }
+ std::sort(items.begin(), items.end());
+ std::string result;
+ for (const std::string &item : items) {
+ result += item;
+ result += ';';
+ }
+ return result;
+}
+
+// Collect the construct selector set of `sel` into `constructs` as an ordered
+// list of elements: a leaf construct directive plus a normalized rendering of
+// any properties it carries. Combined and composite constructs are decomposed
+// into their leaves (e.g. `target teams` -> target, teams). The properties
+// are included so that e.g. `simd(simdlen(4))` and `simd(simdlen(8))` are
+// treated as
diff erent construct selector sets. 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<std::pair<llvm::omp::Directive, std::string>>
+ &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.emplace_back(leaf, std::string{});
+ }
+ } 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 and fold in its properties so they participate in the
+ // comparison.
+ constructs.emplace_back(llvm::omp::Directive::OMPD_simd,
+ CanonicalizeTraitProperties(trait));
+ }
+ }
+ }
+}
+
void OmpStructureChecker::CheckOmpDeclareVariantDirective(
const parser::OmpDeclareVariantDirective &x) {
const parser::OmpDirectiveSpecification &spec{x.v};
@@ -940,23 +1017,65 @@ void OmpStructureChecker::CheckOmpDeclareVariantDirective(
if (base == variant) {
context_.Say(arg.source,
"The variant procedure must
diff er 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
diff ers 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<std::pair<llvm::omp::Directive, std::string>, 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
diff ers
+ // 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..d3e67e2dc123c
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-variant-restriction.f90
@@ -0,0 +1,184 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=51
+
+! Same variant (p3) used with
diff erent '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
+
+! 'simd' (represented specially in the parse tree) appears in only one of the
+! construct selector sets, so the sets
diff er: 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
+
+! 'simd' appears in both sets but with
diff erent simd properties (simdlen), so
+! the construct selector sets
diff er: not conforming.
+subroutine r1_simd_simdlen_
diff
+ !$omp declare variant (p1:p3) match (construct={simd(simdlen(4))})
+ !ERROR: Variant procedure 'p3' must have the same 'construct' selector set in all DECLARE VARIANT directives
+ !$omp declare variant (p2:p3) match (construct={simd(simdlen(8))})
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ end subroutine
+ subroutine p3
+ end subroutine
+end subroutine
+
+! 'simd' with identical simd properties in both sets: allowed.
+subroutine r1_simd_simdlen_same
+ !$omp declare variant (p1:p3) match (construct={simd(simdlen(4))})
+ !$omp declare variant (p2:p3) match (construct={simd(simdlen(4))})
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ end subroutine
+ subroutine p3
+ end subroutine
+end subroutine
+
+! Bare 'simd' versus 'simd' with a property: the sets
diff er.
+subroutine r1_simd_bare_vs_prop
+ !$omp declare variant (p1:p3) match (construct={simd})
+ !ERROR: Variant procedure 'p3' must have the same 'construct' selector set in all DECLARE VARIANT directives
+ !$omp declare variant (p2:p3) match (construct={simd(simdlen(4))})
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ end subroutine
+ subroutine p3
+ end subroutine
+end subroutine
+
+! 'simd' with inbranch versus notinbranch: the sets
diff er.
+subroutine r1_simd_inbranch_
diff
+ !$omp declare variant (p1:p3) match (construct={simd(inbranch)})
+ !ERROR: Variant procedure 'p3' must have the same 'construct' selector set in all DECLARE VARIANT directives
+ !$omp declare variant (p2:p3) match (construct={simd(notinbranch)})
+contains
+ subroutine p1
+ end subroutine
+ subroutine p2
+ end subroutine
+ subroutine p3
+ end subroutine
+end subroutine
+
+! The same simd properties written in a
diff erent order describe the same set:
+! allowed (property order is not significant).
+subroutine r1_simd_prop_reorder
+ !$omp declare variant (p1:p3) match (construct={simd(simdlen(4), inbranch)})
+ !$omp declare variant (p2:p3) match (construct={simd(inbranch, simdlen(4))})
+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
More information about the flang-commits
mailing list