Skip to content

Conversation

@kurisu6912
Copy link
Collaborator

@kurisu6912 kurisu6912 commented Dec 23, 2025

As title

Summary by CodeRabbit

  • Bug Fixes
    • Improved CUDA compilation to properly respect verbosity settings and warning suppression flags during the build process, ensuring more accurate control over compiler output and diagnostic messages.

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions
Copy link

👋 Hi! Thank you for contributing to the TileLang project.

Please remember to run pre-commit run --all-files in the root directory of the project to ensure your changes are properly linted and formatted. This will help ensure your contribution passes the format check.

We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 23, 2025

Walkthrough

A conditional verbose flag is introduced for CUDA compilation that dynamically sets verbosity based on PTXAS output configuration, replacing a hard-coded false value in the nvcc.compile_cuda invocation.

Changes

Cohort / File(s) Summary
CUDA Compilation Verbosity
tilelang/engine/lower.py
Added local verbose flag that conditionally sets to true when PTXAS verbose output is enabled and appends suppression flag (-w); otherwise defaults to false. Updates nvcc.compile_cuda() call to use this dynamic verbose variable instead of hard-coded false.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: enabling the TL_ENABLE_PTXAS_VERBOSE_OUTPUT flag to work correctly in tvm-ffi. It matches the changeset which modifies the verbose flag handling in the CUDA compilation path.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e79bbcc and fa9b83b.

📒 Files selected for processing (1)
  • tilelang/engine/lower.py
🧰 Additional context used
🧬 Code graph analysis (1)
tilelang/engine/lower.py (1)
tilelang/contrib/nvcc.py (1)
  • compile_cuda (22-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
🔇 Additional comments (1)
tilelang/engine/lower.py (1)

110-110: LGTM! The implementation correctly enables verbose PTXAS output.

The changes properly fix the issue where TL_ENABLE_PTXAS_VERBOSE_OUTPUT had no effect:

  1. A local verbose flag is now dynamically set based on the config
  2. When enabled, the ptxas verbose option is added and the flag is passed to nvcc.compile_cuda
  3. The -w flag suppresses NVCC warnings to improve readability of the ptxas output

The -w flag suppresses all NVCC warnings, which is acceptable in debugging/profiling contexts but could potentially hide important compilation warnings. Consider documenting this behavior or making it configurable if users need to see both verbose output and warnings.

Also applies to: 115-118, 125-125


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@LeiWang1999 LeiWang1999 merged commit 783694f into tile-ai:main Dec 23, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants