sriharikrishna marked an inline comment as done.
sriharikrishna added inline comments.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6330-6331
+  if (const auto *C = S.getSingleClause<OMPInitClause>()) {
+    llvm::Value *InteropvarPtr =
+        EmitLValue(C->getInteropVar()).getPointer(*this);
+    llvm::omp::OMPInteropType InteropType = llvm::omp::OMPInteropType::Unknown;
----------------
ABataev wrote:
> This code is common for all `if-else` braches, move out of the conditional 
> blocks?
Moving the common line out will require checking the kind of clause in the 
directive. So, I prefer to leave it as is. 


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6335-6336
+      InteropType = llvm::omp::OMPInteropType::Target;
+    else if (C->getIsTargetSync())
+      InteropType = llvm::omp::OMPInteropType::TargetSync;
+    OMPBuilder.createOMPInteropInit(Builder, InteropvarPtr, InteropType, 
Device,
----------------
ABataev wrote:
> Can we have anything else rather than `C->getIsTargetSync()` here? If no, 
> then it should look like this:
> ```
> if (C->getIsTarget()) {
>   InteropType = llvm::omp::OMPInteropType::Target;
> } else {
>   assert(C->getIsTargetSync() && "Expected ...");
>   InteropType = llvm::omp::OMPInteropType::TargetSync;
> }
> ```
The standard only allows for Target and TargetSync. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105876

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D105876... Sri Hari Krishna Narayanan via Phabricator via cfe-commits
    • [PATCH] D1... Johannes Doerfert via Phabricator via cfe-commits
    • [PATCH] D1... Alexey Bataev via Phabricator via cfe-commits
    • [PATCH] D1... Sri Hari Krishna Narayanan via Phabricator via cfe-commits
    • [PATCH] D1... Alexey Bataev via Phabricator via cfe-commits
    • [PATCH] D1... Sri Hari Krishna Narayanan via Phabricator via cfe-commits
    • [PATCH] D1... Johannes Doerfert via Phabricator via cfe-commits
    • [PATCH] D1... Sri Hari Krishna Narayanan via Phabricator via cfe-commits
    • [PATCH] D1... Sri Hari Krishna Narayanan via Phabricator via cfe-commits
    • [PATCH] D1... Alexey Bataev via Phabricator via cfe-commits
    • [PATCH] D1... Sri Hari Krishna Narayanan via Phabricator via cfe-commits
    • [PATCH] D1... Sri Hari Krishna Narayanan via Phabricator via cfe-commits
    • [PATCH] D1... Sri Hari Krishna Narayanan via Phabricator via cfe-commits

Reply via email to