Skip to content

Commit 02b32b5

Browse files
author
Pavel Samolysov
authored
[sycl-post-link] Remove padding from DeviceGlobalProperty (#5848)
As it is described in [1], MemorySanitizer emits an unexpected report about an uninitialized value when we are reading a structure with padding as a byte array. The patch implements a workaround: the type of the DeviceImageScope flag has been replaced with uint32_t to not require a space for padding. The change should have no impact on performance and ABI because the size is the same. [1] llvm/llvm-project#54476
1 parent 6b4798c commit 02b32b5

File tree

3 files changed

+12
-11
lines changed

3 files changed

+12
-11
lines changed

llvm/test/tools/sycl-post-link/device-globals/test_global_variable.ll

+5-6
Original file line numberDiff line numberDiff line change
@@ -106,16 +106,15 @@ attributes #6 = { "sycl-unique-id"="6da74a122db9f35d____ZL7no_dg_int1" "device_i
106106
; 1. 8 bytes denoting the bit-size of the byte array, here 64 bits or 8 bytes.
107107
; 2. 4 bytes with the value of the 32-bit uint32_t integer with the size of the
108108
; underlying type of the device global variable. Its value being 1.
109-
; 3. 1 byte with the value of the 8-bit uint8_t integer with the flag that
109+
; 3. 4 byte with the value of the 32-bit uint32_t integer with the flag that
110110
; the device global variable has the 'device_image_scope' property.
111111
; Its value being 1, property is present.
112-
; 4. Any 3 bytes used as padding to align the structure to 8 bytes.
113112
;
114113
; CHECK-PROP: [SYCL/device globals]
115-
; CHECK-PROP-NEXT: 6da74a122db9f35d____ZL7dg_int1=2|ABAAAAAAAAABAAAAA
116-
; CHECK-PROP-NEXT: 7da74a1187b9f35d____ZL7dg_int2=2|ABAAAAAAAAABAAAAA
117-
; CHECK-PROP-NEXT: 9d329ad59055e972____ZL8dg_bool3=2|ABAAAAAAAAQAAAAAB
118-
; CHECK-PROP-NEXT: dda2bad52c45c432____ZL8dg_bool4=2|ABAAAAAAAAQAAAAAB
114+
; CHECK-PROP-NEXT: 6da74a122db9f35d____ZL7dg_int1=2|ABAAAAAAAAABAAAAAAAAAA
115+
; CHECK-PROP-NEXT: 7da74a1187b9f35d____ZL7dg_int2=2|ABAAAAAAAAABAAAAAAAAAA
116+
; CHECK-PROP-NEXT: 9d329ad59055e972____ZL8dg_bool3=2|ABAAAAAAAAQAAAAABAAAAA
117+
; CHECK-PROP-NEXT: dda2bad52c45c432____ZL8dg_bool4=2|ABAAAAAAAAQAAAAABAAAAA
119118
;
120119
; The variable is not a device global one and must be ignored
121120
; CHECK-PROP-NOT: 6da74a122db9f35d____ZL7no_dg_int1

llvm/tools/sycl-post-link/DeviceGlobals.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ struct DeviceGlobalProperty {
3535

3636
// Either 1 (true) or 0 (false), telling whether the device global variable
3737
// was declared with the device_image_scope property.
38-
uint8_t DeviceImageScope;
38+
// We use uint32_t for a boolean value to eliminate padding after the field
39+
// and suppress false positive reports from MemorySanitizer.
40+
uint32_t DeviceImageScope;
3941
};
4042

4143
using DeviceGlobalPropertyMapTy =

sycl/source/detail/program_manager/program_manager.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -1217,13 +1217,13 @@ void ProgramManager::addImages(pi_device_binaries DeviceBinary) {
12171217
// The supplied device_global info property is expected to contain:
12181218
// * 8 bytes - Size of the property.
12191219
// * 4 bytes - Size of the underlying type in the device_global.
1220-
// * 1 byte - 0 if device_global has device_image_scope and any value
1220+
// * 4 bytes - 0 if device_global has device_image_scope and any value
12211221
// otherwise.
1222-
// Note: Property may be padded.
1223-
assert(DeviceGlobalInfo.size() >= 13 && "Unexpected property size");
1222+
assert(DeviceGlobalInfo.size() == 16 && "Unexpected property size");
12241223
const std::uint32_t TypeSize =
12251224
*reinterpret_cast<const std::uint32_t *>(&DeviceGlobalInfo[8]);
1226-
const std::uint32_t DeviceImageScopeDecorated = DeviceGlobalInfo[12];
1225+
const std::uint32_t DeviceImageScopeDecorated =
1226+
*reinterpret_cast<const std::uint32_t *>(&DeviceGlobalInfo[12]);
12271227
Entry->second.initialize(TypeSize, DeviceImageScopeDecorated);
12281228
}
12291229
}

0 commit comments

Comments
 (0)