llvm
llvm copied to clipboard
[DeviceSanitizer] Support detecting out-of-bounds error on DeviceGlobals
UR part: https://github.com/oneapi-src/unified-runtime/pull/1354
Hi, @zhaomaosu Does this patch support following scenario: clang++ -fsycl -fsanitize=address -c user_code1.cpp -o user1.o clang++ -fsycl -fsanitize=address -c user_code2.cpp -o user2.o clang++ -fsycl user1.o user2.o -o user.exe Both user_code1.cpp and user_code2.cpp include device globals. Thanks.
Hi, @zhaomaosu Does this patch support following scenario: clang++ -fsycl -fsanitize=address -c user_code1.cpp -o user1.o clang++ -fsycl -fsanitize=address -c user_code2.cpp -o user2.o clang++ -fsycl user1.o user2.o -o user.exe Both user_code1.cpp and user_code2.cpp include device globals. Thanks.
Hi, @zhaomaosu Could you add some tests to cover this scenario? Thanks.
Hi, @zhaomaosu Could you add some tests to cover this scenario? Thanks.
Ok, I'll add tests for it.
Hi, @zhaomaosu Could you add some tests to cover this scenario? Thanks.
Done
@intel/llvm-reviewers-runtime , @intel/dpcpp-tools-reviewers could you please help review this PR? Thanks.
Hi @steffenlarsen, I noticed there are some failures in GEN12 tests, but I think my changes will not affect it. Do you know if these are known failures?
The Gen12 Windows failures are reported in https://github.com/intel/llvm/issues/12797 and https://github.com/intel/llvm/issues/12798 so they are completely unrelated to these changes.
The Linux one I've heard was a problem in passing. I suspect it is also unrelated.
Hi @steffenlarsen, the PR in UR repo has been merged. I updated the UR tag. May I get your approval?
Review still required from @intel/unified-runtime-reviewers & @intel/dpcpp-tools-reviewers. Friendly ping.
@maksimsab, any other comments?
@maksimsab if the changes you requested are sufficient please approve the PR.
@intel/dpcpp-esimd-reviewers please review this ASAP, this PR is blocking any progress of merging other UR changes.
This is affecting multiple teams. Any help to expedite the approval and merging of this PR would be greatly appriciated.
I am OK to let this PR go in order to unblock other teams. Currently, the new Pass has 0 test coverage. @zhaomaosu Are you ok to address this in the follow-up PR?
I am OK to let this PR go in order to unblock other teams. Currently, the new Pass has 0 test coverage. @zhaomaosu Are you ok to address this in the follow-up PR?
Sure, I'll do this later. Thank you.
Can anyone help merge this PR? Thanks a lot.