[PATCH] D92735: [Flang] [OpenMP] Add semantic checks for invalid branch into/from OpenMP constructs

[Praveen G via Phabricator via llvm-commits]

praveen marked an inline comment as done.
praveen added a comment.

> I am not familiar with that. I can look up if required. But what I am suggesting is to do what is similar to the do loop checks like the following.
>
>   $ ./bin/flang ../flang/test/Semantics/doconcurrent03.f90
>   ../flang/test/Semantics/doconcurrent03.f90:11:9: error: Control flow escapes from DO CONCURRENT
>             goto 20
>             ^^^^^^^
>   ../flang/test/Semantics/doconcurrent03.f90:9:3: Enclosing DO CONCURRENT statement
>       do 10 concurrent (i = 1:10)
>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   ../flang/test/Semantics/doconcurrent03.f90:18:3: branch into loop body from outside
>       goto 30
>       ^^^^^^^
>   ../flang/test/Semantics/doconcurrent03.f90:9:3: the loop branched into
>       do 10 concurrent (i = 1:10)
>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   `

@kiranchandramohan As per the current changes , the errors for the various cases are being displayed as follows

**Branch into the OpenMP Structured block**

  !$omp parallel
  10 print*, 10
  !$omp end parallel
  goto 10
   
  ./invalid-branching.f90:15:1: error: invalid entry to OpenMP structured block
  goto 10
  ^^^^^^^
  ./invalid-branching.f90:13:3: In the enclosing PARALLEL directive
   10 print*, 10
    ^^^^^^^^^^^^^

**Branch out of OpenMP structured blocks**

  !$omp parallel
   goto 10
  !$omp end parallel
  10 print*, 10
  
  ./invalid-branching.f90:13:4: error: invalid branch to/from OpenMP structured block
     goto 10
     ^^^^^^^
  ./invalid-branching.f90:15:1: Outside the enclosing PARALLEL directive
  10 print*, 10
  ^^^^^^^^^^^^^

**Branch among the OpenMP structured blocks **

  !$omp parallel
  10 print *, 10
  !$omp end parallel
  
  !$omp parallel
  goto 10
  !$omp end parallel
  
  ./invalid-branching.f90:17:3: error: invalid branch to/from OpenMP structured block
    goto 10
    ^^^^^^^
  ./invalid-branching.f90:13:3: Outside the enclosing PARALLEL directive
    10 print *, 10
    ^^^^^^^^^^^^^^

//Since the second message in the above errors which indicates the label target source are not marked as **error** , I had not included them as plain comments in the test cases.//

> BTW, for a branch leaving/entering a nested construct, how many errors are reported? Just to understand, Not requesting a change here.
>
>   !$omp parallel
>   !$omp single
>   !$omp end single
>   goto 10
>   !$omp end parallel
>   
>   10 print *, "Invalid"

For the branch entering/leaving a nested construct , one error is reported by printing the innermost construct in which the error occured.

  !$omp parallel
  !$omp single
  goto 10
  !$omp end single
  !$omp end parallel
  10 print *, 10
  
  ./invalid-branching.f90:14:3: error: invalid branch to/from OpenMP structured block
    goto 10
    ^^^^^^^
  ./invalid-branching.f90:17:3: Outside the enclosing SINGLE directive
    10 print *, 10



================
Comment at: flang/lib/Semantics/resolve-directives.cpp:1556
+    context_
+        .Say(source, "invalid branch to/from OpenMP structured block"_err_en_US)
+        .Attach(target, "Outside the enclosing %s directive"_en_US,
----------------
kiranchandramohan wrote:
> praveen wrote:
> > kiranchandramohan wrote:
> > > If this is only about branches leaving the OpenMP structured block then can it be renamed to
> > > "invalid branch leaving and OpenMP structured block"?
> > @kiranchandramohan The error "invalid branch to/from OpenMP structured block" is thrown for both the cases where there is a branch out of the OpenMP structured block and also branch among the OpenMP structured blocks in which case it leaves one construct and enters another one.
> > 
> > Can we split the above cases to contain different error messages as below
> > 
> > 
> > **Branch out of the OpenMP blocks** -   "invalid branch leaving an OpenMP structured block"
> > 
> > 
> > **Branch among the OpenMP structured blocks** - 
> >  "invalid branch to an OpenMP structured block in SOURCE directive " 
> >  "invalid branch from an OpenMP structured block in TARGET directive"
> > 
> @praveen In my opinion we should consider only two cases,
> 1) Branch out of an openmp structured block.
> 2) Branch into an openmp structured block.
> 
> either (1) or (2) or both can happen. But we do not need to distinguish further like whether the branch is among or not among OpenMP structured blocks. If I am missing a point here, please let me know.
@kiranchandramohan Yes . As you have pointed out , we should consider only two cases - branch into and branch out of the OpenMP structured blocks. 

  !$omp parallel
  10 print *, 10
  !$omp end parallel

  !$omp parallel
  goto 10
  !$omp end parallel

  ./invalid-branching.f90:17:3: error: invalid branch to/from OpenMP structured block
    goto 10
    ^^^^^^^
  ./invalid-branching.f90:13:3: Outside the enclosing PARALLEL directive
    10 print *, 10
    ^^^^^^^^^^^^^^

I had followed the error reporting from gfortran which displays a single error 
"invalid branch to/from OpenMP structured block" . 

Currently , for the above case , we are displaying a single error similar to gfortran. 

Please suggest if it would be appropriate to display two errors both -  **branch into and branch out errors separately** for the two constructs in cases such as the above ones as shown below?

  !$omp parallel
  10 print *, 10
  !$omp end parallel

  !$omp parallel
  goto 10
  !$omp end parallel

  ./invalid-branching.f90:15:1: error: invalid entry to OpenMP structured block
  goto 10
  ^^^^^^^
  ./invalid-branching.f90:13:3: In the enclosing PARALLEL directive
    10 print*, 10
    ^^^^^^^^^^^^^

  ./invalid-branching.f90:15:1: error: invalid branch leaving an OpenMP structured block
    goto 10
    ^^^^^^^
  ./invalid-branching.f90:13:3: Outside the enclosing PARALLEL directive
    10 print *, 10
    ^^^^^^^^^^^^^^








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

https://reviews.llvm.org/D92735