[CUDA] Resolve symlinked nvcc before deriving CUDA_HOME - #2839
Conversation
|
👋 Hi! Thank you for contributing to the TileLang project. Please remember to run We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesCUDA environment detection
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
testing/python/components/test_tilelang_env.py (1)
87-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
CUDA_PATH.
_find_cuda_home()also readsCUDA_PATHwhenCUDA_HOMEis absent, but this test only exercisesCUDA_HOMEset withCUDA_PATHunset. Add the missingCUDA_PATHcase, preferably parameterized.Proposed test adjustment
+@pytest.mark.parametrize("env_var", ["CUDA_HOME", "CUDA_PATH"]) -def test_find_cuda_home_prefers_explicit_environment(monkeypatch, tmp_path): +def test_find_cuda_home_prefers_explicit_environment( + monkeypatch, tmp_path, env_var +): cuda_home = tmp_path / "explicit-cuda" cuda_home.mkdir() + monkeypatch.delenv("CUDA_HOME", raising=False) + monkeypatch.delenv("CUDA_PATH", raising=False) - monkeypatch.setenv("CUDA_HOME", str(cuda_home)) + monkeypatch.setenv(env_var, str(cuda_home))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@testing/python/components/test_tilelang_env.py` around lines 87 - 97, Extend test_find_cuda_home_prefers_explicit_environment to cover both CUDA_HOME and CUDA_PATH, preferably by parameterizing the environment-variable name while keeping the unexpected nvcc lookup assertion. Ensure the test clears the other variable so each case verifies _find_cuda_home() selects the configured path when CUDA_HOME is absent for the CUDA_PATH case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@testing/python/components/test_tilelang_env.py`:
- Around line 87-97: Extend test_find_cuda_home_prefers_explicit_environment to
cover both CUDA_HOME and CUDA_PATH, preferably by parameterizing the
environment-variable name while keeping the unexpected nvcc lookup assertion.
Ensure the test clears the other variable so each case verifies
_find_cuda_home() selects the configured path when CUDA_HOME is absent for the
CUDA_PATH case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a03cd66f-f273-4e68-b26a-da2581572f0a
📒 Files selected for processing (2)
testing/python/components/test_tilelang_env.pytilelang/env.py
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dc42e7ab8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 209c57c186
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fd3427f89
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if "hpc_sdk" in path.lower(): | ||
| return os.path.dirname(os.path.dirname(os.path.dirname(path))) |
There was a problem hiding this comment.
When nvcc is located under an HPC SDK CUDA subtree such as /opt/nvidia/hpc_sdk/Linux_x86_64/25.7/cuda/12.9/bin/nvcc, this condition now takes precedence over recognizing the CUDA layout and strips three components, returning .../cuda instead of .../cuda/12.9. Because the broader directory exists, validation accepts it, but tilelang/contrib/nvcc.py subsequently derives nonexistent include and bin paths from it. Check the CUDA layout before applying the generic HPC SDK parent rule, as the previous ordering did.
Useful? React with 👍 / 👎.
1 participant
Fixes #2837.
Problem
CUDA discovery derived the installation root from a symlink on PATH rather than the resolved nvcc binary. Pip-installed NVIDIA CUDA toolchains commonly expose nvcc that way, which selected /home/morluto/.local instead of the package root containing cuda_runtime.h.
Change
Resolve the discovered nvcc path before applying the existing parent-directory discovery logic. Explicit CUDA_HOME and CUDA_PATH retain their existing precedence.
Regression coverage
A deterministic symlink fixture verifies both resolved discovery and explicit-environment precedence.
Validation
Summary
nvccpaths before derivingCUDA_HOME.CUDA_HOMEandCUDA_PATHprecedence.Validation
compileallpassed.git diff --checkpassed.