[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))

[Peixin Qiao via Phabricator via llvm-commits]

peixin added a comment.

> According to the spec document, I had to deal with orphaned //section// (without an enclosing //sections//). I added a test case for the same. However, presently, the parser itself is dealing correctly with the issue by giving errors with very less contextual information. Somewhere down the line, we may need to shift this check from the Parser to Semantics, or improve error reporting in the Parser itself. For now, the test case is a //XFAIL// with a //TODO// added to it.

I understand this. What I mean is your implementation is not consistent with the summary, which should be fixed since this patch has not implement the semantic check with informative error message. Otherwise, the summary, which should be the commit message, is confusing.



================
Comment at: flang/test/Semantics/omp-simd01.f90:14
   do i = 1, 10
     do j = 1, 10
       print *, "omp simd"
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108904