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