[PATCH] D92476: [OpenMPIRBuilder] introduce createStaticWorkshareLoop

[Alex Zinenko via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1032
+  // Set up the source location value for OpenMP runtime.
+  updateToLocation(Loc);
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
----------------
fghanim wrote:
> Nit: To keep a consistent behavior throughout the builder, please change to:
> ```
> if (!updateToLocation(Loc))
>     return Loc.IP;
> ```
It's already inconsistent, createCanonicalLoop doesn't follow this pattern, and we cannot return `Loc.IP` here because the return type is `CanonicalLoopInfo *`. Returning nullptr instead.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1106-1108
+    FunctionCallee BarrierFunc =
+        getOrCreateRuntimeFunction(M, omp::OMPRTL___kmpc_barrier);
+    Builder.CreateCall(BarrierFunc, {SrcLoc, ThreadNum});
----------------
kiranchandramohan wrote:
> Nit: There is a create barrier function in the OpenMPIRBuilder.
> CreateBarrier(Builder.saveIP(), llvm::omp::OMPD_barrier);
Yes, but it would emit another call to get_thread_num and `OMPD_barrier` could misled somebody to think the emitted code comes from a barrier directive, which it is not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92476