-
Notifications
You must be signed in to change notification settings - Fork 769
[DevTSAN] Support detecting data race for local memory #18718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR extends ThreadSanitizer support to detect races in local (shared) memory by allocating and initializing a separate shadow region per workgroup and passing local-argument metadata through the interceptor and runtime.
- Added
AllocLocalShadow
API toShadowMemory
and provided CPU/GPU implementations. - Enhanced the interceptor and DDI to record local kernel arguments and calculate per-kernel local shadow offsets.
- Updated the device runtime (
tsan_rtl.cpp
), SPIR-V instrumentation, and end-to-end tests to initialize, clean up, and report races on local memory.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
tsan_shadow.hpp & tsan_shadow.cpp | Defined AllocLocalShadow() and implemented it for CPU/GPU shadows. |
tsan_interceptor.hpp & tsan_interceptor.cpp | Recorded local arg sizes, computed workgroup counts, and invoked AllocLocalShadow . |
tsan_rtl.cpp | Mapped local memory to shadow space, added cleanup hooks, and marked local races. |
llvm/.../ThreadSanitizer.cpp | Instrumented static/dynamic local memory in SPIR-V and factored out common utils. |
Comments suppressed due to low confidence (2)
unified-runtime/source/loader/layers/sanitizer/tsan/tsan_interceptor.hpp:148
- [nitpick] Constructor parameter LocalWorkSize shadows the member of the same name; consider renaming the parameter (e.g. pLocalWorkSize) to avoid confusion.
const size_t *GlobalWorkSize, const size_t *LocalWorkSize,
unified-runtime/source/loader/layers/sanitizer/tsan/tsan_report.cpp:30
- [nitpick] The updated warning message only indicates memory type and omits the actual race address, which may reduce debugging clarity; consider including the address in the log string.
"====WARNING: DeviceSanitizer: data race on {}",
unified-runtime/source/loader/layers/sanitizer/tsan/tsan_shadow.cpp
Outdated
Show resolved
Hide resolved
unified-runtime/source/loader/layers/sanitizer/tsan/tsan_shadow.cpp
Outdated
Show resolved
Hide resolved
unified-runtime/source/loader/layers/sanitizer/tsan/tsan_interceptor.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one nit question.
Hi @intel/unified-runtime-reviewers, could you please help review this PR? Thanks. |
llvm/include/llvm/Transforms/Instrumentation/SPIRVSanitizerCommonUtils.h
Show resolved
Hide resolved
Hi @intel/llvm-gatekeepers, this PR is ready to be merged. Thanks. |
Uh oh!
There was an error while loading. Please reload this page.