[PATCH] D108904: [flang][OpenMP] Added semantic checks for sections (associated section(s) should be structured block(s)) and simd constructs (associated loop(s) should be structured block(s))

[Nimish Mishra via Phabricator via llvm-commits]

NimishMishra added inline comments.


================
Comment at: flang/test/Semantics/omp-simd01.f90:14
   do i = 1, 10
     do j = 1, 10
       print *, "omp simd"
----------------
peixin wrote:
> NimishMishra wrote:
> > peixin wrote:
> > > Does your implementation support the following code:
> > > 
> > > ```
> > > !$omp simd
> > > do i = 1, 10
> > >   30 stop
> > >   do j = 1, 10
> > >     goto 30
> > >   end do
> > > end do
> > > !$omp end simd.
> > > ```
> > > If no collapse clause specified, it behaves as if only one loop immediately following is associated with the construct. Could you please add the test in your test case?
> > The present state of the implementation gives a '//STOP statement is not allowed in a SIMD construct//'. Is that what you were looking for?
> > 
> > If not, this was added as //void Post(const parser::StopStmt &) { EmitBranchOutError("STOP"); }// in //NoBranchingEnforce// (source: https://reviews.llvm.org/D88655). Possibly @kiranchandramohan can offer some insights. According to his comment there, parallel region can have a STOP. I am not sure about SIMD.
> > 
> > 
> > That said, if I replace that STOP statement with something else (like the example below), no errors are generated. That is expected I believe since branching within SIMD's associated DO loop shouldn't be considered a violation of the structured block restriction.
> > 
> > 
> > ```
> >  program sample                                                                                                                                                 use omp_lib                                                                                                                                                !$omp simd                                                                                                                                                   do i = 1, 10                                                                                                                                                 30 print*, "sample"                                                                                                                                          do j = 1, 10                                                                                                                                                   goto 30                                                                                                                                                      end do                                                                                                                                                     end do                                                                                                                                                     !$omp end simd                                                                                                                             end program sample
> > ```
> You can replace it with print *, "sample". My key point is to check branching out to the nested loop which is not associated with `simd` construct. I thought out one more better test case as follows:
> 
> ```
> !$omp simd collapse(2)
> do i = 1, 10
>   30 print *, "This is wrong"
>   do j = 1, 10
>     40 print *, "This is OK"
>     do k = 1, 10
>       if (expression) then
>         goto 30
>       end if
>       if (expression2) then
>         goto 40
>       end if
>     end do
>   end do
> end do
> !$omp end simd
> ```
> If my understanding is correct, please polish this test case and add it.
The constraint is there, albeit not because of my changes. When I run this case, it gives a //The value of the parameter in the COLLAPSE or ORDERED clause must not be larger than the number of nested loops following the construct. //

The reason for this, as it seems to me is, that when we give a //COLLAPSE(2)//, it checks for immediately nesting of exactly 2 DO constructs. Anything else simply raises this error. In this vein, the following modification of this test case succeeds:


```
!$omp simd collapse(2)
do i = 1, 10  ! Removed 30 print*, ...
  do j = 1, 10
    40 print *, "This is OK"
    do k = 1, 10
      if (expression) then
        goto 30
      end if
      if (expression2) then
        goto 40
      end if
    end do
  end do
end do
!$omp end simd
```

Similarly, trying //COLLAPSE(3)// would force removal of the print statement at label 40. So I think we can be reasonably assured that the simd nesting checks implemented in //check-omp-structure.cpp// will assure absence of any such statements which can violate the structured block restriction.

Please let me know your thoughts on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108904