[PATCH] D92735: [Flang] [OpenMP] Add semantic checks for invalid branch into/from OpenMP constructs
Praveen G via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 1 06:42:21 PST 2021
praveen marked 3 inline comments as done.
praveen added a comment.
> Thanks for the detailed listing. I think the second message in the do loop checks (non-openmp) is also not listed as an error. But they still check for that in the tests.
> In short,
>
> 1. you can follow what they do for the do loop checks (non-openmp)
> 2. Add the two separate messages in the tests.
@kiranchandramohan I have updated the error messages and the test cases to check for two separate messages in the tests
================
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:
> > > 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
> > ^^^^^^^^^^^^^^
> >
> >
> >
> >
> >
> >
> Thanks @praveen for the detailed listing. I understand it now.
>
> I would still prefer separate errors as you listed above.
@kiranchandramohan Thanks. I have updated the error messages
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92735/new/
https://reviews.llvm.org/D92735
More information about the llvm-commits
mailing list