[SYCL] Enable parallel execution of llvm-foreach commands under an option #4360
Conversation
…tion This improvement should speed up toolchain execution by launching underlying llvm-foreach commands in parallel. The feature can be enabled by passing '--parallel-exec' to the command line of llvm-foreach.
vmaksimo
force-pushed
the
foreach_parallel_exec
branch
from
41dc82c to
535fb60
Compare
|
/summary:run |
| continue; | ||
| } | ||
| if (WaitResult.ReturnCode != 0) { | ||
| errs() << "llvm-foreach: " << ErrMsg << '\n'; |
There was a problem hiding this comment.
Ideally, we should have launched command in the error message so it could be launched standalone to debug it.
There was a problem hiding this comment.
Good idea! We should maintain a map with the process id and the executed command (didn't find a way to get this info just from the process id) to support this with the parallel execution. I'll do it in a separate PR after this one is merged.
|
@AlexeySachkov thanks a lot for the review! I'll publish a patch with applied changes soon. |
Co-authored-by: Alexey Sachkov <alexey.sachkov@intel.com>
|
@kbobrovs, @mdtoguchi : FYI. Note: for now we do not yet introduce any way of how to use this new functionality in SYCL/DPC++ toolchain, because we haven't yet settled down on how to properly document it and when to recommend for our users |
| Res = checkIfJobsAreFinished(JobsSubmitted, /*BlockingWait*/ false); | ||
|
|
||
| JobsSubmitted.emplace_back(sys::ExecuteNoWait( | ||
| Prog, Args, /*Env=*/None, /*Redirects=*/None, /*MemoryLimit=*/0)); |
There was a problem hiding this comment.
If Res above is not zero, new jobs should not probably be started. Instead, all running could be killed and the tool would just exit with non-zero.
There was a problem hiding this comment.
Yeah, it's a good thought, but it was done on purpose in c69a311:
"we also want to allow for the compilation to continue so the user can use the generated binary knowing the timing issue as stated.").
I believe I have a bug here (should not assign the result to Res if it was successful), I'll apply the fix.
There was a problem hiding this comment.
"we also want to allow for the compilation to continue so the user can use the generated binary knowing the timing issue as stated."
checkIfJobsAreFinished may return non-zero for a variety of reasons, not only due to a timeout. In fact timeout is not even possible, AFAICS, as SecondsToWait for sys::Wait is 0. So I believe the most likely reason for checkIfJobsAreFinished non-zero return is some error during tool execution, in which case the results will be discarded, and it does not seem to make sense to run any other tools after that.
There was a problem hiding this comment.
I suggest that we discuss that matter separately and apply some changes (if needed) through a separate PR: apparently there was a decision to not to stop submitting further jobs if one of them failed when we executed them in serial manner. It is not clear to me why we shouldn't follow the same logic for parallel execution.
Submitting further jobs after the first failed also allows us to detect all issues in all submitted jobs and not just in the first one.
There was a problem hiding this comment.
Submitting further jobs after the first failed also allows us to detect all issues in all submitted jobs and not just in the first one.
that does not seem right to me, and can be resource-costly approach. All jobs in a llvm-foreach batch are similar and in most cases they will fail for the same reason, I believe.
|
@vmaksimo PR has some unresolved threads. Is there any additional work expected in this PR? |
@dm-vodopyanov I believe there is no more work expected - all unresolved discussions can be addressed in separate PRs. |
Thanks |
…tion (intel#4360) This improvement should speed up toolchain execution by launching underlying llvm-foreach commands in parallel. The feature is available under `--jobs`/`-j` option that should be passed to the command line of llvm-foreach.
After intel#4360 extended `llvm-foreach` with an option to specify the desired number of parallel jobs, expose this functionality to the DPC++ compiler users. Co-authored-by: Alexey Sachkov <alexey.sachkov@intel.com> Co-authored-by: Artem Gindinson <artem.gindinson@intel.com>
This improvement should speed up toolchain execution by launching underlying llvm-foreach commands in parallel.
The feature is available under
--jobs/-joption that should be passed to the command line of llvm-foreach.