-
Notifications
You must be signed in to change notification settings - Fork 61
Adding asan sanitizer support for hip #795
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
base: develop
Are you sure you want to change the base?
Conversation
|
Updated cmake command: Updated Output (for hip kernel sanitizer test): Note: Will not work if |
|
Issue: it seems like running the test with |
|
The way this test is ran in |
|
See AMD bug report (277) - made reproducer for AMD to look in to. Seems like this does not yet work as expected in rocm 5.4 |
|
Update: A fix is supposed to be ready in rocm 6.0 |
…an-poisoning-for-hip
|
Edited out... |
…an-poisoning-for-hip
…-1020-asan-poisoning-for-hip
|
The fix is in rocm/6.0.0, but we have to add a |
|
This has been a long time coming! @adrienbernede - once the blt/camp releases come out and we can update umpire to rocm/6.0.0 we should add this test! (No updates yet for the reason behind why the device allocator test is failing with 6.0.0 - ticket has been "submitted to vendor", so it is being looked into at least...) |
|
@kab163 @davidbeckingsale now that the allocation is not limiting anymore I’ll let you deal with the rest ;). |
…an-poisoning-for-hip
|
Updated cmake command: or |
|
Built with then run or |
|
What's the current status of this merge request? I'm testing out asan and finding it works for simple tests. I'm working on enabling it in my application, but until this goes in, I'll have to disable memory pools. |
…an-poisoning-for-hip
|
Is this ready for review? |
Whether I'm running on rzvernal or rzadams, I see tests fail with memory leaks detected. So for example, on rzvernal: or on rzadams: |
|
I'm using rocm/6.3.0 and the build recipe above from late December last year |
|
Update: I updated the cmake to:
so that it reflects the correct hardware for rzadams (gfx942) and I load the rocm/6.3.0 module, but I still end up getting similar failures: |
|
Have you tried rocm 6.3.1? I know at least one internal rocm memory issue was fixed between rocm 6.3.0 and rocm 6.3.1. |
I have not tried that, but I can try that now! |
…an-poisoning-for-hip
…an-poisoning-for-hip
|
Had some troubles compiling all of a sudden, but was able to compile again after adding a new flag. Updated cmake: |
| __global__ void test_write_for_hip(double* data_ptr, std::size_t INDEX) | ||
| { | ||
| if (threadIdx.x == 0) { | ||
| data_ptr[INDEX] = 256; |
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.
Are two writes needed?
|
|
||
| if (strategy.find("QuickPool") != std::string::npos) { | ||
| auto pool = rm.makeAllocator<umpire::strategy::QuickPool>("test_allocator", rm.getAllocator("HOST")); | ||
| if ((resource_type.find("DEVICE") != std::string::npos) || (resource_type.find("UM") != std::string::npos)) { |
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.
I think we might actually want == instead of find.
| run_sanitizer_test('DynamicPoolList', 'read', 'HOST') | ||
| run_sanitizer_test('DynamicPoolList', 'write', 'HOST') | ||
| run_sanitizer_test('QuickPool', 'read', 'HOST') | ||
| run_sanitizer_test('QuickPool', 'write', 'HOST') |
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.
Any device tests?
I did not see any leaks with rocm 6.4.1 on rzadams with blt_hip_smoke or resource_manager_tests! |
How did you build it? |
Cmake command on rzvernal:
Warning while compiling:
clang-15: warning: ignoring '-fsanitize=address' option for offload arch 'gfx90a' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead [-Woption-ignored]Output of
sanitizer_tests.cpp