[flang-commits] [PATCH] D94087: [flang][openmp]At most one threads, simd and depend clause can appear on OpenMP ORDERED construct.

[Peixin Qiao via Phabricator via flang-commits]

peixin added a comment.

BTW, the error of no symbol found in `depend(sink: vec)` should be fixed by https://reviews.llvm.org/D108530.



================
Comment at: flang/test/Semantics/omp-clause-validity01.f90:489
+  !ERROR: At most one DEPEND clause can appear on the ORDERED directive
+  !$omp ordered depend(source) depend(source)
+  !ERROR: At most one THREADS clause can appear on the ORDERED directive
----------------
sameeranjoshi wrote:
> clementval wrote:
> > kiranchandramohan wrote:
> > > Will this catch two or more depend(sink: v) clauses accidentally?
> > > ```
> > > At most one depend(source) clause can appear on an ordered construct.
> > > ```
> > > 
> > > Can threads, simd occur with depend? Is that handled by the parser?
> > According to the standard multiple `depend(sink: vec)` are allowed so you will need to make a specific check for the depend clause since the general allowedOnce check does not support this. 
> > 
> > @kiranchandramohan With the current patch `threads`, `simd` and `depend` are allowed together. The parser is not limiting clauses. 
> > Can threads, simd occur with depend? Is that handled by the parser?
> 
> They are not supposed to occur together.
> No it's not caught in parser.
> This will be handled separately. I am seeing some issues need to debug it.
> 
We may should also give error infomation when threads, simd occur with depend. Otherwise, the behavior would be unspecified. For current clang codegen, it generates the IR only for depend when threads, simd occur with depend. I am implementing the code for flang codegen. I should also use the similar method as clang codegen. The method should be like this:

```
if (isDependPresent) {
  // lowering depend
} else if (isSimdPresent) {
  // lowering simd
} else {
  // It does not matter if isThreadsPresent is true or false since the construct behaves as if threads is specified when no clause is specified.
}
```
I assume that this can be checked in `Leave(const parser::OmpClauseList &)` by using FindClause. The condition should be like this:

```
if ((isThreadsPresent || isSimdPresent) && isDependPresent) {
  // error report
}
```
Maybe there is one better place to check this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94087