Details
- Reviewers
jdoerfert
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Just some minor nits; the bulk of this is above my pay grade. :)
pstl/include/pstl/internal/gcd/parallel_invoke.h | ||
---|---|---|
39 | Here and line 34, extraneous whitespace. (Weird spacing throughout, honestly, but that's clang-format for you.) | |
pstl/include/pstl/internal/gcd/parallel_merge.h | ||
71 | Here and throughout, FYI, the code is not ADL-proof, which means it'll have this problem: | |
pstl/include/pstl/internal/gcd/parallel_stable_sort.h | ||
110 | (1) It kinda looks like __pattern_sort (pstl/include/pstl/internal/algorithm_impl.h line 2198) can call this with __xe == __xs. But maybe I'm wrong. |
pstl/include/pstl/internal/gcd/parallel_invoke.h | ||
---|---|---|
39 | I have re-run clang-format on the code. Not sure what else to do about it. | |
pstl/include/pstl/internal/gcd/parallel_merge.h | ||
71 | Good point. I've tried to fix this everywhere, in the same way that the OpenMP backend fixes it. | |
pstl/include/pstl/internal/gcd/parallel_stable_sort.h | ||
110 | Good catch, thank you! |
pstl/include/pstl/internal/gcd/parallel_invoke.h | ||
---|---|---|
30 | Is there a reason you use ::std here instead of std? | |
pstl/include/pstl/internal/gcd/parallel_stable_partial_sort.h | ||
30 | This isn't ADL-proof. | |
pstl/include/pstl/internal/gcd/util.h | ||
105 | Also not ADL-proof | |
pstl/include/pstl/internal/parallel_backend_gcd.h | ||
15–17 | These comments seem weird to me. |
No. I suppose it's been so long that it won't apply cleanly. I would really like some help getting a builder to turn this on for macOS automatically so that it can run as part of the normal test suite. I can try to carve out some time to rebase it and re-test it. I'm not really sure what anyone is waiting for...
The problem is that the PSTL isn't tested currently, and because of that bit-rotted. We're trying to get it working properly, but it's not exactly a high priority.
[Github PR transition cleanup]
Removed libc++ from this review since we no longer use the code in <monorepo-root>/pstl.
Is there a reason you use ::std here instead of std?