[PATCH] D113399: [flang][OpenMP] Add semantic checks of nesting of region about ordered construct

[Peixin Qiao via Phabricator via llvm-commits]

peixin added a comment.

In D113399#3230599 <https://reviews.llvm.org/D113399#3230599>, @shraiysh wrote:

> Thanks for addressing the comments @peixin. One minor comment, rest LGTM. (Please wait for one more review because I am not an expert in this part of the code).

Sure. Thanks for the notice.



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:771-799
+    for (int i = (int)dirContext_.size() - 2; i >= 0; i--) {
+      if (llvm::omp::doSet.test(dirContext_[i].directive)) {
+        isNestedInDo = true;
+        isNestedInDoSIMD = llvm::omp::doSimdSet.test(dirContext_[i].directive);
+        if (const auto *clause{
+                FindClause(dirContext_[i], llvm::omp::Clause::OMPC_ordered)}) {
+          const auto &orderedClause{
----------------
shraiysh wrote:
> I meant changing it this way. This will eliminate constructs like this - 
> ```
> if(condition)
>   var = true;
> ```
> by setting `var = condition`. This change passes all the tests in the current patch. Please let me know if I have missed something here.
Oh. This change sacrifice the time cost. For example, `isOrderedClauseWithPara` is true for `!$omp simd ordered(2)`. Actually, the code inside the if condition of `(llvm::omp::doSet.test(dirContext_[i].directive))` should never be run for this case. So, the left if conditions are useful to avoid extra operations. What do you think?


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

https://reviews.llvm.org/D113399