[BACKEND] Add Address Sanitizer Pass #5127
Conversation
CRobeck
changed the title
CRobeck
changed the title
|
@antiagainst can you update LLVM hash to atleast: |
|
First step: #5179. |
|
Second step: #5180 |
CRobeck
changed the title
CRobeck
changed the title
CRobeck
requested review from
antiagainst,
zhanglx13 and
ptillet
as code owners
|
@antiagainst don't merge this yet. @AlexAUT is looking into why this seems to sometimes hang the MI300 CI. So far it appears random so we need to look into it a bit. |
| @@ -304,7 +304,8 @@ def compile(src, target=None, options=None): | |||
| # This is needed to safely finalize threads pool inside context: if current process forks before | |||
| # python GC deletes context object, thread pool in child process will be invalid, which could | |||
| # lead to child crash or hang. | |||
| context.disable_multithreading() | |||
| if not os.environ.get("TRITON_ENABLE_ASAN", "0") == "1": | |||
There was a problem hiding this comment.
Why should we enable multi-threading?
There was a problem hiding this comment.
Disabling multi-threading causes ASAN to create a deadlock. We're still hunting down why.
There was a problem hiding this comment.
Could you add a TODO here to mention the issue?
|
@antiagainst this commit from ~6 months ago: |
| @@ -304,7 +304,8 @@ def compile(src, target=None, options=None): | |||
| # This is needed to safely finalize threads pool inside context: if current process forks before | |||
| # python GC deletes context object, thread pool in child process will be invalid, which could | |||
| # lead to child crash or hang. | |||
| context.disable_multithreading() | |||
| if not os.environ.get("TRITON_ENABLE_ASAN", "0") == "1": | |||
There was a problem hiding this comment.
Could you add a TODO here to mention the issue?
| memory leak and out of bounds access detection. Currently only supported on the AMD | ||
| backend. This must be run using the ASAN libraries documented [here](https://rocm.docs.amd.com/projects/llvm-project/en/latest/conceptual/using-gpu-sanitizer.html). | ||
|
|
||
| When enabling the address sanitizer it is recommended to disable various memory caching strategies |
There was a problem hiding this comment.
Maybe we can simplify by pointing to the test: "When enabling the address sanitizer it is recommended to disable various memory caching strategies both within the ROCm stack and PyTorch. See this test for it."
This way it's also always up to date.
There was a problem hiding this comment.
We'll have to do this after this is merged since it will immediately create a broken sym link and break the CI.
CRobeck
force-pushed
the
rocm_asan
branch
2 times, most recently
from
7d2ea8c to
b02ebad
Compare
Add address sanitizer pass to LLVM pass pipeline. Known limitations for this PR: - Currently only the AMD backend is supported - Source code line support not implemented here, coming in follow up patch
Add address sanitizer pass to LLVM pass pipeline.
Known limitations for this PR: