Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Flang][OpenMP] Add LLVM translation support for UNTIED clause in Task #121052

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Thirumalai-Shaktivel
Copy link
Member

@Thirumalai-Shaktivel Thirumalai-Shaktivel commented Dec 24, 2024

Implementation details:
The UNTIED clause is recognized by setting the flag=0 for the default
case or performing logical OR to flag if other clauses are specified,
and this flag is passed as an argument to the __kmpc_omp_task_alloc
runtime call.

Resubmitting the PR with fix for the failure, as it was reverted here: 927a70d
Merged here: #115283

@llvmbot
Copy link
Member

llvmbot commented Dec 24, 2024

@llvm/pr-subscribers-mlir-openmp
@llvm/pr-subscribers-mlir-llvm
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-flang-fir-hlfir

Author: Thirumalai Shaktivel (Thirumalai-Shaktivel)

Changes

Implementation details:
The UNTIED clause is recognized by setting the flag=0 for the default
case or performing logical OR to flag if other clauses are specified,
and this flag is passed as an argument to the __kmpc_omp_task_alloc
runtime call.

Resubmitting the PR with fix for the failure, as it was reverted here: 927a70d


Full diff: https://github.com/llvm/llvm-project/pull/121052.diff

8 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+1)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+35)
  • (removed) flang/test/Lower/OpenMP/Todo/task_untied.f90 (-13)
  • (modified) flang/test/Lower/OpenMP/task.f90 (+17)
  • (added) flang/test/Semantics/OpenMP/task-united01.f90 (+27)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+4-1)
  • (modified) mlir/test/Target/LLVMIR/openmp-llvm.mlir (+12)
  • (modified) mlir/test/Target/LLVMIR/openmp-todo.mlir (+13-11)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index b07e89d201d198..4de5ecf187a4cb 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2867,6 +2867,7 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
         !std::holds_alternative<clause::UseDevicePtr>(clause.u) &&
         !std::holds_alternative<clause::InReduction>(clause.u) &&
         !std::holds_alternative<clause::Mergeable>(clause.u) &&
+        !std::holds_alternative<clause::Untied>(clause.u) &&
         !std::holds_alternative<clause::TaskReduction>(clause.u) &&
         !std::holds_alternative<clause::Detach>(clause.u)) {
       std::string name =
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 95b962f5daf57c..882f28ea1dd2f7 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -213,6 +213,31 @@ class AssociatedLoopChecker {
   std::map<std::string, std::int64_t> constructNamesAndLevels_;
 };
 
+// `OmpDesignatorChecker` is used to check if the designator
+// can appear within the OpenMP construct
+class OmpDesignatorChecker {
+public:
+  OmpDesignatorChecker(SemanticsContext &context, parser::CharBlock source)
+      : context_{context}, source_{source} {}
+
+  template <typename T> bool Pre(const T &) { return true; }
+  template <typename T> void Post(const T &) {}
+
+  bool Pre(const parser::Name &name) {
+    if (name.symbol->test(Symbol::Flag::OmpThreadprivate)) {
+      // OpenMP 6.0: 7.3 threadprivate directive restriction
+      context_.Say(source_,
+          "A THREADPRIVATE variable `%s` cannot appear in a UNTIED TASK region"_err_en_US,
+          name.source);
+    }
+    return true;
+  }
+
+private:
+  SemanticsContext &context_;
+  parser::CharBlock source_;
+};
+
 bool OmpStructureChecker::CheckAllowedClause(llvmOmpClause clause) {
   unsigned version{context_.langOptions().OpenMPVersion};
   DirectiveContext &dirCtx = GetContext();
@@ -1164,6 +1189,16 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
     HasInvalidWorksharingNesting(
         beginDir.source, llvm::omp::nestedWorkshareErrSet);
     break;
+  case llvm::omp::Directive::OMPD_task: {
+    const auto &clauses{std::get<parser::OmpClauseList>(beginBlockDir.t)};
+    for (const auto &clause : clauses.v) {
+      if (std::get_if<parser::OmpClause::Untied>(&clause.u)) {
+        OmpDesignatorChecker ompDesignatorChecker{context_, beginDir.source};
+        parser::Walk(block, ompDesignatorChecker);
+      }
+    }
+    break;
+  }
   default:
     break;
   }
diff --git a/flang/test/Lower/OpenMP/Todo/task_untied.f90 b/flang/test/Lower/OpenMP/Todo/task_untied.f90
deleted file mode 100644
index 87d242ba3e9d21..00000000000000
--- a/flang/test/Lower/OpenMP/Todo/task_untied.f90
+++ /dev/null
@@ -1,13 +0,0 @@
-! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-
-!===============================================================================
-! `untied` clause
-!===============================================================================
-
-! CHECK: not yet implemented: UNTIED clause is not implemented yet
-subroutine omp_task_untied()
-  !$omp task untied
-  call foo()
-  !$omp end task
-end subroutine omp_task_untied
diff --git a/flang/test/Lower/OpenMP/task.f90 b/flang/test/Lower/OpenMP/task.f90
index 6e525a044b011e..a8cc16c540c9c8 100644
--- a/flang/test/Lower/OpenMP/task.f90
+++ b/flang/test/Lower/OpenMP/task.f90
@@ -235,6 +235,10 @@ subroutine task_multiple_clauses()
   !$omp end task
 end subroutine task_multiple_clauses
 
+!===============================================================================
+! `mergeable` clause
+!===============================================================================
+
 subroutine task_mergeable()
 !CHECK: omp.task mergeable {
 !CHECK: omp.terminator
@@ -242,3 +246,16 @@ subroutine task_mergeable()
  !$omp task mergeable
  !$omp end task
 end subroutine
+
+!===============================================================================
+! `untied` clause
+!===============================================================================
+
+!CHECK-LABEL: func.func @_QPomp_task_untied() {
+subroutine omp_task_untied()
+  !CHECK: omp.task untied {
+  !$omp task untied
+    call foo()
+  !CHECK: omp.terminator
+  !$omp end task
+end subroutine omp_task_untied
diff --git a/flang/test/Semantics/OpenMP/task-united01.f90 b/flang/test/Semantics/OpenMP/task-united01.f90
new file mode 100644
index 00000000000000..3a35f76d7c834f
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/task-united01.f90
@@ -0,0 +1,27 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags
+!
+! OpenMP 5.2: 5.2 threadprivate directive restriction
+
+subroutine task_united01()
+    integer, save :: var_01, var_02(2)
+    real          :: var_03
+    common /c/ var_03
+
+    !$omp threadprivate(var_01, var_02)
+    !$omp threadprivate(/c/)
+
+    ! ERROR: A THREADPRIVATE variable `var_01` cannot appear in a UNTIED TASK region
+    ! ERROR: A THREADPRIVATE variable `var_02` cannot appear in a UNTIED TASK region
+    ! ERROR: A THREADPRIVATE variable `var_01` cannot appear in a UNTIED TASK region
+    !$omp task untied
+        var_01    = 10
+        var_02(1) = sum([var_01, 20])
+    !$omp end task
+
+    ! ERROR: A THREADPRIVATE variable `var_02` cannot appear in a UNTIED TASK region
+    ! ERROR: A THREADPRIVATE variable `var_03` cannot appear in a UNTIED TASK region
+    !$omp task untied
+        var_02(2) = product(var_02)
+        var_03    = 3.14
+    !$omp end task
+end subroutine task_united01
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 060113c4123241..d591c98a5497f8 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -258,7 +258,6 @@ static LogicalResult checkImplementationStatus(Operation &op) {
         checkAllocate(op, result);
         checkInReduction(op, result);
         checkPriority(op, result);
-        checkUntied(op, result);
       })
       .Case([&](omp::TaskgroupOp op) {
         checkAllocate(op, result);
@@ -268,6 +267,10 @@ static LogicalResult checkImplementationStatus(Operation &op) {
         checkDepend(op, result);
         checkNowait(op, result);
       })
+      .Case([&](omp::TaskloopOp op) {
+        // TODO: Add other clauses check
+        checkUntied(op, result);
+      })
       .Case([&](omp::WsloopOp op) {
         checkAllocate(op, result);
         checkLinear(op, result);
diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
index 5f8bdf8afdf783..8903bf283b5c75 100644
--- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -3020,6 +3020,18 @@ module attributes {omp.is_target_device = true} {
 
 // -----
 
+llvm.func @omp_task_untied() {
+  // The third argument is 0: which signifies the united task
+  // CHECK: {{.*}} = call ptr @__kmpc_omp_task_alloc(ptr @1, i32 %{{.*}}, i32 0,
+  // CHECK-SAME:     i64 40, i64 0, ptr @{{.*}})
+  omp.task untied {
+        omp.terminator
+  }
+  llvm.return
+}
+
+// -----
+
 // Third argument is 5: essentially (4 || 1)
 // signifying this task is TIED and MERGEABLE
 
diff --git a/mlir/test/Target/LLVMIR/openmp-todo.mlir b/mlir/test/Target/LLVMIR/openmp-todo.mlir
index 8f3e466cfbbeb6..8ae795ec1ec6b0 100644
--- a/mlir/test/Target/LLVMIR/openmp-todo.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir
@@ -440,17 +440,6 @@ llvm.func @task_priority(%x : i32) {
 
 // -----
 
-llvm.func @task_untied() {
-  // expected-error@below {{not yet implemented: Unhandled clause untied in omp.task operation}}
-  // expected-error@below {{LLVM Translation failed for operation: omp.task}}
-  omp.task untied {
-    omp.terminator
-  }
-  llvm.return
-}
-
-// -----
-
 llvm.func @taskgroup_allocate(%x : !llvm.ptr) {
   // expected-error@below {{not yet implemented: Unhandled clause allocate in omp.taskgroup operation}}
   // expected-error@below {{LLVM Translation failed for operation: omp.taskgroup}}
@@ -503,6 +492,19 @@ llvm.func @taskloop(%lb : i32, %ub : i32, %step : i32) {
 
 // -----
 
+llvm.func @taskloop_untied(%lb : i32, %ub : i32, %step : i32) {
+  // expected-error@below {{not yet implemented: omp.taskloop}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.taskloop}}
+  omp.taskloop untied {
+    omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {
+      omp.yield
+    }
+  }
+  llvm.return
+}
+
+// -----
+
 llvm.func @taskwait_depend(%x: !llvm.ptr) {
   // expected-error@below {{not yet implemented: Unhandled clause depend in omp.taskwait operation}}
   // expected-error@below {{LLVM Translation failed for operation: omp.taskwait}}

@llvmbot
Copy link
Member

llvmbot commented Dec 24, 2024

@llvm/pr-subscribers-flang-openmp

Author: Thirumalai Shaktivel (Thirumalai-Shaktivel)

Changes

Implementation details:
The UNTIED clause is recognized by setting the flag=0 for the default
case or performing logical OR to flag if other clauses are specified,
and this flag is passed as an argument to the __kmpc_omp_task_alloc
runtime call.

Resubmitting the PR with fix for the failure, as it was reverted here: 927a70d


Full diff: https://github.com/llvm/llvm-project/pull/121052.diff

8 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+1)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+35)
  • (removed) flang/test/Lower/OpenMP/Todo/task_untied.f90 (-13)
  • (modified) flang/test/Lower/OpenMP/task.f90 (+17)
  • (added) flang/test/Semantics/OpenMP/task-united01.f90 (+27)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+4-1)
  • (modified) mlir/test/Target/LLVMIR/openmp-llvm.mlir (+12)
  • (modified) mlir/test/Target/LLVMIR/openmp-todo.mlir (+13-11)
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index b07e89d201d198..4de5ecf187a4cb 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2867,6 +2867,7 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
         !std::holds_alternative<clause::UseDevicePtr>(clause.u) &&
         !std::holds_alternative<clause::InReduction>(clause.u) &&
         !std::holds_alternative<clause::Mergeable>(clause.u) &&
+        !std::holds_alternative<clause::Untied>(clause.u) &&
         !std::holds_alternative<clause::TaskReduction>(clause.u) &&
         !std::holds_alternative<clause::Detach>(clause.u)) {
       std::string name =
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 95b962f5daf57c..882f28ea1dd2f7 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -213,6 +213,31 @@ class AssociatedLoopChecker {
   std::map<std::string, std::int64_t> constructNamesAndLevels_;
 };
 
+// `OmpDesignatorChecker` is used to check if the designator
+// can appear within the OpenMP construct
+class OmpDesignatorChecker {
+public:
+  OmpDesignatorChecker(SemanticsContext &context, parser::CharBlock source)
+      : context_{context}, source_{source} {}
+
+  template <typename T> bool Pre(const T &) { return true; }
+  template <typename T> void Post(const T &) {}
+
+  bool Pre(const parser::Name &name) {
+    if (name.symbol->test(Symbol::Flag::OmpThreadprivate)) {
+      // OpenMP 6.0: 7.3 threadprivate directive restriction
+      context_.Say(source_,
+          "A THREADPRIVATE variable `%s` cannot appear in a UNTIED TASK region"_err_en_US,
+          name.source);
+    }
+    return true;
+  }
+
+private:
+  SemanticsContext &context_;
+  parser::CharBlock source_;
+};
+
 bool OmpStructureChecker::CheckAllowedClause(llvmOmpClause clause) {
   unsigned version{context_.langOptions().OpenMPVersion};
   DirectiveContext &dirCtx = GetContext();
@@ -1164,6 +1189,16 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
     HasInvalidWorksharingNesting(
         beginDir.source, llvm::omp::nestedWorkshareErrSet);
     break;
+  case llvm::omp::Directive::OMPD_task: {
+    const auto &clauses{std::get<parser::OmpClauseList>(beginBlockDir.t)};
+    for (const auto &clause : clauses.v) {
+      if (std::get_if<parser::OmpClause::Untied>(&clause.u)) {
+        OmpDesignatorChecker ompDesignatorChecker{context_, beginDir.source};
+        parser::Walk(block, ompDesignatorChecker);
+      }
+    }
+    break;
+  }
   default:
     break;
   }
diff --git a/flang/test/Lower/OpenMP/Todo/task_untied.f90 b/flang/test/Lower/OpenMP/Todo/task_untied.f90
deleted file mode 100644
index 87d242ba3e9d21..00000000000000
--- a/flang/test/Lower/OpenMP/Todo/task_untied.f90
+++ /dev/null
@@ -1,13 +0,0 @@
-! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-
-!===============================================================================
-! `untied` clause
-!===============================================================================
-
-! CHECK: not yet implemented: UNTIED clause is not implemented yet
-subroutine omp_task_untied()
-  !$omp task untied
-  call foo()
-  !$omp end task
-end subroutine omp_task_untied
diff --git a/flang/test/Lower/OpenMP/task.f90 b/flang/test/Lower/OpenMP/task.f90
index 6e525a044b011e..a8cc16c540c9c8 100644
--- a/flang/test/Lower/OpenMP/task.f90
+++ b/flang/test/Lower/OpenMP/task.f90
@@ -235,6 +235,10 @@ subroutine task_multiple_clauses()
   !$omp end task
 end subroutine task_multiple_clauses
 
+!===============================================================================
+! `mergeable` clause
+!===============================================================================
+
 subroutine task_mergeable()
 !CHECK: omp.task mergeable {
 !CHECK: omp.terminator
@@ -242,3 +246,16 @@ subroutine task_mergeable()
  !$omp task mergeable
  !$omp end task
 end subroutine
+
+!===============================================================================
+! `untied` clause
+!===============================================================================
+
+!CHECK-LABEL: func.func @_QPomp_task_untied() {
+subroutine omp_task_untied()
+  !CHECK: omp.task untied {
+  !$omp task untied
+    call foo()
+  !CHECK: omp.terminator
+  !$omp end task
+end subroutine omp_task_untied
diff --git a/flang/test/Semantics/OpenMP/task-united01.f90 b/flang/test/Semantics/OpenMP/task-united01.f90
new file mode 100644
index 00000000000000..3a35f76d7c834f
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/task-united01.f90
@@ -0,0 +1,27 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags
+!
+! OpenMP 5.2: 5.2 threadprivate directive restriction
+
+subroutine task_united01()
+    integer, save :: var_01, var_02(2)
+    real          :: var_03
+    common /c/ var_03
+
+    !$omp threadprivate(var_01, var_02)
+    !$omp threadprivate(/c/)
+
+    ! ERROR: A THREADPRIVATE variable `var_01` cannot appear in a UNTIED TASK region
+    ! ERROR: A THREADPRIVATE variable `var_02` cannot appear in a UNTIED TASK region
+    ! ERROR: A THREADPRIVATE variable `var_01` cannot appear in a UNTIED TASK region
+    !$omp task untied
+        var_01    = 10
+        var_02(1) = sum([var_01, 20])
+    !$omp end task
+
+    ! ERROR: A THREADPRIVATE variable `var_02` cannot appear in a UNTIED TASK region
+    ! ERROR: A THREADPRIVATE variable `var_03` cannot appear in a UNTIED TASK region
+    !$omp task untied
+        var_02(2) = product(var_02)
+        var_03    = 3.14
+    !$omp end task
+end subroutine task_united01
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 060113c4123241..d591c98a5497f8 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -258,7 +258,6 @@ static LogicalResult checkImplementationStatus(Operation &op) {
         checkAllocate(op, result);
         checkInReduction(op, result);
         checkPriority(op, result);
-        checkUntied(op, result);
       })
       .Case([&](omp::TaskgroupOp op) {
         checkAllocate(op, result);
@@ -268,6 +267,10 @@ static LogicalResult checkImplementationStatus(Operation &op) {
         checkDepend(op, result);
         checkNowait(op, result);
       })
+      .Case([&](omp::TaskloopOp op) {
+        // TODO: Add other clauses check
+        checkUntied(op, result);
+      })
       .Case([&](omp::WsloopOp op) {
         checkAllocate(op, result);
         checkLinear(op, result);
diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
index 5f8bdf8afdf783..8903bf283b5c75 100644
--- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -3020,6 +3020,18 @@ module attributes {omp.is_target_device = true} {
 
 // -----
 
+llvm.func @omp_task_untied() {
+  // The third argument is 0: which signifies the united task
+  // CHECK: {{.*}} = call ptr @__kmpc_omp_task_alloc(ptr @1, i32 %{{.*}}, i32 0,
+  // CHECK-SAME:     i64 40, i64 0, ptr @{{.*}})
+  omp.task untied {
+        omp.terminator
+  }
+  llvm.return
+}
+
+// -----
+
 // Third argument is 5: essentially (4 || 1)
 // signifying this task is TIED and MERGEABLE
 
diff --git a/mlir/test/Target/LLVMIR/openmp-todo.mlir b/mlir/test/Target/LLVMIR/openmp-todo.mlir
index 8f3e466cfbbeb6..8ae795ec1ec6b0 100644
--- a/mlir/test/Target/LLVMIR/openmp-todo.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir
@@ -440,17 +440,6 @@ llvm.func @task_priority(%x : i32) {
 
 // -----
 
-llvm.func @task_untied() {
-  // expected-error@below {{not yet implemented: Unhandled clause untied in omp.task operation}}
-  // expected-error@below {{LLVM Translation failed for operation: omp.task}}
-  omp.task untied {
-    omp.terminator
-  }
-  llvm.return
-}
-
-// -----
-
 llvm.func @taskgroup_allocate(%x : !llvm.ptr) {
   // expected-error@below {{not yet implemented: Unhandled clause allocate in omp.taskgroup operation}}
   // expected-error@below {{LLVM Translation failed for operation: omp.taskgroup}}
@@ -503,6 +492,19 @@ llvm.func @taskloop(%lb : i32, %ub : i32, %step : i32) {
 
 // -----
 
+llvm.func @taskloop_untied(%lb : i32, %ub : i32, %step : i32) {
+  // expected-error@below {{not yet implemented: omp.taskloop}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.taskloop}}
+  omp.taskloop untied {
+    omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {
+      omp.yield
+    }
+  }
+  llvm.return
+}
+
+// -----
+
 llvm.func @taskwait_depend(%x: !llvm.ptr) {
   // expected-error@below {{not yet implemented: Unhandled clause depend in omp.taskwait operation}}
   // expected-error@below {{LLVM Translation failed for operation: omp.taskwait}}

@Thirumalai-Shaktivel
Copy link
Member Author

The test llvm-test-suite/Fortran/gfortran/regression/gomp/pr44085.f90, throws an error using GFortran, but Flang-new doesn't. So, I added a semantic check for that.

@Thirumalai-Shaktivel Thirumalai-Shaktivel marked this pull request as draft December 24, 2024 13:41
@Thirumalai-Shaktivel
Copy link
Member Author

Thirumalai-Shaktivel commented Dec 24, 2024

There is some failure (It is passing on my remote machine). I will check and report back.

@Thirumalai-Shaktivel
Copy link
Member Author

Thirumalai-Shaktivel commented Dec 24, 2024

I don't know how, but I get the following locally:

$ flang-new test/Semantics/OpenMP/task-united01.f90 -fopenmp
flang-20: warning: OpenMP support in flang is still experimental [-Wexperimental-option]
error: Semantic errors in test/Semantics/OpenMP/task-united01.f90
./test/Semantics/OpenMP/task-united01.f90:16:11: error: A THREADPRIVATE variable `var_01` cannot appear in a UNTIED TASK region
      !$omp task untied
            ^^^^
./test/Semantics/OpenMP/task-united01.f90:16:11: error: A THREADPRIVATE variable `var_02` cannot appear in a UNTIED TASK region
      !$omp task untied
            ^^^^
./test/Semantics/OpenMP/task-united01.f90:16:11: error: A THREADPRIVATE variable `var_01` cannot appear in a UNTIED TASK region
      !$omp task untied
            ^^^^
./test/Semantics/OpenMP/task-united01.f90:23:11: error: A THREADPRIVATE variable `var_02` cannot appear in a UNTIED TASK region
      !$omp task untied
            ^^^^
./test/Semantics/OpenMP/task-united01.f90:23:11: error: A THREADPRIVATE variable `var_03` cannot appear in a UNTIED TASK region
      !$omp task untied
            ^^^^

The check-flang works as well. It only fails here.

Copy link

github-actions bot commented Dec 24, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Implementation details:
The UNTIED clause is recognized by setting the flag=0 for the default
case or performing logical OR to flag if other clauses are specified,
and this flag is passed as an argument to the `__kmpc_omp_task_alloc`
runtime call.
@NimishMishra
Copy link
Contributor

Does !REQUIRES: openmp_runtime help?

@Thirumalai-Shaktivel
Copy link
Member Author

It seems to work, thank you!

@Thirumalai-Shaktivel Thirumalai-Shaktivel marked this pull request as ready for review December 24, 2024 15:43
Copy link
Contributor

@NimishMishra NimishMishra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM. Minor nits.

@@ -3020,6 +3020,18 @@ module attributes {omp.is_target_device = true} {

// -----

llvm.func @omp_task_untied() {
// The third argument is 0: which signifies the united task
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"united" -> "untied"

@@ -0,0 +1,29 @@
! REQUIRES: openmp_runtime
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File name: "task-united01.f90" -> "task-untied01.f90" ?

!
! OpenMP 5.2: 5.2 threadprivate directive restriction

subroutine task_united01()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

task_untied01 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

if (name.symbol->test(Symbol::Flag::OmpThreadprivate)) {
// OpenMP 5.2: 5.2 threadprivate directive restriction
context_.Say(name.source,
"A THREADPRIVATE variable `%s` cannot appear in a UNTIED TASK region"_err_en_US,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: "an UNTIED TASK region"

@Thirumalai-Shaktivel
Copy link
Member Author

Done, thanks for the review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants