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

[Praveen G via Phabricator via llvm-commits]

praveen marked 2 inline comments as done.
praveen added a comment.

In D92735#2436164 <https://reviews.llvm.org/D92735#2436164>, @kiranchandramohan wrote:

> There is a labelEnforce class (https://github.com/llvm/llvm-project/blob/a2f922140f5380571fb74179f2bf622b3b925697/flang/lib/Semantics/tools.cpp#L1338) which is used for checks in do-concurrent and co-arrays. It is used to find control flow escaping from a construct. Can that be reused?

@kiranchandramohan We can make use of labelEnforce class to identify the control flow escaping the construct . But it would not be useful to identify the control flow entering the OpenMP constructs and also it does not identify the transfer of control from one OpenMP construct to another such as this.

  !$omp parallel
  goto 10
  a = a + b 
  !$omp single
  10 call sb()
  !$omp end single
  !$omp end parallel

> Function CheckBranchesIntoDoBody (https://github.com/llvm/llvm-project/blob/a2f922140f5380571fb74179f2bf622b3b925697/flang/lib/Semantics/resolve-labels.cpp#L845) in resolve-labels.cpp checks for branches into the body of a loop. Can this code be reused?

Since the function CheckBranchesIntoDoBody does not follow the OpenMP constructs , I guess it would not be much useful in this case .

Please suggest.



================
Comment at: flang/lib/Semantics/check-omp-structure.h:76
     OmpDirectiveSet{Directive::OMPD_task} | taskloopSet};
+static OmpDirectiveSet noBranchSet{llvm::omp::Directive::OMPD_parallel,
+    llvm::omp::Directive::OMPD_single, llvm::omp::Directive::OMPD_simd,
----------------
kiranchandramohan wrote:
> Isn't this check true for all constructs with structured blocks like master, critical etc?
@kiranchandramohan Had only added the constructs that contained this restriction explicitly. Modified to handle this check for all constructs with structured blocks .

Thanks!


================
Comment at: flang/test/Semantics/omp-parallell01.f90:1
 ! RUN: %S/test_errors.sh %s %t %f18 -fopenmp
 
----------------
kiranchandramohan wrote:
> Nit: The name of this test and the next has an extra 'l'.
Removed the extra 'l' . Thanks


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

https://reviews.llvm.org/D92735