[PATCH] D124156: [flang][OpenMP] Added parser support for in_reduction clause on OpenMP Task directive

[Nimish Mishra via Phabricator via llvm-commits]

NimishMishra marked 4 inline comments as done.
NimishMishra added inline comments.


================
Comment at: flang/test/Parser/omp-task-unparse.f90:5
+
+program block_construct_unparsing
+    integer:: x, y 
----------------
shraiysh wrote:
> Are you sure this program is semantically correct? I think an in_reduction clause must be enclosed inside task_reduction or some form of reduction clause. Having it at the top level could be an error, but I could be wrong. If it is not an error this is okay, otherwise a simple solution would be to change this to a subroutine instead of a program.
Could you check this updated test case?


================
Comment at: flang/test/Semantics/omp-clause-validity01.f90:66
   do i = 1, N
      z = 2
   end do
----------------
kiranchandramohan wrote:
> shraiysh wrote:
> > Why are we adding semantics test when we have no semantic checks for this clause yet? IMO we should not be adding this test right now.
> > 
> > Anyways, if you decide to add this test, maybe add it as a separate test because private(b) and in_reduction(+:b) are probably conflicting. Also, please add some reducing code inside the structured block.
> Possibly, it is checking the allowed clauses that are automatically created due to entries in OMP.td.
> 
> Not clear whether it is allowed or not. But something simple and separate would be better.
> ```
> !$omp taskgroup task_reduction(+:z)
> !$omp task in_reduction(+:z)
> z = z + 5
> !$omp end task
> !$omp end taskgroup
> ```
> 
> We might also need to ask the task modifier to the regular reduction clause sometime.
I think @shraiysh is right in mentioning that I shouldn't be touching the semantics side of things. Removed the test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124156