[PATCH] D131830: Clang Support for taskwait nowait clause

[Shilei Tian via Phabricator via llvm-commits]

tianshilei1992 added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:6132
+                                       const OMPTaskDataTy &Data,
+                                       bool HaveNoWaitClause) {
   if (!CGF.HaveInsertPoint())
----------------
better to use consistent name as declaration. how about `HasNowaitClause`?


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:634
+  void createTaskwait(const LocationDescription &Loc, int OpenMPVer = 0,
+                      bool HaveNoTaskWaitClause = false);
 
----------------
I'd keep the style consistent. Here there is a default argument `bool HaveNoTaskWaitClause = false` but no for `emitTaskwaitCall`. It's better to use default argument (potentially save a couple of changes) for both, or no default for both.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1253
+void OpenMPIRBuilder::emitTaskwaitImpl(const LocationDescription &Loc,
+                                       int OpenMPVer, bool HaveNoWaitClause) {
   // Build call kmp_int32 __kmpc_omp_taskwait(ident_t *loc, kmp_int32
----------------
Is the `OpenMPVer` necessary as an argument? Do we have alternative way to tell the version? If not, I think it makes more sense to take it as a data member of `OpenMPIRBuilder` instead of passing as a function argument.


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

https://reviews.llvm.org/D131830