[Flang][OpenMP] Support depend clause for task construct, excluding array sections

[psoni2628]

Summary

This patch adds support for the OpenMP 4.0 depend clause for the task
construct, excluding array sections, to Flang lowering from parse-tree
to MLIR.

[psoni2628]

• Use TODO instead of llvm_unreachable for array sections

[kiranchandramohan]

Looks OK.

Could you check why the following is crashing at runtime?

subroutine foo()
  implicit none
  integer::x,y
  x=0
  y=2
  !$omp task depend(out:x) shared(x)
   x=x+1
   !$omp end task
  !$omp task depend(out:y) shared(y)
   y=y+1
   !$omp end task
   !$omp task depend(in:x,y) shared(x,y)
   y=y-x
   !$omp end task
   !$omp taskwait
   print*,"y=",y
end subroutine foo

program p
implicit none
!$omp parallel
!$omp single
call foo()
!$omp end single
!$omp end parallel
end program p

flang/test/Lower/OpenMP/task.f90

107

Please add a few more tests to,
-> cover all the dependency types.
-> cover a few FIR types : allocatable, character etc
-> have more than one depend clause for the same task
-> have more than one variable in a single depend clause

[psoni2628]

In D146766#4218118, @kiranchandramohan wrote:

Looks OK.

Could you check why the following is crashing at runtime?

subroutine foo()
  implicit none
  integer::x,y
  x=0
  y=2
  !$omp task depend(out:x) shared(x)
   x=x+1
   !$omp end task
  !$omp task depend(out:y) shared(y)
   y=y+1
   !$omp end task
   !$omp task depend(in:x,y) shared(x,y)
   y=y-x
   !$omp end task
   !$omp taskwait
   print*,"y=",y
end subroutine foo

program p
implicit none
!$omp parallel
!$omp single
call foo()
!$omp end single
!$omp end parallel
end program p

I reduced the test case to the following, and it crashes for me too. Note that this does not have any depend clauses. I will debug this issue.

subroutine foo()
  implicit none
  integer::x,y
  x=0
  y=2
   !$omp task shared(x,y)
   y=x
   !$omp end task
   !$omp taskwait
   print*,"y=",y
end subroutine foo

program p
implicit none
call foo()
end program p

[kiranchandramohan]

@psoni2628 Will you have time this week or next to look into the blocking issue. I think it also shows up in https://github.com/llvm/llvm-project/issues/62013. I guess this has something to do with kmp_task_t structure that is being passed to the tasks and how the shared variables are copied into it.

[psoni2628]

In D146766#4364211, @kiranchandramohan wrote:

@psoni2628 Will you have time this week or next to look into the blocking issue. I think it also shows up in https://github.com/llvm/llvm-project/issues/62013. I guess this has something to do with kmp_task_t structure that is being passed to the tasks and how the shared variables are copied into it.

Sure, I can spend more time on it this week. However, I don't think the issue you linked is related to this one. The issue looks like a compile time crash, whereas I am dealing with a runtime crash.

[psoni2628]

In D146766#4219527, @psoni2628 wrote:

In D146766#4218118, @kiranchandramohan wrote:

Looks OK.

Could you check why the following is crashing at runtime?

subroutine foo()
  implicit none
  integer::x,y
  x=0
  y=2
  !$omp task depend(out:x) shared(x)
   x=x+1
   !$omp end task
  !$omp task depend(out:y) shared(y)
   y=y+1
   !$omp end task
   !$omp task depend(in:x,y) shared(x,y)
   y=y-x
   !$omp end task
   !$omp taskwait
   print*,"y=",y
end subroutine foo

program p
implicit none
!$omp parallel
!$omp single
call foo()
!$omp end single
!$omp end parallel
end program p

I reduced the test case to the following, and it crashes for me too. Note that this does not have any depend clauses. I will debug this issue.

subroutine foo()
  implicit none
  integer::x,y
  x=0
  y=2
   !$omp task shared(x,y)
   y=x
   !$omp end task
   !$omp taskwait
   print*,"y=",y
end subroutine foo

program p
implicit none
call foo()
end program p

This test case failure is caused by the shared clause not being implemented for the task construct. In llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp, there is the following comment from D71989: TODO: Argument - sizeof_shareds.

I don't think we can properly test the depend clause without at least 2 shared variables. Thus, we must first implement the shared clause functionality. Do you have a different opinion @kiranchandramohan ?

[psoni2628]

In D146766#4393477, @psoni2628 wrote:

In D146766#4219527, @psoni2628 wrote:

In D146766#4218118, @kiranchandramohan wrote:

Looks OK.

Could you check why the following is crashing at runtime?

subroutine foo()
  implicit none
  integer::x,y
  x=0
  y=2
  !$omp task depend(out:x) shared(x)
   x=x+1
   !$omp end task
  !$omp task depend(out:y) shared(y)
   y=y+1
   !$omp end task
   !$omp task depend(in:x,y) shared(x,y)
   y=y-x
   !$omp end task
   !$omp taskwait
   print*,"y=",y
end subroutine foo

program p
implicit none
!$omp parallel
!$omp single
call foo()
!$omp end single
!$omp end parallel
end program p

I reduced the test case to the following, and it crashes for me too. Note that this does not have any depend clauses. I will debug this issue.

subroutine foo()
  implicit none
  integer::x,y
  x=0
  y=2
   !$omp task shared(x,y)
   y=x
   !$omp end task
   !$omp taskwait
   print*,"y=",y
end subroutine foo

program p
implicit none
call foo()
end program p

This test case failure is caused by the shared clause not being implemented for the task construct. In llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp, there is the following comment from D71989: TODO: Argument - sizeof_shareds.

I don't think we can properly test the depend clause without at least 2 shared variables. Thus, we must first implement the shared clause functionality. Do you have a different opinion @kiranchandramohan ?

On second thought, I think we can write a test case that has only 1 shared variable to test the depend clause functionality. I tested the following test case below and it runs.

subroutine foo()
  implicit none
  integer::x,y
  x=0
  !$omp task  shared(x) depend(out:x)
    x=2
  !$omp end task
  !$omp task  shared(x) depend(in:x)
    x=x-1
  !$omp end task
  !$omp taskwait
  print*,"x=",x
end subroutine foo

program p
implicit none
!$omp parallel
!$omp single
  call foo()
!$omp end single
!$omp end parallel
end program p

@kiranchandramohan Do you think this test case is fine? We can land this patch and move on to fixing the shared clause.

[kiranchandramohan]

Fine with me. LG.

[psoni2628]

• Add more test cases based on Kiran's feedback

[psoni2628]

In D146766#4218118, @kiranchandramohan wrote:

Looks OK.

Could you check why the following is crashing at runtime?

subroutine foo()
  implicit none
  integer::x,y
  x=0
  y=2
  !$omp task depend(out:x) shared(x)
   x=x+1
   !$omp end task
  !$omp task depend(out:y) shared(y)
   y=y+1
   !$omp end task
   !$omp task depend(in:x,y) shared(x,y)
   y=y-x
   !$omp end task
   !$omp taskwait
   print*,"y=",y
end subroutine foo

program p
implicit none
!$omp parallel
!$omp single
call foo()
!$omp end single
!$omp end parallel
end program p

Could you please check if the added test cases are fine?

[kiranchandramohan]

Could you please check if the added test cases are fine?

Tests look OK to me.

[psoni2628]

This patch caused an unused variable ‘a’ [-Werror=unused-variable] warning in https://lab.llvm.org/buildbot/#/builders/160/builds/20500 for flang/lib/Lower/OpenMP.cpp:1197. This has been corrected in rGe0fed043662cb2410858ad82002f22e64ee552dd.

Side note: I am not sure why the build bot run for this original Differential did not complain about this warning.

[luporl]

In D146766#4399941, @psoni2628 wrote:

Side note: I am not sure why the build bot run for this original Differential did not complain about this warning.

It's probably because this build bot uses gcc instead of clang.

[psoni2628]

In D146766#4400119, @luporl wrote:

In D146766#4399941, @psoni2628 wrote:

Side note: I am not sure why the build bot run for this original Differential did not complain about this warning.

It's probably because this build bot uses gcc instead of clang.

Good to know, thanks!

flang/lib/Lower/OpenMP.cpp

case Fortran::parser::OmpProcBindClause::Type::Spread:
     break;
   case Fortran::parser::OmpProcBindClause::Type::Primary:
     pbKind = omp::ClauseProcBindKind::Primary;
     break;
   }
   return omp::ClauseProcBindKindAttr::get(firOpBuilder.getContext(), pbKind);
 }
 
+static omp::ClauseTaskDependAttr
+genDependKindAttr(fir::FirOpBuilder &firOpBuilder,
+                  const Fortran::parser::OmpClause::Depend *dependClause) {
+  omp::ClauseTaskDepend pbKind;
+  switch (
+      std::get<Fortran::parser::OmpDependenceType>(
+          std::get<Fortran::parser::OmpDependClause::InOut>(dependClause->v.u)
+              .t)
+          .v) {
+  case Fortran::parser::OmpDependenceType::Type::In:
+    pbKind = omp::ClauseTaskDepend::taskdependin;
+    break;
+  case Fortran::parser::OmpDependenceType::Type::Out:
+    pbKind = omp::ClauseTaskDepend::taskdependout;
+    break;
+  case Fortran::parser::OmpDependenceType::Type::Inout:
+    pbKind = omp::ClauseTaskDepend::taskdependinout;
+    break;
+  default:
+    llvm_unreachable("unknown parser task dependence type");
+    break;
+  }
+  return omp::ClauseTaskDependAttr::get(firOpBuilder.getContext(), pbKind);
+}
+
 /* When parallel is used in a combined construct, then use this function to
  * create the parallel operation. It handles the parallel specific clauses
  * and leaves the rest for handling at the inner operations.
  * TODO: Refactor clause handling
  */
 template <typename Directive>
 static void
 createCombinedParallelOp(Fortran::lower::AbstractConverter &converter,
genOMP(Fortran::lower::AbstractConverter &converter,
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   mlir::Location currentLocation = converter.getCurrentLocation();
 
   Fortran::lower::StatementContext stmtCtx;
   llvm::ArrayRef<mlir::Type> argTy;
   mlir::Value ifClauseOperand, numThreadsClauseOperand, finalClauseOperand,
       priorityClauseOperand;
   mlir::omp::ClauseProcBindKindAttr procBindKindAttr;
-  SmallVector<Value> allocateOperands, allocatorOperands;
+  SmallVector<Value> allocateOperands, allocatorOperands, dependOperands;
+  SmallVector<Attribute> dependTypeOperands;
   mlir::UnitAttr nowaitAttr, untiedAttr, mergeableAttr;
 
   const auto &opClauseList =
       std::get<Fortran::parser::OmpClauseList>(beginBlockDirective.t);
   for (const auto &clause : opClauseList.v) {
     if (const auto &ifClause =
             std::get_if<Fortran::parser::OmpClause::If>(&clause.u)) {
       ifClauseOperand = getIfClauseOperand(converter, stmtCtx, ifClause);
if (const auto &ifClause =
                        &clause.u)) {
       priorityClauseOperand = fir::getBase(converter.genExprValue(
           *Fortran::semantics::GetExpr(priorityClause->v), stmtCtx));
     } else if (std::get_if<Fortran::parser::OmpClause::Reduction>(&clause.u)) {
       TODO(currentLocation,
            "Reduction in OpenMP " +
                llvm::omp::getOpenMPDirectiveName(blockDirective.v) +
                " construct");
+    } else if (const auto &dependClause =
+                   std::get_if<Fortran::parser::OmpClause::Depend>(&clause.u)) {
+      const std::list<Fortran::parser::Designator> &depVal =
+          std::get<std::list<Fortran::parser::Designator>>(
+              std::get<Fortran::parser::OmpDependClause::InOut>(
+                  dependClause->v.u)
+                  .t);
+      omp::ClauseTaskDependAttr dependTypeOperand =
+          genDependKindAttr(firOpBuilder, dependClause);
+      dependTypeOperands.insert(dependTypeOperands.end(), depVal.size(),
+                                dependTypeOperand);
+      for (const Fortran::parser::Designator &ompObject : depVal) {
+        Fortran::semantics::Symbol *sym = nullptr;
+        std::visit(
+            Fortran::common::visitors{
+                [&](const Fortran::parser::DataRef &designator) {
+                  if (const Fortran::parser::Name *name =
+                          std::get_if<Fortran::parser::Name>(&designator.u)) {
+                    sym = name->symbol;
+                  } else if (const Fortran::common::Indirection<
+                                 Fortran::parser::ArrayElement> *a =
+                                 std::get_if<Fortran::common::Indirection<
+                                     Fortran::parser::ArrayElement>>(
+                                     &designator.u)) {
+                    TODO(converter.getCurrentLocation(),
+                         "array sections not supported for task depend");
+                  }
+                },
+                [&](const Fortran::parser::Substring &designator) {
+                  TODO(converter.getCurrentLocation(),
+                       "substring not supported for task depend");
+                }},
+            (ompObject).u);
+        const mlir::Value variable = converter.getSymbolAddress(*sym);
+        dependOperands.push_back(((variable)));
+      }
     } else {
       TODO(converter.getCurrentLocation(), "OpenMP Block construct clause");
     }
   }
 
   for (const auto &clause :
        std::get<Fortran::parser::OmpClauseList>(endBlockDirective.t).v) {
     if (std::get_if<Fortran::parser::OmpClause::Nowait>(&clause.u))
if (blockDirective.v == llvm::omp::OMPD_parallel) {
     auto orderedOp = firOpBuilder.create<mlir::omp::OrderedRegionOp>(
         currentLocation, /*simd=*/false);
     createBodyOfOp<omp::OrderedRegionOp>(orderedOp, converter, currentLocation,
                                          eval);
   } else if (blockDirective.v == llvm::omp::OMPD_task) {
     auto taskOp = firOpBuilder.create<mlir::omp::TaskOp>(
         currentLocation, ifClauseOperand, finalClauseOperand, untiedAttr,
         mergeableAttr, /*in_reduction_vars=*/ValueRange(),
-        /*in_reductions=*/nullptr, priorityClauseOperand, /*depends=*/nullptr,
-        /*depend_vars=*/ValueRange(), allocateOperands, allocatorOperands);
+        /*in_reductions=*/nullptr, priorityClauseOperand,
+        dependTypeOperands.empty()
+            ? nullptr
+            : mlir::ArrayAttr::get(firOpBuilder.getContext(),
+                                   dependTypeOperands),
+        dependOperands, allocateOperands, allocatorOperands);
     createBodyOfOp(taskOp, converter, currentLocation, eval, &opClauseList);
   } else if (blockDirective.v == llvm::omp::OMPD_taskgroup) {
     // TODO: Add task_reduction support
     auto taskGroupOp = firOpBuilder.create<mlir::omp::TaskGroupOp>(
         currentLocation, /*task_reduction_vars=*/ValueRange(),
         /*task_reductions=*/nullptr, allocateOperands, allocatorOperands);
     createBodyOfOp(taskGroupOp, converter, currentLocation, eval,
                    &opClauseList);

flang/test/Lower/OpenMP/task.f90

subroutine task_allocate()
   !$omp task allocate(omp_high_bw_mem_alloc: x) private(x)
   !CHECK: arith.addi
   x = x + 12
   !CHECK: omp.terminator
   !$omp end task
 end subroutine task_allocate
 
 !===============================================================================
+! `depend` clause
+!===============================================================================
+
+!CHECK-LABEL: func @_QPtask_depend
+subroutine task_depend()
+  integer :: x
+  !CHECK: omp.task depend(taskdependin -> %{{.+}} : !fir.ref<i32>) {
+  !$omp task depend(in : x)
+  !CHECK: arith.addi
+  x = x + 12
+  !CHECK: omp.terminator
+  !$omp end task
+end subroutine task_depend
+
+!CHECK-LABEL: func @_QPtask_depend_non_int
+subroutine task_depend_non_int()
+  character(len = 15) :: x
+  integer, allocatable :: y
+  complex :: z
+  !CHECK: omp.task depend(taskdependin -> %{{.+}} : !fir.ref<!fir.char<1,15>>, taskdependin -> %{{.+}} : !fir.ref<!fir.box<!fir.heap<i32>>>, taskdependin ->  %{{.+}} : !fir.ref<!fir.complex<4>>) {
+  !$omp task depend(in : x, y, z)
+  !CHECK: omp.terminator
+  !$omp end task
+end subroutine task_depend_non_int
+
+!CHECK-LABEL: func @_QPtask_depend_all_kinds_one_task
+subroutine task_depend_all_kinds_one_task()
+  integer :: x
+  !CHECK: omp.task depend(taskdependin -> %{{.+}} : !fir.ref<i32>, taskdependout -> %{{.+}} : !fir.ref<i32>, taskdependinout -> %{{.+}} : !fir.ref<i32>) {
+  !$omp task depend(in : x) depend(out : x) depend(inout : x)
+  !CHECK: arith.addi
+  x = x + 12
+  !CHECK: omp.terminator
+  !$omp end task
+end subroutine task_depend_all_kinds_one_task
+
+!CHECK-LABEL: func @_QPtask_depend_multi_var
+subroutine task_depend_multi_var()
+  integer :: x
+  integer :: y
+  !CHECK: omp.task depend(taskdependin -> %{{.*}} : !fir.ref<i32>, taskdependin -> %{{.+}} : !fir.ref<i32>) {
+  !$omp task depend(in :x,y)
+  !CHECK: arith.addi
+  x = x + 12
+  y = y + 12
+  !CHECK: omp.terminator
+  !$omp end task
+end subroutine task_depend_multi_var
+
+!CHECK-LABEL: func @_QPtask_depend_multi_task
+subroutine task_depend_multi_task()
+  integer :: x
+  !CHECK: omp.task depend(taskdependout -> %{{.+}} : !fir.ref<i32>)
+  !$omp task depend(out : x)
+  !CHECK: arith.addi
+  x = x + 12
+  !CHECK: omp.terminator
+  !$omp end task
+  !CHECK: omp.task depend(taskdependinout -> %{{.+}} : !fir.ref<i32>)
+  !$omp task depend(inout : x)
+  !CHECK: arith.addi
+  x = x + 12
+  !CHECK: omp.terminator
+  !$omp end task
+  !CHECK: omp.task depend(taskdependin -> %{{.+}} : !fir.ref<i32>)
+  !$omp task depend(in : x)
+  !CHECK: arith.addi
+  x = x + 12
+  !CHECK: omp.terminator
+  !$omp end task
+end subroutine task_depend_multi_task
+
+!===============================================================================
 ! `private` clause
 !===============================================================================
 !CHECK-LABEL: func @_QPtask_private
 subroutine task_private
   type mytype
   integer :: x
   end type mytype