[PATCH] D105876: OMPIRBuilder for Interop directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6310-6311
+  int DependClauseCount = 0;
+  for (const auto *DC : S.getClausesOfKind<OMPDependClause>())
+    DependClauseCount++;
+  assert(DependClauseCount <= 1 && "Multiple OMPDependClause not supported.");
----------------
This code can be removed.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6328
+
+  int HaveNowaitClause = 0;
+  if (S.getSingleClause<OMPNowaitClause>())
----------------
`bool`


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6329
+  int HaveNowaitClause = 0;
+  if (S.getSingleClause<OMPNowaitClause>())
+    HaveNowaitClause = 1;
----------------
`hasClausesOfKind` should be better


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6334
+    llvm::Value *InteropvarPtr =
+        (EmitLValue(C->getInteropVar()).getAddress(*this)).getPointer();
+    llvm::OMPInteropType InteropType = llvm::OMPInteropType::Unknown;
----------------
`EmitLValue(C->getInteropVar()).getPointer(*this);`.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6344-6345
+  } else if (const auto *C = S.getSingleClause<OMPDestroyClause>()) {
+    llvm::Value *InteropvarPtr =
+        (EmitLValue(C->getInteropVar()).getAddress(*this)).getPointer();
+    OMPBuilder.createOMPInteropDestroy(Builder, InteropvarPtr, Device,
----------------
`EmitLValue(C->getInteropVar()).getPointer(*this);`.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6350-6351
+  } else if (const auto *C = S.getSingleClause<OMPUseClause>()) {
+    llvm::Value *InteropvarPtr =
+        (EmitLValue(C->getInteropVar()).getAddress(*this)).getPointer();
+    OMPBuilder.createOMPInteropUse(Builder, InteropvarPtr, Device,
----------------
`EmitLValue(C->getInteropVar()).getPointer(*this);`.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6355-6358
+  } else if (HaveNowaitClause == true) {
+    llvm_unreachable("Nowait clause is used separately in Interop Directive.");
+  } else {
+    llvm_unreachable("Missing Interop clauses.");
----------------
Is this allowed at all? If not, better to turn it into an `assert`.


================
Comment at: clang/test/OpenMP/interop_irbuilder.cpp:1
+// RUN: %clang_cc1 -verify -fopenmp  -o -  %s
+
----------------
Actual codegen checks are missing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105876