This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by psoni2628 on Mar 23 2023, 3:21 PM.

Details

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.

Diff Detail

Event Timeline

psoni2628 created this revision.Mar 23 2023, 3:21 PM
psoni2628 requested review of this revision.Mar 23 2023, 3:21 PM
psoni2628 updated this revision to Diff 507902.Mar 23 2023, 3:34 PM
  • Use TODO instead of llvm_unreachable for array sections

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 added a comment.EditedMar 24 2023, 7:33 AM

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

@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 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 edited the summary of this revision. (Show Details)May 23 2023, 9:24 AM

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 ?

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 accepted this revision.Jun 4 2023, 5:34 AM

Fine with me. LG.

This revision is now accepted and ready to land.Jun 4 2023, 5:34 AM
psoni2628 updated this revision to Diff 528250.Jun 4 2023, 12:45 PM
  • Add more test cases based on Kiran's feedback

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?

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

Tests look OK to me.

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 added a subscriber: luporl.Jun 6 2023, 10:10 AM

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.

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!