[DeviceSanitizer] Generate unique sycl-unique-id for device global. #17649
Conversation
| // Create a memory buffer to hold the serialized module | ||
| std::string Buffer; | ||
| llvm::raw_string_ostream Stream(Buffer); | ||
| llvm::WriteBitcodeToFile(M, Stream); |
There was a problem hiding this comment.
Using the entire bitcode to do the hash seems quite heavy. Could we use others to do that? @wenju-he what do you think?
There was a problem hiding this comment.
I don't think WriteBitcodeToFile should be used here just for computing hash.
llvm::StructuralHash can be used to compute hash from module.
Does the hash needs to be different for different modules?
There was a problem hiding this comment.
Does the hash needs to be different for different modules?
@wenju-he, I think yes, https://github.com/intel/llvm/blob/sycl/sycl/test-e2e/DeviceLib/bfloat16_conversion_test.cpp has 2 modules, one is BUILD_LIB code for dynamic link, another is BUILD_EXE code. If the 2 modules have same sycl-unique-id, test will assert fail in DeviceGlobalMapEntry::initialize https://github.com/intel/llvm/blob/sycl/sycl/source/detail/device_global_map_entry.hpp#L92.
There was a problem hiding this comment.
https://github.com/intel/llvm/blob/sycl/sycl/test-e2e/DeviceLib/bfloat16_conversion_test.cpp has 2 modules
When sanitizer isn't enabled, how is sycl-unique-id handled for the case if there is a device globals in the test?
There was a problem hiding this comment.
I think this is because we haven't handled "dynamic link" properly.
This is a new feature for device sanitizer, not a quick bug fix.
There was a problem hiding this comment.
Feel free to issue a JIRA to track this.
There was a problem hiding this comment.
https://github.com/intel/llvm/blob/sycl/sycl/test-e2e/DeviceLib/bfloat16_conversion_test.cpp has 2 modules
When sanitizer isn't enabled, how is sycl-unique-id handled for the case if there is a device globals in the test?
@wenju-he , sycl-unique-id is generated by dpcpp FE, The rules for creating this string are the same as __builtin_sycl_unique_stable_id https://github.com/intel/llvm/blob/sycl/sycl/doc/design/DeviceGlobal.md#changes-to-the-dpc-front-end. We generate fixed sycl-unique-id in DeviceSanitizer.
Do you mean we want same implementation for DeviceSanitizer attributes? Thanks.
There was a problem hiding this comment.
When sanitizer isn't enabled, how is sycl-unique-id handled for the case if there is a device globals in the test?
For the above case, is it right that there is only one device global when sanitizer is disabled. However, if sanitizer is enabled, there are two distinct device globals which are supposed to be the same one?
There was a problem hiding this comment.
When sanitizer isn't enabled, how is sycl-unique-id handled for the case if there is a device globals in the test?
For the above case, is it right that there is only one device global when sanitizer is disabled. However, if sanitizer is enabled, there are two distinct device globals which are supposed to be the same one?
@wenju-he, when sanitizer is disabled, there is no device global. The device global is added by device sanitizer, for BUILD_LIB module, it's 2xi64 array which is ptrtoint for foo and main.
@__AsanKernelMetadata = dso_local local_unnamed_addr addrspace(1) global %0 { [2 x { i64, i64 }] [{ i64, i64 } { i64 ptrtoint (ptr addrspace(2) @__asan_kernel to i64), i64 61 }, { i64, i64 } { i64 ptrtoint (ptr addrspace(2) @__asan_kernel.1 to i64), i64 50 }] }, !spirv.Decorations !0 #0
for BUILD_EXE, it is ptrtoint for main.
@__AsanKernelMetadata = dso_local local_unnamed_addr addrspace(1) global %0 { [1 x { i64, i64 }] [{ i64, i64 } { i64 ptrtoint (ptr addrspace(2) @__asan_kernel to i64), i64 50 }] }, !spirv.Decorations !0 #0
Yes, the 2 @__AsanKernelMetadata has same main since main is compiled on both BUILD_EXE and BUILD_EXE.
There was a problem hiding this comment.
when sanitizer is disabled, there is no device global.
Yeah, in the question there is a condition that if there is a device globals in the test
I understand that sanitizer adds new device global.
|
All device Sanitizer such as Address Sanitizer, Memory Sanitizer and thread Sanitizer need similar change, the rename can be added to common utilts SPIRVSanitizerCommonUtils #17597. |
4 participants
According to https://github.com/intel/llvm/blob/sycl/sycl/doc/design/DeviceGlobal.md#changes-to-the-dpc-front-end, LLVM IR attribute sycl-unique-id is the definition of each device_global variable, which provides a unique string identifier for each device global variable.