https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/229526
>From 57f7dee9ffb8f32d051b115afbf2a976e84854c3 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <[email protected]> Date: Thu, 1 Oct 2026 13:41:06 -0500 Subject: [PATCH] [flang][OpenMP] Improve diagnostics about modifier properties Use OpenMPDeprecated and OpenMPFuture warning categories for modifier diagnostics as well, analogously to how they are used for clauses. --- flang/lib/Semantics/check-omp-syntax.cpp | 96 +++++++++++++------ flang/test/Semantics/OpenMP/allocate_do1.f90 | 4 +- .../Semantics/OpenMP/clause-validity01.f90 | 2 +- flang/test/Semantics/OpenMP/depend05.f90 | 2 +- .../Semantics/OpenMP/depobj-construct-v52.f90 | 2 +- .../test/Semantics/OpenMP/from-clause-v45.f90 | 6 +- .../test/Semantics/OpenMP/linear-clause02.f90 | 2 +- .../Semantics/OpenMP/map-modifiers-v60.f90 | 8 +- flang/test/Semantics/OpenMP/task-depend.f90 | 2 +- flang/test/Semantics/OpenMP/to-clause-v45.f90 | 6 +- .../OpenMP/uses-allocators-version50.f90 | 6 +- .../OpenMP/uses-allocators-version51.f90 | 4 +- 12 files changed, 88 insertions(+), 52 deletions(-) diff --git a/flang/lib/Semantics/check-omp-syntax.cpp b/flang/lib/Semantics/check-omp-syntax.cpp index 429c4e74d2acd0..37585d3a0e5389 100644 --- a/flang/lib/Semantics/check-omp-syntax.cpp +++ b/flang/lib/Semantics/check-omp-syntax.cpp @@ -19,6 +19,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SetVector.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/Frontend/Directive/Spelling.h" #include "llvm/Frontend/OpenMP/OMP.h" #include "llvm/Frontend/OpenMP/OMPDescriptors.h" @@ -52,6 +53,42 @@ static llvm::omp::Version GetClosestVersion( return llvm::omp::Version(); } +static llvm::omp::Version NextVersion(llvm::omp::Version version) { + auto versions{llvm::omp::getOpenMPVersions()}; + for (auto [idx, ver] : llvm::enumerate(versions)) { + if (ver == version && idx + 1 < versions.size()) { + return versions[idx + 1]; + } + } + return llvm::omp::Version(); +} + +static std::string EnumSetToString( + llvm::omp::Modifiers set, llvm::omp::Version version) { + llvm::SmallVector<std::string> names; + for (llvm::omp::Modifier m : set) { + names.emplace_back(llvm::omp::getDescriptor(m).getName().str()); + } + if (names.size() == 1) { + return names.front(); + } + return llvm::join(llvm::ArrayRef(names).drop_back(), ", ") + " or " + + names.back(); +} + +static std::string OneOfModifiers( + llvm::omp::ModifierSet set, llvm::omp::Version version) { + auto &sdesc{llvm::omp::getDescriptor(set)}; + llvm::omp::Modifiers members{sdesc.getModifiers(version)}; + + if (size_t count{members.size()}; count == 1) { + return EnumSetToString(members, version) + " modifier"; + } else if (count > 1) { + return "One of " + EnumSetToString(members, version) + " modifiers"; + } + return ""; +} + template < // typename ElemTy, typename SetsSetTy, typename OwnerTy, typename ResultTy = llvm::DenseMap<ElemTy, @@ -179,13 +216,13 @@ static ResultTy VerifyExclusive( template < // typename ElemTy, typename SetsSetTy, typename OwnerTy, + typename SetTy = typename SetsSetTy::value_type, typename ResultTy = llvm::DenseMap<ElemTy, - std::tuple<ElemTy, parser::CharBlock, parser::CharBlock>>> + std::tuple<ElemTy, SetTy, parser::CharBlock, parser::CharBlock>>> static ResultTy VerifyMutuallyExclusive( const AppliedElementInfo<ElemTy, SetsSetTy> &info, OwnerTy ownerId, llvm::omp::Version version) { using AppliedElementTy = AppliedElement<ElemTy, SetsSetTy>; - using SetTy = typename SetsSetTy::value_type; ResultTy result; @@ -235,7 +272,7 @@ static ResultTy VerifyMutuallyExclusive( const AppliedElementTy *prev{where->second}; if (prev->id.value != elem.id.value) { result.insert({elem.id.value, - {prev->id.value, elem.id.source, prev->id.source}}); + {prev->id.value, s, elem.id.source, prev->id.source}}); // Stop version traversal. break; } @@ -294,25 +331,27 @@ bool OmpStructureChecker::VerifyModifierVersion( auto result = VerifyVersions(info, clause.value, version); for (auto &[m, svr] : result) { - std::string modName{llvm::omp::getDescriptor(m).getName().str()}; + std::string modName{llvm::omp::getDescriptor(m).getName()}; std::string clauseName{GetUpperName(clause.value, version)}; llvm::omp::Version since(svr.second.Min); llvm::omp::Version until(svr.second.Max); if (since == ~0u && until == 0u) { - // This shouldn't really happen, but have it just in case. + // This shouldn't really happen because the set of allowed modifiers + // is specified in the AST node for the clause, but have this check + // just to cover all bases. context_.Say(svr.first, - "'%s' modifier is not supported on %s clause"_err_en_US, modName, + "'%s' modifier is not allowed on %s clause"_err_en_US, modName, clauseName); } else if (since != ~0u && version < since) { - context_.Say(svr.first, - "'%s' modifier is not supported on %s clause in %s, %s"_warn_en_US, + context_.Warn(common::UsageWarning::OpenMPFuture, svr.first, + "'%s' modifier is not allowed on %s clause in %s, %s"_warn_en_US, modName, clauseName, omp::ThisVersion(version), omp::TryVersion(since)); } else if (until != 0u && version > until) { - context_.Say(svr.first, - "'%s' modifier is no longer supported on %s clause in %s"_warn_en_US, - modName, clauseName, omp::ThisVersion(version)); + context_.Warn(common::UsageWarning::OpenMPDeprecated, svr.first, + "'%s' modifier is no longer allowed on %s clause since %s"_warn_en_US, + modName, clauseName, omp::ThisVersion(NextVersion(until))); } } @@ -331,18 +370,8 @@ bool OmpStructureChecker::VerifyModifierRequired( clause.source, "'%s' modifier is required"_err_en_US, mdesc.getName()); } for (llvm::omp::ModifierSet s : result.second) { - auto &sdesc{llvm::omp::getDescriptor(s)}; - // If the group is required, at least one modifier from that group must - // be present. - if (llvm::omp::isModifierGroup(s)) { - context_.Say(clause.source, - "modifier from '%s' modifier group is required"_err_en_US, - sdesc.getName()); - } else { - context_.Say(clause.source, - "modifier from the modifier set on %s clause is required"_err_en_US, - GetUpperName(clause.value, version)); - } + context_.Say(clause.source, "%s is required on %s clause"_err_en_US, + OneOfModifiers(s, version), GetUpperName(clause.value, version)); } return result.first.empty() && result.second.empty(); @@ -357,9 +386,10 @@ bool OmpStructureChecker::VerifyModifierUnique( for (auto [id, where] : result) { auto &mdesc{llvm::omp::getDescriptor(id)}; context_ - .Say(where.first, "'%s' modifier cannot occur multiple times"_err_en_US, + .Say(where.second, + "'%s' modifier cannot occur multiple times"_err_en_US, mdesc.getName()) - .Attach(where.second, "previous occurrence of this modifier"_en_US); + .Attach(where.first, "previous occurrence of this modifier"_en_US); } return result.empty(); @@ -375,7 +405,7 @@ bool OmpStructureChecker::VerifyModifierExclusive( auto [otherId, source, otherSource] = wrong; context_ .Say(source, - "An exclusive '%s' modifier cannot be specified together with a modifier of a different type"_err_en_US, + "'%s' modifier cannot be specified together with a modifier of a different type"_err_en_US, llvm::omp::getDescriptor(id).getName()) .Attach(otherSource, "'%s' provided here"_en_US, llvm::omp::getDescriptor(otherId).getName()); @@ -384,12 +414,17 @@ bool OmpStructureChecker::VerifyModifierExclusive( auto resultMut = VerifyMutuallyExclusive(info, clause.value, version); for (auto [id, wrong] : resultMut) { - auto [otherId, source, otherSource] = wrong; - auto thisName{llvm::omp::getDescriptor(id).getName().str()}; + auto [otherId, setId, source, otherSource] = wrong; + auto thisName{llvm::omp::getDescriptor(id).getName()}; + std::string annot; + if (llvm::omp::isModifierGroup(setId)) { + auto &sdesc{llvm::omp::getDescriptor(setId)}; + annot = " as members of '" + sdesc.getName().str() + "' modifier group"; + } context_ .Say(otherSource, - "The '%s' and '%s' modifiers are mutually exclusive"_err_en_US, - llvm::omp::getDescriptor(otherId).getName(), thisName) + "'%s' and '%s' modifiers are mutually exclusive%s"_err_en_US, + llvm::omp::getDescriptor(otherId).getName(), thisName, annot) .Attach(source, "'%s' modifier specified here"_en_US, thisName); } @@ -427,6 +462,7 @@ template <typename UnionTy> AppliedModifierInfo GetAppliedModifiers(llvm::omp::Clause clauseId, llvm::omp::Version version, const std::optional<std::list<UnionTy>> &modifiers) { + using AppliedModifier = AppliedModifierInfo::ElementTy; AppliedModifierInfo info; if (modifiers) { auto cdesc{llvm::omp::getDescriptor(clauseId)}; diff --git a/flang/test/Semantics/OpenMP/allocate_do1.f90 b/flang/test/Semantics/OpenMP/allocate_do1.f90 index 6f5e4355af86c6..0fd6f9d0f30727 100644 --- a/flang/test/Semantics/OpenMP/allocate_do1.f90 +++ b/flang/test/Semantics/OpenMP/allocate_do1.f90 @@ -22,7 +22,7 @@ subroutine test_omp_do() use all_mod !$omp parallel shared(AAA,BBB,CCC,DDD,val) !ERROR: ALLOCATE clause is not allowed on DO directive in OpenMP v4.5, try -fopenmp-version=50 [-Wopenmp-future] - !ERROR: 'allocator-simple-modifier' modifier is not supported on ALLOCATE clause in OpenMP v4.5, try -fopenmp-version=50 + !ERROR: 'allocator-simple-modifier' modifier is not allowed on ALLOCATE clause in OpenMP v4.5, try -fopenmp-version=50 [-Wopenmp-future] !$omp do private(CCC, val) allocate(0:CCC, val) do i=1,N CCC(i) = AAA(i) + BBB(i) @@ -36,7 +36,7 @@ end subroutine test_omp_do subroutine test_omp_parallel_do() use all_mod !ERROR: ALLOCATE clause is not allowed on PARALLEL DO directive in OpenMP v4.5, try -fopenmp-version=50 [-Wopenmp-future] - !ERROR: 'allocator-simple-modifier' modifier is not supported on ALLOCATE clause in OpenMP v4.5, try -fopenmp-version=50 + !ERROR: 'allocator-simple-modifier' modifier is not allowed on ALLOCATE clause in OpenMP v4.5, try -fopenmp-version=50 [-Wopenmp-future] !$omp parallel do private(CCC, val) allocate(0:CCC, val) shared(AAA,BBB,DDD) do i=1,N CCC(i) = AAA(i) + BBB(i) diff --git a/flang/test/Semantics/OpenMP/clause-validity01.f90 b/flang/test/Semantics/OpenMP/clause-validity01.f90 index a01f0cff43ccf3..62c45818822c68 100644 --- a/flang/test/Semantics/OpenMP/clause-validity01.f90 +++ b/flang/test/Semantics/OpenMP/clause-validity01.f90 @@ -509,7 +509,7 @@ !$omp taskwait depend(mutexinoutset: x) !ERROR: 'task-dependence-type' modifier is required !ERROR: The SINK and SOURCE dependence types can only be used with the ORDERED directive, used here in the TASKWAIT construct - !WARNING: 'dependence-type' modifier is no longer supported on DEPEND clause in OpenMP v5.2 + !WARNING: 'dependence-type' modifier is no longer allowed on DEPEND clause since OpenMP v5.2 [-Wopenmp-deprecated] !$omp taskwait depend(source) ! !$omp taskwait depend(sink:i-1) ! !$omp target enter data map(to:arrayA) map(alloc:arrayB) diff --git a/flang/test/Semantics/OpenMP/depend05.f90 b/flang/test/Semantics/OpenMP/depend05.f90 index 7cbe6c24089b11..aa579fb0058642 100644 --- a/flang/test/Semantics/OpenMP/depend05.f90 +++ b/flang/test/Semantics/OpenMP/depend05.f90 @@ -2,7 +2,7 @@ subroutine f00(x) integer :: x(10) -!WARNING: 'iterator' modifier is not supported on DEPEND clause in OpenMP v4.5, try -fopenmp-version=50 +!WARNING: 'iterator' modifier is not allowed on DEPEND clause in OpenMP v4.5, try -fopenmp-version=50 [-Wopenmp-future] !$omp task depend(iterator(i = 1:10), in: x(i)) x = 0 !$omp end task diff --git a/flang/test/Semantics/OpenMP/depobj-construct-v52.f90 b/flang/test/Semantics/OpenMP/depobj-construct-v52.f90 index 37bd6191f215ef..6d59d98568ab5a 100644 --- a/flang/test/Semantics/OpenMP/depobj-construct-v52.f90 +++ b/flang/test/Semantics/OpenMP/depobj-construct-v52.f90 @@ -3,7 +3,7 @@ subroutine f00 integer :: obj !ERROR: 'task-dependence-type' modifier is required -!WARNING: 'dependence-type' modifier is no longer supported on DEPEND clause in OpenMP v5.2 +!WARNING: 'dependence-type' modifier is no longer allowed on DEPEND clause since OpenMP v5.2 [-Wopenmp-deprecated] !ERROR: A DEPEND clause on a DEPOBJ construct must not have SINK or SOURCE as dependence type !$omp depobj(obj) depend(source) end diff --git a/flang/test/Semantics/OpenMP/from-clause-v45.f90 b/flang/test/Semantics/OpenMP/from-clause-v45.f90 index 35787f872ea898..fcd9158e4adfb1 100644 --- a/flang/test/Semantics/OpenMP/from-clause-v45.f90 +++ b/flang/test/Semantics/OpenMP/from-clause-v45.f90 @@ -8,14 +8,14 @@ subroutine f00(x) subroutine f01(x) integer :: x(10) -!WARNING: 'iterator' modifier is not supported on FROM clause in OpenMP v4.5, try -fopenmp-version=51 +!WARNING: 'iterator' modifier is not allowed on FROM clause in OpenMP v4.5, try -fopenmp-version=51 [-Wopenmp-future] !$omp target update from(iterator(i = 1:5): x(i)) end subroutine f02(x) integer :: x(10) -!WARNING: 'motion-modifier' modifier is not supported on FROM clause in OpenMP v4.5, try -fopenmp-version=51 -!WARNING: 'iterator' modifier is not supported on FROM clause in OpenMP v4.5, try -fopenmp-version=51 +!WARNING: 'motion-modifier' modifier is not allowed on FROM clause in OpenMP v4.5, try -fopenmp-version=51 [-Wopenmp-future] +!WARNING: 'iterator' modifier is not allowed on FROM clause in OpenMP v4.5, try -fopenmp-version=51 [-Wopenmp-future] !$omp target update from(present, iterator(i = 1:5): x(i)) end diff --git a/flang/test/Semantics/OpenMP/linear-clause02.f90 b/flang/test/Semantics/OpenMP/linear-clause02.f90 index e6eedbcae7c92c..16c470646a3463 100644 --- a/flang/test/Semantics/OpenMP/linear-clause02.f90 +++ b/flang/test/Semantics/OpenMP/linear-clause02.f90 @@ -9,7 +9,7 @@ subroutine f00(x) subroutine f01(x) integer :: x !WARNING: The 'modifier(<list>)' syntax is deprecated in OpenMP v5.2, use '<list> : modifier' instead - !ERROR: An exclusive 'step-simple-modifier' modifier cannot be specified together with a modifier of a different type + !ERROR: 'step-simple-modifier' modifier cannot be specified together with a modifier of a different type !$omp declare simd linear(uval(x) : 2) end diff --git a/flang/test/Semantics/OpenMP/map-modifiers-v60.f90 b/flang/test/Semantics/OpenMP/map-modifiers-v60.f90 index 00fd524ab12a9a..0599034467c8da 100644 --- a/flang/test/Semantics/OpenMP/map-modifiers-v60.f90 +++ b/flang/test/Semantics/OpenMP/map-modifiers-v60.f90 @@ -2,7 +2,7 @@ subroutine f00(x) integer :: x -!WARNING: 'self-modifier' modifier is not supported on MAP clause in OpenMP v5.2, try -fopenmp-version=60 +!WARNING: 'self-modifier' modifier is not allowed on MAP clause in OpenMP v5.2, try -fopenmp-version=60 [-Wopenmp-future] !$omp target map(self: x) x = x + 1 !$omp end target @@ -10,7 +10,7 @@ subroutine f00(x) subroutine f01(x) integer, pointer :: x -!WARNING: 'ref-modifier' modifier is not supported on MAP clause in OpenMP v5.2, try -fopenmp-version=60 +!WARNING: 'ref-modifier' modifier is not allowed on MAP clause in OpenMP v5.2, try -fopenmp-version=60 [-Wopenmp-future] !$omp target map(ref_ptr: x) x = x + 1 !$omp end target @@ -18,7 +18,7 @@ subroutine f01(x) subroutine f02(x) integer, pointer :: x -!WARNING: 'ref-modifier' modifier is not supported on MAP clause in OpenMP v5.2, try -fopenmp-version=60 +!WARNING: 'ref-modifier' modifier is not allowed on MAP clause in OpenMP v5.2, try -fopenmp-version=60 [-Wopenmp-future] !$omp target map(ref_ptee: x) x = x + 1 !$omp end target @@ -26,7 +26,7 @@ subroutine f02(x) subroutine f03(x) integer, pointer :: x -!WARNING: 'ref-modifier' modifier is not supported on MAP clause in OpenMP v5.2, try -fopenmp-version=60 +!WARNING: 'ref-modifier' modifier is not allowed on MAP clause in OpenMP v5.2, try -fopenmp-version=60 [-Wopenmp-future] !$omp target map(ref_ptr_ptee: x) x = x + 1 !$omp end target diff --git a/flang/test/Semantics/OpenMP/task-depend.f90 b/flang/test/Semantics/OpenMP/task-depend.f90 index def3e2cf24c0c3..51cadf17b8001a 100644 --- a/flang/test/Semantics/OpenMP/task-depend.f90 +++ b/flang/test/Semantics/OpenMP/task-depend.f90 @@ -1,7 +1,7 @@ ! RUN: %python %S/../test_errors.py %s %flang -fopenmp program test -! ERROR: modifier from the modifier set on DEPEND clause is required +! ERROR: One of dependence-type or task-dependence-type modifiers is required on DEPEND clause ! ERROR: A DEPEND clause on a TASK construct must have a valid task dependence type !$omp task depend(ii) !$omp end task diff --git a/flang/test/Semantics/OpenMP/to-clause-v45.f90 b/flang/test/Semantics/OpenMP/to-clause-v45.f90 index b336005699377f..9e6b1790dc9cca 100644 --- a/flang/test/Semantics/OpenMP/to-clause-v45.f90 +++ b/flang/test/Semantics/OpenMP/to-clause-v45.f90 @@ -8,14 +8,14 @@ subroutine f00(x) subroutine f01(x) integer :: x(10) -!WARNING: 'iterator' modifier is not supported on TO clause in OpenMP v4.5, try -fopenmp-version=51 +!WARNING: 'iterator' modifier is not allowed on TO clause in OpenMP v4.5, try -fopenmp-version=51 [-Wopenmp-future] !$omp target update to(iterator(i = 1:5): x(i)) end subroutine f02(x) integer :: x(10) -!WARNING: 'motion-modifier' modifier is not supported on TO clause in OpenMP v4.5, try -fopenmp-version=51 -!WARNING: 'iterator' modifier is not supported on TO clause in OpenMP v4.5, try -fopenmp-version=51 +!WARNING: 'motion-modifier' modifier is not allowed on TO clause in OpenMP v4.5, try -fopenmp-version=51 [-Wopenmp-future] +!WARNING: 'iterator' modifier is not allowed on TO clause in OpenMP v4.5, try -fopenmp-version=51 [-Wopenmp-future] !$omp target update to(present, iterator(i = 1:5): x(i)) end diff --git a/flang/test/Semantics/OpenMP/uses-allocators-version50.f90 b/flang/test/Semantics/OpenMP/uses-allocators-version50.f90 index 2bc3a07859682d..74f5856b841390 100644 --- a/flang/test/Semantics/OpenMP/uses-allocators-version50.f90 +++ b/flang/test/Semantics/OpenMP/uses-allocators-version50.f90 @@ -33,20 +33,20 @@ subroutine uses_allocators_50 x = 3 !$omp end target - !WARNING: 'traits-array' modifier is not supported on USES_ALLOCATORS clause in OpenMP v5.0, try -fopenmp-version=52 + !WARNING: 'traits-array' modifier is not allowed on USES_ALLOCATORS clause in OpenMP v5.0, try -fopenmp-version=52 [-Wopenmp-future] !$omp target uses_allocators(traits(tr): my_alloc) x = 4 !$omp end target ! Accepting the newer syntax with a warning does not skip the remaining ! checks on the specification. - !WARNING: 'traits-array' modifier is not supported on USES_ALLOCATORS clause in OpenMP v5.0, try -fopenmp-version=52 + !WARNING: 'traits-array' modifier is not allowed on USES_ALLOCATORS clause in OpenMP v5.0, try -fopenmp-version=52 [-Wopenmp-future] !ERROR: The traits array must be a named constant array !$omp target uses_allocators(traits(1): my_alloc) x = 5 !$omp end target - !WARNING: 'mem-space' modifier is not supported on USES_ALLOCATORS clause in OpenMP v5.0, try -fopenmp-version=52 + !WARNING: 'mem-space' modifier is not allowed on USES_ALLOCATORS clause in OpenMP v5.0, try -fopenmp-version=52 [-Wopenmp-future] !ERROR: A non-predefined allocator 'my_alloc' in a USES_ALLOCATORS clause must have traits specified in OpenMP v5.0 !$omp target uses_allocators(memspace(omp_default_mem_space): my_alloc) x = 6 diff --git a/flang/test/Semantics/OpenMP/uses-allocators-version51.f90 b/flang/test/Semantics/OpenMP/uses-allocators-version51.f90 index 01e1b01ed36bce..dd5c40e5f899c8 100644 --- a/flang/test/Semantics/OpenMP/uses-allocators-version51.f90 +++ b/flang/test/Semantics/OpenMP/uses-allocators-version51.f90 @@ -27,14 +27,14 @@ subroutine uses_allocators_51 x = 2 !$omp end target - !WARNING: 'traits-array' modifier is not supported on USES_ALLOCATORS clause in OpenMP v5.1, try -fopenmp-version=52 + !WARNING: 'traits-array' modifier is not allowed on USES_ALLOCATORS clause in OpenMP v5.1, try -fopenmp-version=52 [-Wopenmp-future] !$omp target uses_allocators(traits(tr): my_alloc) x = 3 !$omp end target ! Accepting the newer syntax with a warning does not skip the remaining ! checks on the specification. - !WARNING: 'traits-array' modifier is not supported on USES_ALLOCATORS clause in OpenMP v5.1, try -fopenmp-version=52 + !WARNING: 'traits-array' modifier is not allowed on USES_ALLOCATORS clause in OpenMP v5.1, try -fopenmp-version=52 [-Wopenmp-future] !ERROR: The traits array must be a named constant array !$omp target uses_allocators(traits(1): my_alloc) x = 4 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
