llvm
llvm copied to clipboard
[DeviceSanitizer] Support nullpointer detection & enable GPU tests
UR: https://github.com/oneapi-src/unified-runtime/pull/1914
Maybe we need to wait for cpu&gpu driver upgrading to support "__devicelib_exit" builtin.
gpu e2e are skiped, can't find cpu e2e results
Maybe we need to wait for cpu&gpu driver upgrading to support "__devicelib_exit" builtin.
Recently upgraded gpu driver in https://github.com/intel/llvm/pull/14838 has support of "__devicelib_exit" builtin
@intel/unified-runtime-reviewers, please review
Kindly ping @intel/unified-runtime-reviewers
@intel/llvm-gatekeepers please merge, Thanks!
Approval still needed from @intel/dpcpp-sanitizers-review (e.g. @yingcong-wu )
@intel/llvm-gatekeepers This should be ready now. Please merge.
@AllanZyne - The following tests are failing in post-commit after this: SYCL :: AddressSanitizer/invalid-argument/out-of-bounds.cpp SYCL :: AddressSanitizer/invalid-argument/released-pointer.cpp SYCL :: AddressSanitizer/nullpointer/global_nullptr.cpp Please address these ASAP or we will will have to revert the patch.
@AllanZyne - The following tests are failing in post-commit after this: SYCL :: AddressSanitizer/invalid-argument/out-of-bounds.cpp SYCL :: AddressSanitizer/invalid-argument/released-pointer.cpp SYCL :: AddressSanitizer/nullpointer/global_nullptr.cpp Please address these ASAP or we will will have to revert the patch.
Hi @steffenlarsen , @AllanZyne is OOO for now, so I think you should revert the patch.
@yingcong-wu - Thank you for making me aware! Looking at the changes, I don't think we need a full revert. If I'm reading it correctly, it seems like we didn't enable DG2 testing before, so I think it would be better to just disable the failing tests. Note that global_nullptr.cpp just seems to be failing to compile, likely due to unlucky timing, so I aim to fix that too. Patch in https://github.com/intel/llvm/pull/15450.
Thank you for making me aware! Looking at the changes, I don't think we need a full revert. If I'm reading it correctly, it seems like we didn't enable DG2 testing before, so I think it would be better to just disable the failing tests.
@steffenlarsen, as far as I remember our discussion in SYCL CI meetings - it was an conclusion and agreement what we won't disable failing tests without at least tracker being created in regards of this failures.
Was it done in this case? I maybe wrong, but I don't see any tracker being mention in this PR in regards of this disabled tests and I also do not see it in https://github.com/intel/llvm/pull/15450.
Thank you for making me aware! Looking at the changes, I don't think we need a full revert. If I'm reading it correctly, it seems like we didn't enable DG2 testing before, so I think it would be better to just disable the failing tests.
@steffenlarsen, as far as I remember our discussion in SYCL CI meetings - it was an conclusion and agreement what we won't disable failing tests without at least tracker being created in regards of this failures.
Was it done in this case? I maybe wrong, but I don't see any tracker being mention in this PR in regards of this disabled tests and I also do not see it in #15450.
Hi @nsirgien! You're right, I did not mention the trackers in the PR itself. However, they are mentioned in the test files above the line disabling them. The trackers are https://github.com/intel/llvm/issues/15449 and https://github.com/intel/llvm/issues/15453.