[PATCH] D92732: [Flang][OpenMP 4.5] Add semantic check for OpenMP Do Loop Constructs

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan requested changes to this revision.
kiranchandramohan added a comment.
This revision now requires changes to proceed.

-> Threadprivate check is not working for the following case (when the declaration is in the parent scope).

  program omp_do
    integer, save:: i, j, k
    !$omp threadprivate(i)
    !$omp threadprivate(j)
    call compute()
  contains
    subroutine compute()
    !$omp  do collapse(2)
    do k = 1, 10
      do i = 1, 10
        print *, "Hello"
      end do
    end do
    !$omp end do
    end subroutine
  end program omp_do

-> Do loop cycle restrictions should also include ordered besides collapse. I think this should be the same checks.



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:113
+      context_.Say(*cyclesource_,
+          "CYCLE statement to non-innermost collapsed !$OMP DO loop"_err_en_US);
+    }
----------------
We usually do not add the "!$" in the error message.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:217
+        context_.Say(doStmt.source,
+            "The do loop cannot be a DO WHILE with do directive."_err_en_US);
+      }
----------------
Can the "do" be capitalized for the loop and the directive?



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:234
+            context_.Say(itrVal.source,
+                "The do-loop iteration"
+                " variable must be of the type integer."_err_en_US,
----------------
Use DO loop instead of do-loop.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:405
+      dir.source, llvm::omp::Directive::OMPD_threadprivate);
+  threadPrivateSymbols.clear();
+  const auto &list{std::get<parser::OmpObjectList>(x.t)};
----------------
This clear will cause programs with multiple declarations to not work properly. See example below.
```
program omp_do
  integer, save:: i, j, k
  !$omp threadprivate(i)
  !$omp threadprivate(j)
  !$omp do collapse(2)
  do k = 1, 10
    do i = 1, 10
      print *, "Hello"
    end do
  end do
  !$omp end do
end program omp_do
```


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:294
+  parser::CharBlock src{"cycle"};
+  if (collapseLevel) {
+    if (const auto &loopConstruct{
----------------
yhegde wrote:
> kiranchandramohan wrote:
> > Will checks only happen for loops with collapse clause? What about those without collapse? Should it be handled elsewhere?
> > ```
> >   !$omp parallel
> >   foo: do i = 0, 10
> >     !$omp do
> >     bar: do j = 0, 10
> >            cycle foo
> >          end do bar
> >     !$omp end do
> >     !$omp end parallel
> >   end do foo
> > ```
> Cases with OpenMP blocks in between are not handled . It requires different error message. But cases with out collapse - like the following case, works fine. Will add the test cases with out collapse. Thank you.  
> program test
> !$omp parallel
> foo: do i = 0, 10
>   !!$omp do
>   bar: do j = 0, 10
>          cycle foo
>   end do bar
>   !!$omp end do
> end do foo
> !$omp end parallel
> end program test
My concern was because the cyclechecker is entered only if the collapse level is greater than 0. If there is no collapse then i guess it will return 0 and hence the cyclechecker might not be called.
```
  std::int64_t collapseLevel{GetCollapseLevel(x)};
  if (collapseLevel) {
```


================
Comment at: flang/test/Semantics/omp-do13.f90:1
+! RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! OpenMP Version 4.5
----------------
yhegde wrote:
> yhegde wrote:
> > kiranchandramohan wrote:
> > > Please add a test for an openmp do loop with collapse nested inside an openmp do loop with collapse.
> > ok.
> > Please add a test for an openmp do loop with collapse nested inside an openmp do loop with collapse.
> 
> Added a case like this . Hope this is what is suggested.
> 
> !$omp do  collapse(2)
>   foo: do i = 0, 10
>     foo1: do j = 0, 10
>       !ERROR: CYCLE statement to non-innermost collapsed !$OMP DO loop
>       cycle foo
>       !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
>       !$omp do  collapse(1)
>       foo2:  do k  = 0, 10
>         print *, i, j, k
>         end do foo2
>      !$omp end do
>      end do foo1
>   end do foo
>   !$omp end do
> 
I mean't the following where you have the same error in both the inner and outer collapsed set of loops. Also add another one with parallel do collapse(2).
```
  !$omp parallel
  !$omp do collapse(2)
  foo: do i = 0, 10
    foo1: do j = 0, 10
      cycle foo
      !$omp parallel
      !$omp do collapse(2)
      foo2:  do k  = 0, 10
        foo3:  do l  = 0, 10
        print *, i, j, k, l
        cycle foo2
        end do foo3
      end do foo2
      !$omp end do
      !$omp end parallel
     end do foo1
  end do foo
  !$omp end do
  !$omp end parallel
  end program
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92732/new/

https://reviews.llvm.org/D92732