This is an archive of the discontinued LLVM Phabricator instance.

[Parallel] Add support for parallel execution policies to match C++ Parallelism TS
ClosedPublic

Authored by zturner on May 9 2017, 1:33 PM.

Details

Summary

This is the next step towards moving LLD's parallel algorithms over to LLVM.

With this patch, I introduce parallel_execution_policy, sequential_execution_policy, and is_execution_policy to as described in N4507.

It does not introduce the type erasing dynamic execution policy.

All callers of existing parallel algorithms are updated to use the new function syntax, and parallel_for is renamed to for_each_n to match the specification.

After this change, I am hoping this library can be moved to LLVM wholesale with no further changes.

Diff Detail

Repository
rL LLVM

Event Timeline

zturner created this revision.May 9 2017, 1:33 PM
ruiu added inline comments.May 9 2017, 2:17 PM
lld/include/lld/Core/Parallel.h
32–33 ↗(On Diff #98346)

nit: do you need {}?

176 ↗(On Diff #98346)

I think you can remove detail::sequential_sort and use std::sort instead.

190 ↗(On Diff #98346)

This function is called once. You can remove the function and inline.

ruiu edited edge metadata.May 9 2017, 3:38 PM

I think I prefer inlining them as sequential_sort and sequential_for_each_n are in the end just sort and for_each.

ruiu accepted this revision.May 9 2017, 3:50 PM

LGTM with inlining.

This revision is now accepted and ready to land.May 9 2017, 3:50 PM
zturner updated this revision to Diff 98365.May 9 2017, 4:14 PM

Inlined sequential functions. I know you already LGTM'ed it, but I'm re-uploading because I want to get feedback from chandlerc@ before submitting. I'd like to be sure this is close enough to what is needed from the Parallelism TS that no more major changes are required before trying to move this down to LLVM again. I will, of course, still submit another review to LLVM for the final move, but I just want to make sure this is close enough to try again.

zturner updated this revision to Diff 98379.May 9 2017, 5:40 PM

I went ahead and added the dynamic execution policy.

chandlerc edited edge metadata.May 9 2017, 6:00 PM

Other than a YAGNI argument against the type-erased stuff, seems fine to me.

lld/include/lld/Core/Parallel.h
54 ↗(On Diff #98379)

This seems like a *lot* of machinery for one use case in one part of LLD.... I'd honestly just not implement this until someone really wants it because they need it....

This revision was automatically updated to reflect the committed changes.