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

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added a comment.

In D92735#2530901 <https://reviews.llvm.org/D92735#2530901>, @praveen wrote:

> In D92735#2530795 <https://reviews.llvm.org/D92735#2530795>, @kiranchandramohan wrote:
>
>> Can the error messages be updated to check for the entire message, both the place where it is entering and leaving (or viceversa)?
>
> Currently , the error message with the source of the statement causing the branch is thrown as a fatal error and the error message with source of the target label printed as part of the Attach() is not being marked as fatal .
>
> Should both the error messages be marked as fatal and the error messages be updated accordingly as separate messages ?

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)
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  `

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"



================
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,
----------------
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.


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

https://reviews.llvm.org/D92735