Skip to content

[CI/Build][ROCm] Run the TileLang HIP symbol checks in their own interpreter - #53117

Merged
AndreasKaratzas merged 2 commits into
vllm-project:mainfrom
stefankoncarevic:ci-tilelang-hip-symbol-checks
Aug 21, 2026
Merged

[CI/Build][ROCm] Run the TileLang HIP symbol checks in their own interpreter#53117
AndreasKaratzas merged 2 commits into
vllm-project:mainfrom
stefankoncarevic:ci-tilelang-hip-symbol-checks

Conversation

@stefankoncarevic

@stefankoncarevic stefankoncarevic commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

The test belongs to the Kernels Root Misc Test group, which AMD CI does not run yet.
This fix should land before the PR that enables that group (#50519).

Purpose

test_deepseek_v4_import_and_jit_monitor_do_not_hijack_hip_symbols asserts two
things about process-global state: that importing DeepSeek V4 and activating the
JIT monitor leaves TileLang out of sys.modules, and that hipFree in the
global symbol table still resolves to libamdhip64.so rather than the
libhip_stub.so TileLang ships.

On its own the test holds. In a whole-directory run it does not, because the
sibling test_mhc_kernels.py imports TileLang on purpose to exercise those
kernels. By the time this test runs, TileLang is already in sys.modules and
its stub is already loaded, so the test fails on suite-wide state rather than on
anything about the code it is guarding.

Move the two checks into tests/kernels/scripts/check_no_tilelang_hijack.py and
have the test run it with sys.executable. In a fresh interpreter the
assertions mean what they say no matter what the rest of the suite imports, and
the test reports the script's output when it fails.

Test Plan

Run the two files together, in the order a whole-directory collection gives
them:

pytest tests/kernels/test_mhc_kernels.py tests/kernels/test_mhc_tilelang_jit.py

Test Result

On MI355 (gfx950, ROCm 7.2.3), before the change, the sibling import is enough
to take the test down:

FAILED kernels/test_mhc_tilelang_jit.py::test_deepseek_v4_import_and_jit_monitor_do_not_hijack_hip_symbols
E       assert not True
1 failed, 54 passed, 39 warnings in 21.65s

After:

55 passed, 39 warnings in 77.35s

The check still bites when the symbols really are taken over. Running the script
in an interpreter that has TileLang loaded fails it, as it should:

AssertionError: TileLang was imported: ['tilelang.env', 'tilelang.libinfo', ...]

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

…rpreter

test_mhc_tilelang_jit.py asserts that TileLang stays out of sys.modules and
that hipFree still resolves to libamdhip64, so that the libhip_stub.so TileLang
ships never takes the symbol over. Both claims are about process-global state,
and the neighbouring test_mhc_kernels.py legitimately imports TileLang to
exercise those kernels, so in a whole-directory run there is nothing left to
assert and the test fails.

Move the checks into a script that the test runs in a fresh interpreter, which
keeps them meaningful regardless of what the rest of the suite imports.

Signed-off-by: Stefan Koncarevic <Stefan.Koncarevic@amd.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added the rocm Related to AMD ROCm label Aug 20, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Aug 20, 2026
@AndreasKaratzas

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #84851 for commit b47766614393.

@@ -0,0 +1,60 @@
#!/usr/bin/env python3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need a separate file for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The subprocess is the point of the change, and I put the script in a file to
follow what is already in the tree: tests/cuda/test_platform_no_cuda_init.py
keeps its checks in tests/cuda/scripts/check_*.py and runs them the same way,
with subprocess.run([sys.executable, str(script_path)]) and a pytest.fail on
a non-zero exit.

The alternative is to inline the source as a string and pass it with python -c,
as tests/config/test_config_utils.py does, but this check needs a ctypes
structure and the dlsym/dladdr calls, and inside a string none of that is
formatted or linted. I can switch if you prefer no new file in the tree.

@AndreasKaratzas

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #85096 for commit f9861a0943e0.

@AndreasKaratzas
AndreasKaratzas merged commit f37d586 into vllm-project:main Aug 21, 2026
18 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in AMD Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rocm Related to AMD ROCm

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants