[PATCH] D90697: [Flang][OpenMP 4.5] Add semantic check for OpenMP Reduction Clause

[Yashaswini Hegde via Phabricator via llvm-commits]

yhegde marked an inline comment as done.
yhegde added inline comments.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:572
+                "A list item that appears in a REDUCTION clause"
+                " cannot have a zero-length array section."_err_en_US,
+                ContextDirectiveAsFortran());
----------------
kiranchandramohan wrote:
> CheckDependArraySection has a similar check. Can you either reuse or refactor such that common code is not duplicated?
Sure. Will check what all can be reused. 


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:578
+            auto stride{evaluate::ToInt64(*st)};
+            if ((stride && stride != 1)) {
+              context_.Say(GetContext().clauseSource,
----------------
kiranchandramohan wrote:
> This will not catch cases like the following.
> 
> ```
> program mn
>   integer :: c(10,10,10)
>   integer :: i
>   integer :: k = 10
> 
>   !$omp parallel do reduction(+:c(1:10,5,1:6))
>   do i = 1, 10
>     k = k + 1
>   end do
>   !$omp end parallel do
>   print *, is_contiguous(c(1:10,5,1:6))
> end program
> ```
Thank you. This will be addressed. 


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:639
 }
+void OmpStructureChecker::CheckPrivateClauseRestrictions(
+    const parser::OmpObjectList &objList) {
----------------
kiranchandramohan wrote:
> This does not seem to work for the following case. I think you should start from the reduction list and the check whether those variables are private.
> 
> ```
> program omp_reduction
> 
>   integer :: i
>   integer :: k = 10
>   !$omp parallel private(k)
>   !$omp do reduction(+:k)
>   do i = 1, 10
>     k = k + 1
>   end do
>   !$omp end do
>   !$omp end parallel
> end program
> ```
Thank you once again.  This case will be addressed. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90697