intel
[SYCL][NATIVECPU] Enable source-based code coverage in Native CPU
Supports clang's source-based code coverage to enable code coverage testing of SYCL applications via the Native CPU SYCL target.
Clang's -fprofile-instr-generate -fcoverage-mapping options can now be used with the native_cpu SYCL target to compile/instrument host and device code, enabling 'llvm-cov' to render a coverage report after running the SYCL application (see also updated documentation in this PR).
Subsequent PRs will enable in NativeCPU more of the currently unsupported options for device compilation, also for performance profiling.
@intel/dpcpp-tools-reviewers @intel/dpcpp-cfe-reviewers @intel/dpcpp-clang-driver-reviewers please could you review? Thank you!
It isn't clear to me what the FE changes are doing in this patch. Please add a FE test if there is functionality that can be tested with these changes. It looks like you are only testing driver changes here. If that is the only functionality that can be tested at the moment, please remove FE changes from this PR and add them when it can be tested.
@elizabethandrews I have updated the PR description with more details and explanations about the changes, which are all covered by existing tests. Please let me know if anything needs further clarification. Thanks
It isn't clear to me what the FE changes are doing in this patch. Please add a FE test if there is functionality that can be tested with these changes. It looks like you are only testing driver changes here. If that is the only functionality that can be tested at the moment, please remove FE changes from this PR and add them when it can be tested.
@elizabethandrews I have updated the PR description with more details and explanations about the changes, which are all covered by existing tests. Please let me know if anything needs further clarification. Thanks
@uwedolinsky FE changes need FE tests. The tests you added are to test driver changes and an E2E test verifying FE changes. I do not think this is sufficient. IIUC you patch should have changed source locations in AST. This should be tested via a FE AST test.
@uwedolinsky FE changes need FE tests. The tests you added are to test driver changes and an E2E test verifying FE changes. I do not think this is sufficient. IIUC you patch should have changed source locations in AST. This should be tested via a FE AST test.
@elizabethandrews I've added a new AST test to check that the compound statement of the kernel body has a valid source location. Is that acceptable?
@uwedolinsky FE changes need FE tests. The tests you added are to test driver changes and an E2E test verifying FE changes. I do not think this is sufficient. IIUC you patch should have changed source locations in AST. This should be tested via a FE AST test.
@elizabethandrews I've added a new AST test to check that the compound statement of the kernel body has a valid source location. Is that acceptable?
Yes. Thank you
@intel/llvm-gatekeepers this PR looks ready to merge. The approval from @intel/dpcpp-tools-reviewers seems no longer required because I reverted a modifcation to a file owned by this group. Please could you double-check? Thank you