[PATCH] D98558: [OPENMP51]Initial support for the interop directive

[Mike Rice via Phabricator via llvm-commits]

mikerice marked 5 inline comments as done.
mikerice added a comment.

Thanks for the review.



================
Comment at: clang/include/clang/AST/OpenMPClause.h:7491
+/// \endcode
+class OMPUseClause final : public OMPClause {
+  friend class OMPClauseReader;
----------------
ABataev wrote:
> I suggest splitting this patch anв keep just one clause. Other clauses should be added in follow-up patches.
Sorry I should have done that.  Unfortunately the logic is somewhat shared among these three and breaking it up at this point would mean rewriting it without those interactions.  I can do it if you really want me to.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:14632-14633
+      IsTargetSync = InitClause->getIsTargetSync();
+    else if (const auto *DC = dyn_cast<OMPDependClause>(C))
+      DependClause = DC;
+  }
----------------
ABataev wrote:
> ```
> else {
> assert(isa<OMPDependClause>(C) && "...");
> ...
> }
> ```
Sorry, I'm not following this comment.  This is just doing something on 'init' and 'depend' clauses.  Other clauses can exist on the directive and need to be skipped in the loop.


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

https://reviews.llvm.org/D98558