[PATCH] D92735: [Flang] [OpenMP] Add semantic checks for invalid branch into/from OpenMP 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.

Can the error messages be updated to check for the entire message, both the place where it is entering and leaving (or viceversa)?



================
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,
----------------
If this is only about branches leaving the OpenMP structured block then can it be renamed to
"invalid branch leaving and OpenMP structured block"?


================
Comment at: flang/test/Semantics/omp-invalid-branch.f90:11
+  !$omp parallel
+  !ERROR: Control flow escapes from PARALLEL
+  open (10, file="myfile.dat", err=100)
----------------
praveen wrote:
> praveen wrote:
> > kiranchandramohan wrote:
> > > praveen wrote:
> > > > kiranchandramohan wrote:
> > > > > Why doesn't the other error (invalid branches to/from OpenMP structured blocks) show here?
> > > > @kiranchandramohan The error "invalid branches to/from OpenMP structured blocks" will be identified in the CheckLabelContext function for the cases where the control flow reaches out of OpenMP structured blocks. 
> > > > 
> > > > Because //we are already throwing// the "Control flow escapes from PARALLEL" error,  thought it would result in multiple error messages for the same error. 
> > > > 
> > > > Hence was throwing the invalid branches to/from OpenMP structured blocks only for branching among the nested OpenMP constructs which were not handled as part of LabelEnforce such as the following case.
> > > > 
> > > >   !$omp parallel
> > > >   goto 10
> > > >   !$omp single
> > > >   10 print*, "OMP SINGLE"
> > > >   !$omp end single
> > > >   !$omp end parallel
> > > > 
> > > > 
> > > > Do we display** both Control flow escapes from 'CONSTRUCT' and invalid branches to/from OpenMP structured block** for cases where the control escapes out of OpenMP blocks such as the following?
> > > > 
> > > > 
> > > >   !$omp parallel
> > > >   goto 10
> > > >   !$omp end parallel
> > > >   10 print*, 10
> > > > 
> > > >Hence was throwing the invalid branches to/from OpenMP structured blocks only for branching among the nested OpenMP constructs which were not handled as part of LabelEnforce such as the following case.
> > > 
> > > I am assuming that nesting check should work but it is not happening here because CheckNoBranching is not called for the single construct.
> > > 
> > > > Do we display both Control flow escapes from 'CONSTRUCT' and invalid branches to/from OpenMP structured block for cases where the control escapes out of OpenMP blocks such as the following?
> > > 
> > > Ideally only one error should displayed.
> > Throwing the error 'invalid branches to/from OpenMP structured blocks' instead of Control flow escaping from the CONSTRUCT 
> > I am assuming that nesting check should work but it is not happening here because CheckNoBranching is not called for the single construct.
> 
> We are calling the CheckNoBranching for the single construct. 
> The error was not getting identified because in the parse-tree walk of the enclosing parallel construct , it does not identify the code between !$omp single and !$omp end single as that in a different scope and the label would be identified to be in the same scope as that of the statement causing the jump.
> 
> 
```
!$omp parallel
goto 10
!$omp single
10 print*, "OMP SINGLE"
!$omp end single
!$omp end parallel
```

For the above case label enforce is not printing out an error because there is no control flow leaving an OpenMP region here. It always stays inside parallel. The error here is that there is a branch entering the single construct  which is not what label enforce is concerned with.


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

https://reviews.llvm.org/D92735