[SYCL] Implement coverage instrumentation for device code #20710
Conversation
This PR extends Clang's source-based code coverage to work with SYCL device code. It includes the following changes: 1. The `InstrProfilingLoweringPass` was updated to lower profiling counters to SYCL device globals. 2. A new function was added to the compiler runtime to increment the host-side profiling counters. 3. The SYCL runtime was updated to send the contents of the device-side profiling counters to the new compiler runtime function when their device global map entries are being freed. This feature may not work correctly for functions that differ between host and device due to the use of the `__SYCL_DEVICE_ONLY__` macro. In such cases, it may not be possible to correlate the profiling counters from the device to the host. Resolves intel#7803. --------- Signed-off-by: Michael Aziz <[email protected]> Co-authored-by: Steffen Larsen <[email protected]> Signed-off-by: Michael Aziz <[email protected]>
Signed-off-by: Michael Aziz <[email protected]>
Signed-off-by: Michael Aziz <[email protected]>
|
@steffenlarsen, @AlexeySachkov, could you please help review this updated PR. The new changes added to fix the post-commit failures are in 40f2af7 and 3d8fee6. |
| CmdArgs.push_back("-fdeclare-spirv-builtins"); | ||
|
|
||
| // Set the atomic profile update flag to increment counters atomically. | ||
| CmdArgs.push_back("-fprofile-update=atomic"); |
There was a problem hiding this comment.
Should this option only be passed when instrumentation is enabled? Also, please add/update a test verifying that this option being passed when needed.
steffenlarsen
left a comment
steffenlarsen
left a comment
There was a problem hiding this comment.
Let's try throwing it at post-commit again and see what happens! 🚀
Push new branch to origin and trigger a manual post-commit run: |
AlexeySachkov
left a comment
AlexeySachkov
left a comment
There was a problem hiding this comment.
The two extra commits LGTM
0x12CC
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
| @@ -1386,11 +1385,7 @@ | |||
| options::OPT_fno_profile_generate, // -f[no-]profile-generate | |||
| options::OPT_ftest_coverage, | |||
| options::OPT_fno_test_coverage, // -f[no-]test-coverage | |||
There was a problem hiding this comment.
If there are tests added for these options, please make sure they are removed as well.
|
@aelovikov-intel, could you please run the post-commit testing? I don't think I have the required permissions to create a new branch or manually run any workflow. |
aelovikov-intel
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
https://github.com/intel/llvm/actions/runs/19546938418 Please ping me once that finishes to remove the branch. |
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
Thanks, @aelovikov-intel. It's done so you can delete the branch. The only test failure is in the Windows build: This is the same failure as in the post-commit testing for 2b96dd3 so it shouldn't be related to these changes. |
6 participants
This PR reapplies the changes from #20206. It also includes the following new changes:
compiler-rt.__sycl_increment_profile_counterswas added to fix build errors when building with Apple Clang.