Add TL_DISABLE_SHARED_MEMORY_REUSE pass config#2228
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! 🚀 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a pass config key and wiring to disable shared-memory lifetime-based reuse; implements a sequential-layout fallback in the merge transform, updates the C++ and Python pass APIs, integrates the option into the engine pipeline, and adds CUDA tests validating correctness. ChangesDisable shared-memory reuse feature
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
surprised the test can pass, because dynamic shared memory can only be allocated once. That means allocations for shared.dyn buffers must be merged. Would you mind double-checking the allocations? |
…in C++ Instead of skipping MergeSharedMemoryAllocations entirely, add a disable_reuse flag that still merges allocations into one but lays out each buffer sequentially without lifetime-based sharing.
56424ec to
f8c4721
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@testing/python/transform/test_tilelang_transform_disable_memory_reuse.py`:
- Around line 105-118: The test only checks numeric equality but must assert a
structural difference in the lowered/kernel source between disable_reuse=True
and False; call make_kernel(disable_reuse=True) and
make_kernel(disable_reuse=False) (already present as kernel_no_reuse and
kernel_with_reuse), extract their lowered kernel source or allocation metadata
(e.g., the kernel source string or allocation list exposed by your lowering
API), and assert a concrete structural change such as different shared-memory
allocation layout/offsets or total shared-memory bytes (for example, no
overlapping offsets or larger total shared-memory size when reuse is disabled).
Implement an assertion comparing the allocation entries (names/offsets/size) or
a substring in the lowered source that indicates reuse was disabled versus
enabled to ensure the pass changed the allocation strategy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed1d1772-b0b9-4644-be7b-cf21dba76269
📒 Files selected for processing (1)
testing/python/transform/test_tilelang_transform_disable_memory_reuse.py
0ee4417 to
36e660a
Compare
36e660a to
93861c5
Compare
Summary
TL_DISABLE_SHARED_MEMORY_REUSEpass config to disable lifetime-based shared memory reuse inMergeSharedMemoryAllocations. Allocations are still merged into a single buffer but each gets its own dedicated region without sharing.Summary by CodeRabbit