-
Notifications
You must be signed in to change notification settings - Fork 450
[Bugfix] Add kernel_global_source property to TVMFFIKernelAdapter #1589
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
Conversation
…eturns `device_kernel_source`
|
👋 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! 🚀 |
📝 WalkthroughWalkthroughThis change fixes a missing attribute error in the TVMFFIKernelAdapter class by adding a single attribute assignment during initialization. The Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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 |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tilelang/jit/adapter/tvm_ffi.py (2)
37-55: Addkernel_global_sourceas a class attribute for consistency.The new attribute
kernel_global_sourceis set in__init__at line 100 but is not declared as a class attribute like its counterpartshost_kernel_sourceanddevice_kernel_source(lines 42–43). This inconsistency can affect type checking and readability.🔎 Proposed fix to add class attribute declaration
host_kernel_source: str | None = None device_kernel_source: str | None = None + kernel_global_source: str | None = None executable: tvm.runtime.Executable | None = None
264-297: Missingkernel_global_sourceassignment infrom_databaseclassmethod affects multiple adapters.The
from_databasealternative constructor inTVMFFIKernelAdapter,CythonKernelAdapter, andNVRTCKernelAdapterdoes not setkernel_global_source, unlike their__init__methods. When loaded from the autotuner cache viafrom_database, accessing thekernel_sourceproperty will raise anAttributeError. This affects the autotuner cache loading path confirmed intilelang/autotuner/param.pyandtilelang/cache/kernel_cache.py.🔎 Proposed fix
Add the assignment in all three adapter classes'
from_databasemethods:adapter.host_kernel_source = host_kernel_source adapter.device_kernel_source = device_kernel_source + adapter.kernel_global_source = device_kernel_source(CuTeDSLKernelAdapter already has this assignment correctly at line 151.)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tilelang/jit/adapter/tvm_ffi.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: lucifer1004
Repo: tile-ai/tilelang PR: 1483
File: tilelang/jit/adapter/cutedsl/adapter.py:93-95
Timestamp: 2025-12-26T06:45:51.789Z
Learning: For the CuTeDSL backend in tilelang/jit/adapter/cutedsl/adapter.py, the host_kernel_source and device_kernel_source have the same value.
📚 Learning: 2025-12-26T06:45:51.789Z
Learnt from: lucifer1004
Repo: tile-ai/tilelang PR: 1483
File: tilelang/jit/adapter/cutedsl/adapter.py:93-95
Timestamp: 2025-12-26T06:45:51.789Z
Learning: For the CuTeDSL backend in tilelang/jit/adapter/cutedsl/adapter.py, the host_kernel_source and device_kernel_source have the same value.
Applied to files:
tilelang/jit/adapter/tvm_ffi.py
|
LGTM, Thanks @haok1402 ! |
fix #1576
JITKernel.kernel_source falls back to self.adapter.kernel_global_source when artifact is None:
tilelang/tilelang/jit/kernel.py
Lines 616 to 618 in dcacc5a
which causes the following error,
TVMFFIKernelAdapter was missing this attribute.
Add kernel_global_source assignment in init, consistent with CuTeDSLKernelAdapter:
tilelang/tilelang/jit/adapter/cutedsl/adapter.py
Line 95 in dcacc5a
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.