[PATCH] D34038: Implement the non-parallel versions of exclusive_scan and transform_exclusive_scan

wash added a comment. This implementation works, but performs unnecessary operations. https://gist.github.com/brycelelbach/907ac3b8a74603cc189897ab789a65a9 The "next" result is calculated in each iteration; this means one unnecessary application of the binary op (the "next" result on the final

[PATCH] D33997: Implement the non-execution policy versions of `reduce` and `transform_reduce` for C++17

wash added a comment. I think the test `reduce_iter_iter_T.pass.cpp` can be improved a little bit. Right now, it: - Tests that the three argument overload (iterators + init) work correctly when the iterator value type is the same as the init type. - Tests that the return type of the three

[PATCH] D33997: Implement the non-execution policy versions of `reduce` and `transform_reduce` for C++17

wash added a comment. Suppose you have: struct A {}; struct B {}; A operator+(A, B); std::vector v; std::reduce(v.begin(), v.end(), A{}); The implementation in this patch works fine for the above case - as would `accumulate`, which it is equivalent to. However, `reduce`

[PATCH] D33997: Implement the non-execution policy versions of `reduce` and `transform_reduce` for C++17

wash added a comment. For all the `transform_` variants, the spec allows the "inner" operation (the transformation)'s return type is not constrained. We should have a test for this. For example, consider the binary-unary `transform_reduce` implementation: template inline

[PATCH] D34007: Implement inclusive_scan and transform_inclusive_scan

wash added a comment. Minor note: there's a mix of tabs and spaces in this diff. https://reviews.llvm.org/D34007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34007: Implement inclusive_scan and transform_inclusive_scan

wash added a comment. So, `inclusive_scan` should be equivalent to `partial_sum` for the non-parallel version. https://reviews.llvm.org/D34007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34007: Implement inclusive_scan and transform_inclusive_scan

wash added a comment. Here's `partial_sum`: template inline _LIBCPP_INLINE_VISIBILITY _OutputIterator partial_sum(_InputIterator __first, _InputIterator __last, _OutputIterator __result, _BinaryOperation __binary_op) { if (__first != __last) {

[PATCH] D33997: Implement the non-execution policy versions of `reduce` and `transform_reduce` for C++17

wash added a comment. I think we need a test case like this for all of the `transform_*`s struct A {}; struct B {}; struct C {}; B unary_op(C); B unary_op(A) { assert(false); /* unary op applied to init value! */ } A binary_op(A, A); A binary_op(A, B); A binary_op(B, A); A

[PATCH] D34038: Implement the non-parallel versions of exclusive_scan and transform_exclusive_scan

wash added inline comments. Comment at: include/numeric:182 +{ + _Tp __saved = __init; + for (; __first != __last; ++__first, (void) ++__result) { If `__first == __last`, this initialization is unnecessary; I'd refactor this so that we check for

[PATCH] D34038: Implement the non-parallel versions of exclusive_scan and transform_exclusive_scan

wash added a comment. https://gist.github.com/brycelelbach/137f1e45b737d615134e228ec0c84f3a <- some crude tests I wrote based on slides/notes of mine. https://reviews.llvm.org/D34038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34038: Implement the non-parallel versions of exclusive_scan and transform_exclusive_scan

wash added a comment. The init typedef test looks good to me. This all looks good to go. https://reviews.llvm.org/D34038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits