[PATCH] D93091: [Flang] [OpenMP] Add semantic checks for OpenMP Workshare Construct

[Praveen G via Phabricator via llvm-commits]

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

@kiranchandramohan Updated the patch to handle all the checks on workshare construct . Since the WhereConstruct and ForallConstructs have nested WhereStmt/WhereConstructs and ForAllStmt/ForallConstructs , separate functions are added to handle the same recursively.



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:766
+              context_.Say(expr.source,
+                  "User defined non-ELEMENTAL function "
+                  "'%s' is not allowed in a WORKSHARE construct"_err_en_US,
----------------
kiranchandramohan wrote:
> I think use of non-elemental function can be outside an assignment statement like in the where statement below.
> ```
> program mn
>   integer :: a(10) = (/1,2,3,4,5,6,7,8,9,10/)
>   !$omp workshare
>   where ( a .lt. f()) a = a + 5
>   !$omp end workshare
> contains
>   integer function f()
>     f = 5
>   end function
> end program
> ```
Thanks @kiranchandramohan Will handle the same.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:766
+              context_.Say(expr.source,
+                  "User defined non-ELEMENTAL function "
+                  "'%s' is not allowed in a WORKSHARE construct"_err_en_US,
----------------
praveen wrote:
> kiranchandramohan wrote:
> > I think use of non-elemental function can be outside an assignment statement like in the where statement below.
> > ```
> > program mn
> >   integer :: a(10) = (/1,2,3,4,5,6,7,8,9,10/)
> >   !$omp workshare
> >   where ( a .lt. f()) a = a + 5
> >   !$omp end workshare
> > contains
> >   integer function f()
> >     f = 5
> >   end function
> > end program
> > ```
> Thanks @kiranchandramohan Will handle the same.
@kiranchandramohan Modified the code to handle the same . Thanks!


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:790-793
+        context_.Say(source,
+            "OpenMP '%s' construct is not allowed "
+            "in a WORKSHARE construct"_err_en_US,
+            parser::ToUpperCaseLetters(getDirectiveName(beginDir.v).str()));
----------------
kiranchandramohan wrote:
> Is it better to use OmpStructureChecker::HasInvalidWorksharingNesting for handling this?
@kiranchandramohan We would have to check for workshare directive and call OmpStructureChecker::HasInvalidWorksharingNesting for all the OpenMP Constructs. 

Since the critical and atomic constructs are being checked here , thought it would be simpler to handle here.

Please let me know if we can have the check for other OpenMP constructs here or make use of HasInvalidWorksharingNesting in all the OpenMP Constructs


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

https://reviews.llvm.org/D93091