[libcxx-commits] [PATCH] D99836: [pstl] Implement OpenMP backend

[Christopher Nelson via Phabricator via libcxx-commits]

nadiasvertex marked 2 inline comments as done.
nadiasvertex added a comment.

All of the serious comments were addressed. The nits were style differences I disagree with. :-) If these nits are LLVM required style I will adjust them.



================
Comment at: pstl/include/pstl/internal/omp/parallel_invoke.h:39
+    if (omp_in_parallel())
+    {
+        __parallel_invoke_body(std::forward<_F1>(__f1), std::forward<_F2>(__f2));
----------------
MikeDvorskiy wrote:
> Nit: redundant braces.
I prefer the braces to make it clear what is in the block and for maintenance reasons.


================
Comment at: pstl/include/pstl/internal/omp/parallel_merge.h:42
+    {
+        __ym = __ys + (__size_y / 2);
+        __xm = ::std::upper_bound(__xs, __xe, *__ym, __comp);
----------------
MikeDvorskiy wrote:
> We have redundant parentheses, according to the C++ operation priority list.
I prefer them.


================
Comment at: pstl/include/pstl/internal/omp/parallel_merge.h:47
+    {
+        __xm = __xs + (__size_x / 2);
+        __ym = ::std::lower_bound(__ys, __ye, *__xm, __comp);
----------------
MikeDvorskiy wrote:
> We have redundant parentheses, according to the C++ operation priority list.
I prefer them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99836