[flang-commits] [flang] [flang][OpenMP] Properly use GetAllowedElements (PR #229523)

[Krzysztof Parzyszek via flang-commits]

https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/229523

>From 90eba5e5f968013cd616758803e17959bba5f60a Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 1 Oct 2026 12:44:04 -0500
Subject: [PATCH 1/2] [flang][OpenMP] Properly use GetAllowedElements

Allowed elements are not just those explicitly listed, but also those
from the union of allowed sets.

For example, the set of modifiers allowed on a clause are those that
are listed on a given clause, plus the union of all modifier sets that
the clause allows.
---
 flang/lib/Semantics/check-omp-syntax.cpp | 38 ++++++++++++++++--------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-syntax.cpp b/flang/lib/Semantics/check-omp-syntax.cpp
index d96d9670367f97..ef2d08adb06202 100644
--- a/flang/lib/Semantics/check-omp-syntax.cpp
+++ b/flang/lib/Semantics/check-omp-syntax.cpp
@@ -49,6 +49,16 @@ static llvm::omp::ModifierSets GetSets(
   return desc.getModifierSets(version);
 }
 
+template <typename DescriptorTy>
+static auto GetAllowedElements(
+    const DescriptorTy &desc, llvm::omp::Version version) {
+  auto allowed{GetElements(desc, version)};
+  for (auto s : GetSets(desc, version)) {
+    allowed |= GetElements(llvm::omp::getDescriptor(s), version);
+  }
+  return allowed;
+}
+
 template < //
     typename ElemTy, typename SetsSetTy, typename OwnerTy,
     typename ResultTy = llvm::DenseMap<ElemTy,
@@ -57,13 +67,14 @@ static ResultTy VerifyVersions(
     const AppliedElementInfo<ElemTy, SetsSetTy> &info, OwnerTy ownerId,
     llvm::omp::Version version) {
   using AppliedElementTy = AppliedElement<ElemTy, SetsSetTy>;
+  using ElemSetTy = llvm::omp::EnumSet<ElemTy>;
   ResultTy result;
 
   auto &odesc{llvm::omp::getDescriptor(ownerId)};
-  auto elements{GetElements(odesc, version)};
+  ElemSetTy allowed{GetAllowedElements(odesc, version)};
 
   for (const AppliedElementTy &elem : info.elements) {
-    if (elements.test(elem.id.value)) {
+    if (allowed.test(elem.id.value)) {
       continue;
     }
     llvm::omp::Version since{~0u}, until{0u};
@@ -123,12 +134,13 @@ template < //
 static ResultTy VerifyUnique(const AppliedElementInfo<ElemTy, SetsSetTy> &info,
     OwnerTy ownerId, llvm::omp::Version version) {
   using AppliedElementTy = AppliedElement<ElemTy, SetsSetTy>;
-  llvm::omp::EnumSet<ElemTy> unique;
+  using ElemSetTy = llvm::omp::EnumSet<ElemTy>;
+  ElemSetTy unique;
 
   auto &odesc{llvm::omp::getDescriptor(ownerId)};
-  auto elements{GetElements(odesc, version)};
+  ElemSetTy allowed{GetAllowedElements(odesc, version)};
 
-  for (auto e : elements) {
+  for (auto e : GetElements(odesc, version)) {
     auto &edesc{llvm::omp::getDescriptor(e)};
     // Ultimate modifiers should have the "unique" property present as well.
     if (edesc.getProperties(version).test(llvm::omp::Property::Unique)) {
@@ -145,7 +157,7 @@ static ResultTy VerifyUnique(const AppliedElementInfo<ElemTy, SetsSetTy> &info,
   ResultTy repeated;
   llvm::DenseMap<ElemTy, parser::CharBlock> present;
   for (const AppliedElementTy &elem : info.elements) {
-    if (!elements.test(elem.id.value)) {
+    if (!allowed.test(elem.id.value)) {
       // Skip invalid elements.
       continue;
     }
@@ -171,11 +183,11 @@ static ResultTy VerifyExclusive(
   ResultTy result;
 
   auto &odesc{llvm::omp::getDescriptor(ownerId)};
-  auto elements{GetElements(odesc, version)};
+  auto allowed{GetAllowedElements(odesc, version)};
 
   llvm::DenseMap<ElemTy, parser::CharBlock> present;
   for (const AppliedElementTy &elem : info.elements) {
-    if (!elements.test(elem.id.value)) {
+    if (!allowed.test(elem.id.value)) {
       // Skip invalid elements.
       continue;
     }
@@ -212,11 +224,11 @@ static ResultTy VerifyMutuallyExclusive(
   ResultTy result;
 
   auto &odesc{llvm::omp::getDescriptor(ownerId)};
-  auto elements{GetElements(odesc, version)};
+  auto allowed{GetAllowedElements(odesc, version)};
 
   llvm::DenseMap<SetTy, const AppliedElementTy *> exclusive;
   for (const AppliedElementTy &elem : info.elements) {
-    if (!elements.test(elem.id.value)) {
+    if (!allowed.test(elem.id.value)) {
       // Skip invalid elements.
       continue;
     }
@@ -254,9 +266,9 @@ static ResultTy VerifyUltimate(
   llvm::omp::EnumSet<ElemTy> ultimate;
 
   auto &odesc{llvm::omp::getDescriptor(ownerId)};
-  auto elements{GetElements(odesc, version)};
+  auto allowed{GetAllowedElements(odesc, version)};
 
-  for (auto e : elements) {
+  for (auto e : GetElements(odesc, version)) {
     auto &edesc{llvm::omp::getDescriptor(e)};
     if (edesc.getProperties(version).test(llvm::omp::Property::Ultimate)) {
       ultimate.set(e);
@@ -275,7 +287,7 @@ static ResultTy VerifyUltimate(
           : llvm::ArrayRef<AppliedElementTy>(info.elements).drop_front(1)};
 
   for (const AppliedElementTy &elem : rest) {
-    if (!elements.test(elem.id.value)) {
+    if (!allowed.test(elem.id.value)) {
       // Skip invalid elements.
       continue;
     }

>From 2149d4f422a322a845929c5d915f6f4a403de6b2 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Sun, 18 Oct 2026 09:05:54 -0500
Subject: [PATCH 2/2] Add requested testcases

---
 .../Semantics/OpenMP/map-modifiers-v61.f90    | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/flang/test/Semantics/OpenMP/map-modifiers-v61.f90 b/flang/test/Semantics/OpenMP/map-modifiers-v61.f90
index ce2f95ae4afc2c..7515293055b355 100644
--- a/flang/test/Semantics/OpenMP/map-modifiers-v61.f90
+++ b/flang/test/Semantics/OpenMP/map-modifiers-v61.f90
@@ -47,3 +47,26 @@ subroutine f04(n)
   !$omp end target
 end
 end module
+
+subroutine f05(x)
+  integer :: x
+  !Expect no diagonstics
+  !$omp target map(always, to: x)
+  !$omp end target
+end
+
+subroutine f06(x)
+  integer :: x
+  x = 0
+  !Expect no diagonstics
+  !$omp target defaultmap(tofrom: scalar)
+  x = x + 1
+  !$omp end target
+end
+
+subroutine f07(x)
+  integer :: x
+  !ERROR: 'variable-category' modifier cannot occur multiple times
+  !$omp target defaultmap(tofrom: scalar, aggregate)
+  !$omp end target
+end