[flang-commits] [flang] [llvm] [Flang] [OpenMP] Add semantic checks for detach clause in task (PR #119172)

Thirumalai Shaktivel via flang-commits flang-commits at lists.llvm.org
Thu Dec 26 01:26:59 PST 2024


https://github.com/Thirumalai-Shaktivel updated https://github.com/llvm/llvm-project/pull/119172

>From 6e491ccd80b902df6946713a372ec9667e0811c3 Mon Sep 17 00:00:00 2001
From: Thirumalai-Shaktivel <thirumalaishaktivel at gmail.com>
Date: Mon, 9 Dec 2024 07:37:01 +0000
Subject: [PATCH 1/6] [Flang] [OpenMP] Add semantic checks for detach clause in
 Task

Fixes:
- Add semantic checks along with the tests
- Move the detach clause to allowedOnceClauses list in
  Task construct

Restrictions:\
OpenMP 5.0: Task construct
- At most one detach clause can appear on the directive.
- If a detach clause appears on the directive, then a mergeable
  clause cannot appear on the same directive.

OpenMP 5.2: Detach contruct
- If a detach clause appears on a directive, then the encountering
  task must not be a final task.
- A variable that appears in a detach clause cannot appear as a
  list item on a data-environment attribute clause on the same
  construct.
- A variable that is part of another variable (as an array element or a
  structure element) cannot appear in a detach clause.
- event-handle must not have the POINTER attribute.
---
 flang/lib/Semantics/check-omp-structure.cpp | 141 +++++++++++++++-----
 flang/lib/Semantics/check-omp-structure.h   |   2 +
 flang/test/Semantics/OpenMP/detach01.f90    |  65 +++++++++
 llvm/include/llvm/Frontend/OpenMP/OMP.td    |   2 +-
 4 files changed, 179 insertions(+), 31 deletions(-)
 create mode 100644 flang/test/Semantics/OpenMP/detach01.f90

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 95b962f5daf57c..6641e39c6e3582 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2733,6 +2733,59 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
         llvm::omp::Clause::OMPC_copyprivate, {llvm::omp::Clause::OMPC_nowait});
   }
 
+  if (GetContext().directive == llvm::omp::Directive::OMPD_task) {
+    if (auto *d_clause{FindClause(llvm::omp::Clause::OMPC_detach)}) {
+      // OpenMP 5.0: Task construct restrictions
+      CheckNotAllowedIfClause(
+          llvm::omp::Clause::OMPC_detach, {llvm::omp::Clause::OMPC_mergeable});
+
+      // OpenMP 5.2: Task construct restrictions
+      if (FindClause(llvm::omp::Clause::OMPC_final)) {
+        context_.Say(GetContext().clauseSource,
+            "If a DETACH clause appears on a directive, then the encountering task must not be a FINAL task"_err_en_US);
+      }
+
+      const auto &detachClause{
+          std::get<parser::OmpClause::Detach>(d_clause->u)};
+      if (const auto *name{parser::Unwrap<parser::Name>(detachClause.v.v)}) {
+        if (name->symbol) {
+          std::string eventHandleSymName{name->ToString()};
+          auto checkVarAppearsInDataEnvClause = [&](const parser::OmpObjectList
+                                                        &objs,
+                                                    std::string clause) {
+            for (const auto &obj : objs.v) {
+              if (const parser::Name *objName{
+                      parser::Unwrap<parser::Name>(obj)}) {
+                if (objName->ToString() == eventHandleSymName) {
+                  context_.Say(GetContext().clauseSource,
+                      "A variable: `%s` that appears in a DETACH clause cannot appear on %s clause on the same construct"_err_en_US,
+                      eventHandleSymName, clause);
+                }
+              }
+            }
+          };
+          if (auto *dataEnvClause{
+                  FindClause(llvm::omp::Clause::OMPC_private)}) {
+            const auto &pClause{
+                std::get<parser::OmpClause::Private>(dataEnvClause->u)};
+            checkVarAppearsInDataEnvClause(pClause.v, "PRIVATE");
+          } else if (auto *dataEnvClause{
+                         FindClause(llvm::omp::Clause::OMPC_firstprivate)}) {
+            const auto &fpClause{
+                std::get<parser::OmpClause::Firstprivate>(dataEnvClause->u)};
+            checkVarAppearsInDataEnvClause(fpClause.v, "FIRSTPRIVATE");
+          } else if (auto *dataEnvClause{
+                         FindClause(llvm::omp::Clause::OMPC_in_reduction)}) {
+            const auto &irClause{
+                std::get<parser::OmpClause::InReduction>(dataEnvClause->u)};
+            checkVarAppearsInDataEnvClause(
+                std::get<parser::OmpObjectList>(irClause.v.t), "IN_REDUCTION");
+          }
+        }
+      }
+    }
+  }
+
   auto testThreadprivateVarErr = [&](Symbol sym, parser::Name name,
                                      llvmOmpClause clauseTy) {
     if (sym.test(Symbol::Flag::OmpThreadprivate))
@@ -2815,7 +2868,6 @@ CHECK_SIMPLE_CLAUSE(Capture, OMPC_capture)
 CHECK_SIMPLE_CLAUSE(Contains, OMPC_contains)
 CHECK_SIMPLE_CLAUSE(Default, OMPC_default)
 CHECK_SIMPLE_CLAUSE(Depobj, OMPC_depobj)
-CHECK_SIMPLE_CLAUSE(Detach, OMPC_detach)
 CHECK_SIMPLE_CLAUSE(DeviceType, OMPC_device_type)
 CHECK_SIMPLE_CLAUSE(DistSchedule, OMPC_dist_schedule)
 CHECK_SIMPLE_CLAUSE(Exclusive, OMPC_exclusive)
@@ -3386,40 +3438,45 @@ void OmpStructureChecker::CheckIsVarPartOfAnotherVar(
     const parser::CharBlock &source, const parser::OmpObjectList &objList,
     llvm::StringRef clause) {
   for (const auto &ompObject : objList.v) {
-    common::visit(
-        common::visitors{
-            [&](const parser::Designator &designator) {
-              if (const auto *dataRef{
-                      std::get_if<parser::DataRef>(&designator.u)}) {
-                if (IsDataRefTypeParamInquiry(dataRef)) {
+    CheckIsVarPartOfAnotherVar(source, ompObject, clause);
+  }
+}
+
+void OmpStructureChecker::CheckIsVarPartOfAnotherVar(
+    const parser::CharBlock &source, const parser::OmpObject &ompObject,
+    llvm::StringRef clause) {
+  common::visit(
+      common::visitors{
+          [&](const parser::Designator &designator) {
+            if (const auto *dataRef{
+                    std::get_if<parser::DataRef>(&designator.u)}) {
+              if (IsDataRefTypeParamInquiry(dataRef)) {
+                context_.Say(source,
+                    "A type parameter inquiry cannot appear on the %s "
+                    "directive"_err_en_US,
+                    ContextDirectiveAsFortran());
+              } else if (parser::Unwrap<parser::StructureComponent>(
+                             ompObject) ||
+                  parser::Unwrap<parser::ArrayElement>(ompObject)) {
+                if (llvm::omp::nonPartialVarSet.test(GetContext().directive)) {
                   context_.Say(source,
-                      "A type parameter inquiry cannot appear on the %s "
+                      "A variable that is part of another variable (as an "
+                      "array or structure element) cannot appear on the %s "
                       "directive"_err_en_US,
                       ContextDirectiveAsFortran());
-                } else if (parser::Unwrap<parser::StructureComponent>(
-                               ompObject) ||
-                    parser::Unwrap<parser::ArrayElement>(ompObject)) {
-                  if (llvm::omp::nonPartialVarSet.test(
-                          GetContext().directive)) {
-                    context_.Say(source,
-                        "A variable that is part of another variable (as an "
-                        "array or structure element) cannot appear on the %s "
-                        "directive"_err_en_US,
-                        ContextDirectiveAsFortran());
-                  } else {
-                    context_.Say(source,
-                        "A variable that is part of another variable (as an "
-                        "array or structure element) cannot appear in a "
-                        "%s clause"_err_en_US,
-                        clause.data());
-                  }
+                } else {
+                  context_.Say(source,
+                      "A variable that is part of another variable (as an "
+                      "array or structure element) cannot appear in a "
+                      "%s clause"_err_en_US,
+                      clause.data());
                 }
               }
-            },
-            [&](const parser::Name &name) {},
-        },
-        ompObject.u);
-  }
+            }
+          },
+          [&](const parser::Name &name) {},
+      },
+      ompObject.u);
 }
 
 void OmpStructureChecker::Enter(const parser::OmpClause::Firstprivate &x) {
@@ -3746,6 +3803,30 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Linear &x) {
   }
 }
 
+void OmpStructureChecker::Enter(const parser::OmpClause::Detach &x) {
+  // OpenMP 5.0: Task construct restrictions
+  CheckAllowedClause(llvm::omp::Clause::OMPC_detach);
+
+  // OpenMP 5.2: Detach clause restrictions
+  CheckIsVarPartOfAnotherVar(GetContext().clauseSource, x.v.v, "DETACH");
+  if (const auto *name{parser::Unwrap<parser::Name>(x.v.v)}) {
+    if (name->symbol) {
+      if (IsPointer(*name->symbol)) {
+        context_.Say(GetContext().clauseSource,
+            "The event-handle: `%s` must not have the POINTER attribute"_err_en_US,
+            name->ToString());
+      }
+    }
+    auto type{name->symbol->GetType()};
+    if (!name->symbol->GetType()->IsNumeric(TypeCategory::Integer) ||
+        evaluate::ToInt64(type->numericTypeSpec().kind()) != 8) {
+      context_.Say(GetContext().clauseSource,
+          "The event-handle: `%s` must be of type integer(kind=omp_event_handle_kind)"_err_en_US,
+          name->ToString());
+    }
+  }
+}
+
 void OmpStructureChecker::CheckAllowedMapTypes(
     const parser::OmpMapType::Value &type,
     const std::list<parser::OmpMapType::Value> &allowedMapTypeList) {
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 346a7bed9138f0..a8f94992ff0914 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -186,6 +186,8 @@ class OmpStructureChecker
       const common::Indirection<parser::ArrayElement> &, const parser::Name &);
   void CheckDoacross(const parser::OmpDoacross &doa);
   bool IsDataRefTypeParamInquiry(const parser::DataRef *dataRef);
+  void CheckIsVarPartOfAnotherVar(const parser::CharBlock &source,
+      const parser::OmpObject &obj, llvm::StringRef clause = "");
   void CheckIsVarPartOfAnotherVar(const parser::CharBlock &source,
       const parser::OmpObjectList &objList, llvm::StringRef clause = "");
   void CheckThreadprivateOrDeclareTargetVar(
diff --git a/flang/test/Semantics/OpenMP/detach01.f90 b/flang/test/Semantics/OpenMP/detach01.f90
new file mode 100644
index 00000000000000..e342fcd1b19b4a
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/detach01.f90
@@ -0,0 +1,65 @@
+! REQUIRES: openmp_runtime
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=50
+
+! OpenMP Version 5.2
+! Various checks for DETACH Clause (12.5.2)
+
+program test_detach
+    use omp_lib
+    implicit none
+    integer :: e, x
+    integer(omp_event_handle_kind) :: event_01, event_02(2)
+    integer(omp_event_handle_kind), pointer :: event_03
+
+
+    type :: t
+        integer(omp_event_handle_kind) :: event
+    end type
+
+    type(t) :: t_01
+
+    !ERROR: The event-handle: `e` must be of type integer(kind=omp_event_handle_kind)
+    !$omp task detach(e)
+        x = x + 1
+    !$omp end task
+
+    !ERROR: At most one DETACH clause can appear on the TASK directive
+    !$omp task detach(event_01) detach(event_01)
+        x = x + 1
+    !$omp end task
+
+    !ERROR: Clause MERGEABLE is not allowed if clause DETACH appears on the TASK directive
+    !$omp task detach(event_01) mergeable
+        x = x + 1
+    !$omp end task
+
+    !ERROR: If a DETACH clause appears on a directive, then the encountering task must not be a FINAL task
+    !$omp task detach(event_01) final(.false.)
+        x = x + 1
+    !$omp end task
+
+    !ERROR: A variable: `event_01` that appears in a DETACH clause cannot appear on PRIVATE clause on the same construct
+    !$omp task detach(event_01) private(event_01)
+        x = x + 1
+    !$omp end task
+
+    !ERROR: A variable: `event_01` that appears in a DETACH clause cannot appear on IN_REDUCTION clause on the same construct
+    !$omp task detach(event_01) in_reduction(+:event_01)
+        x = x + 1
+    !$omp end task
+
+    !ERROR: A variable that is part of another variable (as an array or structure element) cannot appear in a DETACH clause
+    !$omp task detach(event_02(1))
+        x = x + 1
+    !$omp end task
+
+    !ERROR: A variable that is part of another variable (as an array or structure element) cannot appear in a DETACH clause
+    !$omp task detach(t_01%event)
+        x = x + 1
+    !$omp end task
+
+    !ERROR: The event-handle: `event_03` must not have the POINTER attribute
+    !$omp task detach(event_03)
+        x = x + 1
+    !$omp end task
+end program
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index e36eb77cefe7e3..aec80decf60395 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -1090,7 +1090,6 @@ def OMP_Task : Directive<"task"> {
     VersionedClause<OMPC_Affinity, 50>,
     VersionedClause<OMPC_Allocate>,
     VersionedClause<OMPC_Depend>,
-    VersionedClause<OMPC_Detach, 50>,
     VersionedClause<OMPC_FirstPrivate>,
     VersionedClause<OMPC_InReduction>,
     VersionedClause<OMPC_Mergeable>,
@@ -1100,6 +1099,7 @@ def OMP_Task : Directive<"task"> {
   ];
   let allowedOnceClauses = [
     VersionedClause<OMPC_Default>,
+    VersionedClause<OMPC_Detach, 50>,
     VersionedClause<OMPC_Final>,
     VersionedClause<OMPC_If>,
     VersionedClause<OMPC_Priority>,

>From fa2b051e724898f3be9b2d74d218816088b3975f Mon Sep 17 00:00:00 2001
From: Thirumalai-Shaktivel <thirumalaishaktivel at gmail.com>
Date: Thu, 26 Dec 2024 05:51:11 +0000
Subject: [PATCH 2/6] Do not check for interger kind

---
 flang/lib/Semantics/check-omp-structure.cpp | 4 +---
 flang/test/Semantics/OpenMP/detach01.f90    | 4 ++--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 6641e39c6e3582..e2f897f5c92466 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3817,9 +3817,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Detach &x) {
             name->ToString());
       }
     }
-    auto type{name->symbol->GetType()};
-    if (!name->symbol->GetType()->IsNumeric(TypeCategory::Integer) ||
-        evaluate::ToInt64(type->numericTypeSpec().kind()) != 8) {
+    if (!name->symbol->GetType()->IsNumeric(TypeCategory::Integer)) {
       context_.Say(GetContext().clauseSource,
           "The event-handle: `%s` must be of type integer(kind=omp_event_handle_kind)"_err_en_US,
           name->ToString());
diff --git a/flang/test/Semantics/OpenMP/detach01.f90 b/flang/test/Semantics/OpenMP/detach01.f90
index e342fcd1b19b4a..7ba2888be9237f 100644
--- a/flang/test/Semantics/OpenMP/detach01.f90
+++ b/flang/test/Semantics/OpenMP/detach01.f90
@@ -7,8 +7,8 @@
 program test_detach
     use omp_lib
     implicit none
-    integer :: e, x
-    integer(omp_event_handle_kind) :: event_01, event_02(2)
+    real                                    :: e, x
+    integer(omp_event_handle_kind)          :: event_01, event_02(2)
     integer(omp_event_handle_kind), pointer :: event_03
 
 

>From 569712fc9b14f3a1f4b3a7d2c55febfe216f5e7f Mon Sep 17 00:00:00 2001
From: Thirumalai-Shaktivel <thirumalaishaktivel at gmail.com>
Date: Thu, 26 Dec 2024 06:14:26 +0000
Subject: [PATCH 3/6] Fix snake_case

---
 flang/lib/Semantics/check-omp-structure.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index e2f897f5c92466..aa19b71e69c62a 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2734,7 +2734,7 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
   }
 
   if (GetContext().directive == llvm::omp::Directive::OMPD_task) {
-    if (auto *d_clause{FindClause(llvm::omp::Clause::OMPC_detach)}) {
+    if (auto *detachClause{FindClause(llvm::omp::Clause::OMPC_detach)}) {
       // OpenMP 5.0: Task construct restrictions
       CheckNotAllowedIfClause(
           llvm::omp::Clause::OMPC_detach, {llvm::omp::Clause::OMPC_mergeable});
@@ -2745,9 +2745,9 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
             "If a DETACH clause appears on a directive, then the encountering task must not be a FINAL task"_err_en_US);
       }
 
-      const auto &detachClause{
-          std::get<parser::OmpClause::Detach>(d_clause->u)};
-      if (const auto *name{parser::Unwrap<parser::Name>(detachClause.v.v)}) {
+      const auto &detach{
+          std::get<parser::OmpClause::Detach>(detachClause->u)};
+      if (const auto *name{parser::Unwrap<parser::Name>(detach.v.v)}) {
         if (name->symbol) {
           std::string eventHandleSymName{name->ToString()};
           auto checkVarAppearsInDataEnvClause = [&](const parser::OmpObjectList

>From 42cc3e75d041b9935b38e19cb70d7b3273f4087f Mon Sep 17 00:00:00 2001
From: Thirumalai-Shaktivel <thirumalaishaktivel at gmail.com>
Date: Thu, 26 Dec 2024 06:36:11 +0000
Subject: [PATCH 4/6] Compare symbols instead of name

---
 flang/lib/Semantics/check-omp-structure.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index aa19b71e69c62a..084dbf48c5c50b 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2749,17 +2749,17 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
           std::get<parser::OmpClause::Detach>(detachClause->u)};
       if (const auto *name{parser::Unwrap<parser::Name>(detach.v.v)}) {
         if (name->symbol) {
-          std::string eventHandleSymName{name->ToString()};
+          Symbol *eventHandleSym{name->symbol->GetUltimate()};
           auto checkVarAppearsInDataEnvClause = [&](const parser::OmpObjectList
                                                         &objs,
                                                     std::string clause) {
             for (const auto &obj : objs.v) {
               if (const parser::Name *objName{
                       parser::Unwrap<parser::Name>(obj)}) {
-                if (objName->ToString() == eventHandleSymName) {
+                if (objName->symbol->GetUltimate() == eventHandleSym) {
                   context_.Say(GetContext().clauseSource,
                       "A variable: `%s` that appears in a DETACH clause cannot appear on %s clause on the same construct"_err_en_US,
-                      eventHandleSymName, clause);
+                      objName->source, clause);
                 }
               }
             }

>From a3c6a4517354991dc5f166d57d14012571b3f553 Mon Sep 17 00:00:00 2001
From: Thirumalai-Shaktivel <thirumalaishaktivel at gmail.com>
Date: Thu, 26 Dec 2024 09:09:15 +0000
Subject: [PATCH 5/6] Add OpenMP version based checks

---
 flang/lib/Semantics/check-omp-structure.cpp | 105 ++++++++++----------
 flang/test/Semantics/OpenMP/detach01.f90    |  22 +---
 flang/test/Semantics/OpenMP/detach02.f90    |  22 ++++
 llvm/include/llvm/Frontend/OpenMP/OMP.td    |   2 +-
 4 files changed, 83 insertions(+), 68 deletions(-)
 create mode 100644 flang/test/Semantics/OpenMP/detach02.f90

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 084dbf48c5c50b..41ed858ed650fd 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2735,51 +2735,54 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
 
   if (GetContext().directive == llvm::omp::Directive::OMPD_task) {
     if (auto *detachClause{FindClause(llvm::omp::Clause::OMPC_detach)}) {
-      // OpenMP 5.0: Task construct restrictions
-      CheckNotAllowedIfClause(
-          llvm::omp::Clause::OMPC_detach, {llvm::omp::Clause::OMPC_mergeable});
-
-      // OpenMP 5.2: Task construct restrictions
-      if (FindClause(llvm::omp::Clause::OMPC_final)) {
-        context_.Say(GetContext().clauseSource,
-            "If a DETACH clause appears on a directive, then the encountering task must not be a FINAL task"_err_en_US);
-      }
+      unsigned version{context_.langOptions().OpenMPVersion};
+      if (version == 50 || version == 51) {
+        // OpenMP 5.0: 2.10.1 Task construct restrictions
+        CheckNotAllowedIfClause(
+            llvm::omp::Clause::OMPC_detach, {llvm::omp::Clause::OMPC_mergeable});
+      } else if (version >= 52) {
+        // OpenMP 5.2: 12.5.2 Detach construct restrictions
+        if (FindClause(llvm::omp::Clause::OMPC_final)) {
+          context_.Say(GetContext().clauseSource,
+              "If a DETACH clause appears on a directive, then the encountering task must not be a FINAL task"_err_en_US);
+        }
 
-      const auto &detach{
-          std::get<parser::OmpClause::Detach>(detachClause->u)};
-      if (const auto *name{parser::Unwrap<parser::Name>(detach.v.v)}) {
-        if (name->symbol) {
-          Symbol *eventHandleSym{name->symbol->GetUltimate()};
-          auto checkVarAppearsInDataEnvClause = [&](const parser::OmpObjectList
-                                                        &objs,
-                                                    std::string clause) {
-            for (const auto &obj : objs.v) {
-              if (const parser::Name *objName{
-                      parser::Unwrap<parser::Name>(obj)}) {
-                if (objName->symbol->GetUltimate() == eventHandleSym) {
-                  context_.Say(GetContext().clauseSource,
-                      "A variable: `%s` that appears in a DETACH clause cannot appear on %s clause on the same construct"_err_en_US,
-                      objName->source, clause);
+        const auto &detach{
+            std::get<parser::OmpClause::Detach>(detachClause->u)};
+        if (const auto *name{parser::Unwrap<parser::Name>(detach.v.v)}) {
+          if (name->symbol) {
+            Symbol *eventHandleSym{name->symbol};
+            auto checkVarAppearsInDataEnvClause = [&](const parser::OmpObjectList
+                                                          &objs,
+                                                      std::string clause) {
+              for (const auto &obj : objs.v) {
+                if (const parser::Name *objName{
+                        parser::Unwrap<parser::Name>(obj)}) {
+                  if (&objName->symbol->GetUltimate() == eventHandleSym) {
+                    context_.Say(GetContext().clauseSource,
+                        "A variable: `%s` that appears in a DETACH clause cannot appear on %s clause on the same construct"_err_en_US,
+                        objName->source, clause);
+                  }
                 }
               }
+            };
+            if (auto *dataEnvClause{
+                    FindClause(llvm::omp::Clause::OMPC_private)}) {
+              const auto &pClause{
+                  std::get<parser::OmpClause::Private>(dataEnvClause->u)};
+              checkVarAppearsInDataEnvClause(pClause.v, "PRIVATE");
+            } else if (auto *dataEnvClause{
+                          FindClause(llvm::omp::Clause::OMPC_firstprivate)}) {
+              const auto &fpClause{
+                  std::get<parser::OmpClause::Firstprivate>(dataEnvClause->u)};
+              checkVarAppearsInDataEnvClause(fpClause.v, "FIRSTPRIVATE");
+            } else if (auto *dataEnvClause{
+                          FindClause(llvm::omp::Clause::OMPC_in_reduction)}) {
+              const auto &irClause{
+                  std::get<parser::OmpClause::InReduction>(dataEnvClause->u)};
+              checkVarAppearsInDataEnvClause(
+                  std::get<parser::OmpObjectList>(irClause.v.t), "IN_REDUCTION");
             }
-          };
-          if (auto *dataEnvClause{
-                  FindClause(llvm::omp::Clause::OMPC_private)}) {
-            const auto &pClause{
-                std::get<parser::OmpClause::Private>(dataEnvClause->u)};
-            checkVarAppearsInDataEnvClause(pClause.v, "PRIVATE");
-          } else if (auto *dataEnvClause{
-                         FindClause(llvm::omp::Clause::OMPC_firstprivate)}) {
-            const auto &fpClause{
-                std::get<parser::OmpClause::Firstprivate>(dataEnvClause->u)};
-            checkVarAppearsInDataEnvClause(fpClause.v, "FIRSTPRIVATE");
-          } else if (auto *dataEnvClause{
-                         FindClause(llvm::omp::Clause::OMPC_in_reduction)}) {
-            const auto &irClause{
-                std::get<parser::OmpClause::InReduction>(dataEnvClause->u)};
-            checkVarAppearsInDataEnvClause(
-                std::get<parser::OmpObjectList>(irClause.v.t), "IN_REDUCTION");
           }
         }
       }
@@ -3804,23 +3807,25 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Linear &x) {
 }
 
 void OmpStructureChecker::Enter(const parser::OmpClause::Detach &x) {
-  // OpenMP 5.0: Task construct restrictions
   CheckAllowedClause(llvm::omp::Clause::OMPC_detach);
+  unsigned version{context_.langOptions().OpenMPVersion};
+  // OpenMP 5.2: 12.5.2 Detach clause restrictions
+  if (version >= 52) {
+    CheckIsVarPartOfAnotherVar(GetContext().clauseSource, x.v.v, "DETACH");
+  }
 
-  // OpenMP 5.2: Detach clause restrictions
-  CheckIsVarPartOfAnotherVar(GetContext().clauseSource, x.v.v, "DETACH");
   if (const auto *name{parser::Unwrap<parser::Name>(x.v.v)}) {
     if (name->symbol) {
-      if (IsPointer(*name->symbol)) {
+      if (version >= 52 && IsPointer(*name->symbol)) {
         context_.Say(GetContext().clauseSource,
             "The event-handle: `%s` must not have the POINTER attribute"_err_en_US,
             name->ToString());
       }
-    }
-    if (!name->symbol->GetType()->IsNumeric(TypeCategory::Integer)) {
-      context_.Say(GetContext().clauseSource,
-          "The event-handle: `%s` must be of type integer(kind=omp_event_handle_kind)"_err_en_US,
-          name->ToString());
+      if (!name->symbol->GetType()->IsNumeric(TypeCategory::Integer)) {
+        context_.Say(GetContext().clauseSource,
+            "The event-handle: `%s` must be of type integer(kind=omp_event_handle_kind)"_err_en_US,
+            name->ToString());
+      }
     }
   }
 }
diff --git a/flang/test/Semantics/OpenMP/detach01.f90 b/flang/test/Semantics/OpenMP/detach01.f90
index 7ba2888be9237f..ea8208c022ef19 100644
--- a/flang/test/Semantics/OpenMP/detach01.f90
+++ b/flang/test/Semantics/OpenMP/detach01.f90
@@ -1,21 +1,19 @@
 ! REQUIRES: openmp_runtime
-! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=50
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=52
 
-! OpenMP Version 5.2
-! Various checks for DETACH Clause (12.5.2)
+! OpenMP Version 5.2: 12.5.2
+! Various checks for DETACH Clause
 
-program test_detach
-    use omp_lib
+program detach01
+    use omp_lib, only: omp_event_handle_kind
     implicit none
     real                                    :: e, x
     integer(omp_event_handle_kind)          :: event_01, event_02(2)
     integer(omp_event_handle_kind), pointer :: event_03
 
-
     type :: t
         integer(omp_event_handle_kind) :: event
     end type
-
     type(t) :: t_01
 
     !ERROR: The event-handle: `e` must be of type integer(kind=omp_event_handle_kind)
@@ -23,16 +21,6 @@ program test_detach
         x = x + 1
     !$omp end task
 
-    !ERROR: At most one DETACH clause can appear on the TASK directive
-    !$omp task detach(event_01) detach(event_01)
-        x = x + 1
-    !$omp end task
-
-    !ERROR: Clause MERGEABLE is not allowed if clause DETACH appears on the TASK directive
-    !$omp task detach(event_01) mergeable
-        x = x + 1
-    !$omp end task
-
     !ERROR: If a DETACH clause appears on a directive, then the encountering task must not be a FINAL task
     !$omp task detach(event_01) final(.false.)
         x = x + 1
diff --git a/flang/test/Semantics/OpenMP/detach02.f90 b/flang/test/Semantics/OpenMP/detach02.f90
new file mode 100644
index 00000000000000..13042339763512
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/detach02.f90
@@ -0,0 +1,22 @@
+! REQUIRES: openmp_runtime
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=50
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=51
+
+! OpenMP Version 5.0: 2.10.1
+! Various checks for DETACH Clause
+
+program detach02
+    use omp_lib, only: omp_event_handle_kind
+    integer(omp_event_handle_kind)          :: event_01, event_02
+
+    !TODO: Throw following error for the versions 5.0 and 5.1
+    !ERR: At most one DETACH clause can appear on the TASK directive
+    !!$omp task detach(event_01) detach(event_02)
+    !    x = x + 1
+    !!$omp end task
+
+    !ERROR: Clause MERGEABLE is not allowed if clause DETACH appears on the TASK directive
+    !$omp task detach(event_01) mergeable
+        x = x + 1
+    !$omp end task
+end program
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index aec80decf60395..e36eb77cefe7e3 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -1090,6 +1090,7 @@ def OMP_Task : Directive<"task"> {
     VersionedClause<OMPC_Affinity, 50>,
     VersionedClause<OMPC_Allocate>,
     VersionedClause<OMPC_Depend>,
+    VersionedClause<OMPC_Detach, 50>,
     VersionedClause<OMPC_FirstPrivate>,
     VersionedClause<OMPC_InReduction>,
     VersionedClause<OMPC_Mergeable>,
@@ -1099,7 +1100,6 @@ def OMP_Task : Directive<"task"> {
   ];
   let allowedOnceClauses = [
     VersionedClause<OMPC_Default>,
-    VersionedClause<OMPC_Detach, 50>,
     VersionedClause<OMPC_Final>,
     VersionedClause<OMPC_If>,
     VersionedClause<OMPC_Priority>,

>From 019a24c814b226352a13c18b840cc22a9b28ec7e Mon Sep 17 00:00:00 2001
From: Thirumalai-Shaktivel <thirumalaishaktivel at gmail.com>
Date: Thu, 26 Dec 2024 09:26:33 +0000
Subject: [PATCH 6/6] Fix formatting

---
 flang/lib/Semantics/check-omp-structure.cpp | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 41ed858ed650fd..7ca695acc74b3e 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2738,8 +2738,8 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
       unsigned version{context_.langOptions().OpenMPVersion};
       if (version == 50 || version == 51) {
         // OpenMP 5.0: 2.10.1 Task construct restrictions
-        CheckNotAllowedIfClause(
-            llvm::omp::Clause::OMPC_detach, {llvm::omp::Clause::OMPC_mergeable});
+        CheckNotAllowedIfClause(llvm::omp::Clause::OMPC_detach,
+            {llvm::omp::Clause::OMPC_mergeable});
       } else if (version >= 52) {
         // OpenMP 5.2: 12.5.2 Detach construct restrictions
         if (FindClause(llvm::omp::Clause::OMPC_final)) {
@@ -2752,12 +2752,12 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
         if (const auto *name{parser::Unwrap<parser::Name>(detach.v.v)}) {
           if (name->symbol) {
             Symbol *eventHandleSym{name->symbol};
-            auto checkVarAppearsInDataEnvClause = [&](const parser::OmpObjectList
-                                                          &objs,
+            auto checkVarAppearsInDataEnvClause = [&](const parser::
+                                                          OmpObjectList &objs,
                                                       std::string clause) {
               for (const auto &obj : objs.v) {
-                if (const parser::Name *objName{
-                        parser::Unwrap<parser::Name>(obj)}) {
+                if (const parser::Name *
+                    objName{parser::Unwrap<parser::Name>(obj)}) {
                   if (&objName->symbol->GetUltimate() == eventHandleSym) {
                     context_.Say(GetContext().clauseSource,
                         "A variable: `%s` that appears in a DETACH clause cannot appear on %s clause on the same construct"_err_en_US,
@@ -2772,16 +2772,17 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
                   std::get<parser::OmpClause::Private>(dataEnvClause->u)};
               checkVarAppearsInDataEnvClause(pClause.v, "PRIVATE");
             } else if (auto *dataEnvClause{
-                          FindClause(llvm::omp::Clause::OMPC_firstprivate)}) {
+                           FindClause(llvm::omp::Clause::OMPC_firstprivate)}) {
               const auto &fpClause{
                   std::get<parser::OmpClause::Firstprivate>(dataEnvClause->u)};
               checkVarAppearsInDataEnvClause(fpClause.v, "FIRSTPRIVATE");
             } else if (auto *dataEnvClause{
-                          FindClause(llvm::omp::Clause::OMPC_in_reduction)}) {
+                           FindClause(llvm::omp::Clause::OMPC_in_reduction)}) {
               const auto &irClause{
                   std::get<parser::OmpClause::InReduction>(dataEnvClause->u)};
               checkVarAppearsInDataEnvClause(
-                  std::get<parser::OmpObjectList>(irClause.v.t), "IN_REDUCTION");
+                  std::get<parser::OmpObjectList>(irClause.v.t),
+                  "IN_REDUCTION");
             }
           }
         }



More information about the flang-commits mailing list