Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Mar 21, 2022

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

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
@ghost
Copy link
Author

ghost commented Mar 22, 2022

clang-cl.exe crashes during pre-processing USM/hmemll.cpp. This cannot be related to the current changes because the failure occurred in the compiler frontend before the sycl-post-link tool. I've opened a ticket.

8 bytes is the size of the data plus 8 bytes is the size of the size field.

Co-authored-by: Steffen Larsen <[email protected]>
steffenlarsen
steffenlarsen previously approved these changes Mar 22, 2022
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ghost ghost marked this pull request as ready for review March 22, 2022 11:34
@ghost ghost requested review from a team as code owners March 22, 2022 11:34
AlexeySachkov
AlexeySachkov previously approved these changes Mar 22, 2022
@ghost ghost dismissed stale reviews from AlexeySachkov and steffenlarsen via 3e18f2b March 22, 2022 14:27
@ghost ghost requested review from AlexeySachkov and steffenlarsen March 22, 2022 14:28
steffenlarsen
steffenlarsen previously approved these changes Mar 22, 2022
@bader bader requested a review from mlychkov March 22, 2022 14:34
AlexeySachkov
AlexeySachkov previously approved these changes Mar 22, 2022
@ghost ghost dismissed stale reviews from AlexeySachkov and steffenlarsen via fc2bd8f March 23, 2022 06:56
Co-authored-by: Mikhail Lychkov <[email protected]>
@ghost ghost requested review from mlychkov, AlexeySachkov and steffenlarsen March 23, 2022 06:57
@ghost
Copy link
Author

ghost commented Mar 23, 2022

I've opened a ticket for the SYCL :: Basic/parallel_for_indexers.cpp test failure on AMDGPU. There are no device globals and specialization constants in the test, so no properties should be generated at all. This is a little bit strange because the test is marked with XFAIL: hip_amd, so the failure should be expected.

@bader bader merged commit 02b32b5 into intel:sycl Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants