Skip to content

[BugFix] Include buffer offsets in TMA descriptor bases - #2833

Open
morluto wants to merge 10 commits into
tile-ai:mainfrom
morluto:audit/tma-offset
Open

[BugFix] Include buffer offsets in TMA descriptor bases#2833
morluto wants to merge 10 commits into
tile-ai:mainfrom
morluto:audit/tma-offset

Conversation

@morluto

@morluto morluto commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Fixes #2832.

Problem

Descriptor-based TMA lowering used Buffer::data as the global base address and dropped Buffer.elem_offset. Coordinates were therefore relative to the backing allocation rather than the buffer view for bulk load/store, gather/scatter, im2col, and TMA atomic paths.

This is orthogonal to #2681: that PR validates the alignment of the Range::min coordinate used by bulk copies, while this change preserves the buffer view displacement before descriptor coordinates are applied.

Before: Buffer.data ----------------------> descriptor base
                    elem_offset (dropped)

After:  Buffer.data -> + elem_offset bytes -> descriptor base
             Range::min remains a descriptor coordinate

Change

Add one CUDA lowering helper that converts elem_offset through the existing TMA payload-byte calculation and emits handle_add_byte_offset when the offset is nonzero. Use that adjusted base for tiled bulk copy, gather/scatter, im2col, and TMA atomic descriptors. Zero-offset buffers retain the existing raw-data expression.

Regression coverage

The new lowering test inspects the stable descriptor-creation IR before host extraction. It covers:

  • SM90 bulk load and store with 32-byte and 64-byte view offsets;
  • SM100a gather and scatter with the same offsets;
  • SM90 im2col with a 32-byte offset;
  • SM90 TMA atomic add with a 32-byte offset.

The bulk fixture uses a non-contiguous 2D view so it cannot take the descriptorless 1D fast path. On the affected source, descriptor argument 2 is the raw buffer data variable; the regression requires the corresponding handle_add_byte_offset expression.

Validation

  • ninja -C build tilelang -j$(nproc) with the pip CUDA 13.0 toolchain — CUDA-enabled libtilelang.so linked successfully.
  • .venv/bin/python -m pytest testing/python/cuda/test_tilelang_tma_descriptor_elem_offset.py -q — 4 passed.
  • bash format.sh --files src/cuda/op/copy.h src/cuda/op/copy.cc src/cuda/op/atomic_add.cc testing/python/cuda/test_tilelang_tma_descriptor_elem_offset.py — all configured checks passed.

Proof gap

The available GPU is SM86, so Hopper/Blackwell runtime execution was not available. The regression validates the emitted descriptor address flow for all affected lowering paths, but it does not execute TMA instructions on SM90/SM100 hardware.

Summary

  • Adjust TMA descriptor base addresses by applying Buffer.elem_offset in payload bytes.
  • Use handle_add_byte_offset for nonzero offsets.
  • Apply adjusted bases to bulk copy, gather/scatter, im2col, and TMA atomic descriptors.
  • Validate required address alignment, including FP4 and sub-byte offset restrictions.
  • Preserve the existing raw-data expression for zero-offset buffers.
  • Add regression coverage for SM90 and SM100a descriptor lowering paths.
  • Preserve byte-offset expressions in NVRTC and CuTe DSL descriptor generation.
  • Resolve descriptor aliases across kernels without losing regular tensor arguments.

Validation

  • CUDA-enabled build passed.
  • Four regression tests passed.
  • Formatting checks passed.
  • Runtime validation on SM90 and SM100 hardware was unavailable.

C++ style / lint notes

  • The PR changes C++ code and adds public CUDA helper declarations.
  • It does not change rules documented in docs/developer_guide/cpp_style.md.
  • The “C++ API Style Audit (warning only)” CI step is relevant to the new public helpers.
  • No correctness or build issues were reported. TLCPP003/TLCPP004 findings are advisory and should not block the PR unless they identify a clear API, FFI, or maintainability risk.

@github-actions

Copy link
Copy Markdown

👋 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

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

TMA descriptor lowering now includes Buffer.elem_offset in global base addresses. Bulk copy, gather/scatter, im2col, and atomic-add paths validate adjusted-address alignment. Host-side descriptor generation preserves byte-offset expressions. CUDA tests cover supported and rejected offset cases.

Changes

TMA descriptor offset handling

Layer / File(s) Summary
Define offset-aware descriptor addressing
src/cuda/op/copy.h, src/cuda/op/copy.cc, src/cuda/op/copy_analysis.cc
Adds GetTMADescriptorBaseAddress, TMARequiredGlobalAddressAlignment, and CanProveTMADescriptorBaseAligned helpers. These compute adjusted descriptor addresses, compute required alignment, and prove alignment is available.
Apply descriptor bases and alignment checks
src/cuda/op/copy.cc, src/cuda/op/copy_analysis.cc, src/cuda/op/atomic_add.cc
Bulk, gather/scatter, im2col, and atomic-add lowering uses adjusted global addresses and validates alignment. Rejects operations when alignment cannot be proven.
Resolve offset-wrapped descriptor variables
src/cuda/transform/lower_hopper_intrin.cc
Unwraps handle_add_byte_offset expressions to extract base variables. Matches descriptor initialization records to allocated buffers using structural identity.
Render host-side descriptor addresses
tilelang/jit/adapter/utils.py, tilelang/jit/adapter/wrapper.py, tilelang/jit/adapter/nvrtc/wrapper.py, tilelang/jit/adapter/cutedsl/wrapper.py
Adds custom global-address renderers that preserve byte-offset expressions. Tiled, im2col, and CuTe DSL descriptor templates receive adjusted addresses. CuTe DSL creates per-kernel descriptor aliases and translates alias names in kernel calls.
Validate offset behavior
testing/python/cuda/test_tilelang_tma_descriptor_elem_offset.py
Tests descriptor offsets across multiple architectures. Validates architecture-specific output, alignment fallback, unproven and symbolic offsets, and FP4 rejection cases.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • tile-ai/tilelang#2681: Adds related TMA alignment validation that this change extends to descriptor base addresses.
  • tile-ai/tilelang#2830: Modifies TMA atomic-add lowering with a separate dtype-validation change.
  • tile-ai/tilelang#2391: Related through shared CUDA TMA copy lowering; addresses different alignment concerns.

Suggested reviewers: leiwang1999

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.30% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: incorporating buffer element offsets into TMA descriptor base addresses, which is the central objective across all modified files.
Linked Issues check ✅ Passed The PR addresses all requirements from issue #2832: buffer elem_offset is incorporated into TMA descriptor bases for bulk copy, gather/scatter, im2col, and atomic add operations, with validation that descriptor coordinates remain relative to the adjusted base.
Out of Scope Changes check ✅ Passed All changes are within scope: they implement the fix for TMA descriptors to include buffer offsets, add necessary helper functions, update validation logic, and include comprehensive regression tests without unrelated modifications.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7a7a4ef054

ℹ️ 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".

Comment thread src/cuda/op/copy.cc
Comment thread src/cuda/op/copy.cc
Comment thread src/cuda/op/copy.cc

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6af6bfa6bb

ℹ️ 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".

Comment thread src/cuda/op/copy.cc

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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/cuda/test_tilelang_tma_descriptor_elem_offset.py`:
- Around line 161-168: Update both pytest.raises match patterns in
test_fp4_unpacked_copy_rejects_16_byte_offset and
test_fp4_unpacked_copy_rejects_subbyte_offset to escape the dot in the literal
T.tma_copy text, using the existing regex-safe approach so near-miss messages
are rejected.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c78e7e22-24ea-41d6-89b0-a95ddf745749

📥 Commits

Reviewing files that changed from the base of the PR and between 7a7a4ef and 6af6bfa.

📒 Files selected for processing (9)
  • src/cuda/op/atomic_add.cc
  • src/cuda/op/copy.cc
  • src/cuda/op/copy.h
  • src/cuda/op/copy_analysis.cc
  • src/cuda/transform/lower_hopper_intrin.cc
  • testing/python/cuda/test_tilelang_tma_descriptor_elem_offset.py
  • tilelang/jit/adapter/nvrtc/wrapper.py
  • tilelang/jit/adapter/utils.py
  • tilelang/jit/adapter/wrapper.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/cuda/op/atomic_add.cc
  • src/cuda/op/copy.h

Comment thread testing/python/cuda/test_tilelang_tma_descriptor_elem_offset.py

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f9ddd04332

ℹ️ 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".

Comment thread src/cuda/transform/lower_hopper_intrin.cc
Comment thread tilelang/jit/adapter/cutedsl/wrapper.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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/cuda/test_tilelang_tma_descriptor_elem_offset.py`:
- Around line 224-227: Update CanProveTMADescriptorBaseAligned to prove
alignment for symbolic elem_offset expressions whose multiplication by the
element size is guaranteed to be a 16-byte multiple, including elem_offset * 8
for the float16 case; then update
test_auto_copy_falls_back_for_symbolically_aligned_device_offset to expect
descriptor lowering rather than fallback.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 92492f59-289d-403e-856f-11bec7669a6e

📥 Commits

Reviewing files that changed from the base of the PR and between a47961a and 629d3da.

📒 Files selected for processing (3)
  • src/cuda/op/copy_analysis.cc
  • testing/python/cuda/test_tilelang_tma_descriptor_elem_offset.py
  • tilelang/jit/adapter/cutedsl/wrapper.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/cuda/op/copy_analysis.cc

Comment thread testing/python/cuda/test_tilelang_tma_descriptor_elem_offset.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9c6fa8917a

ℹ️ 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".

Comment thread tilelang/jit/adapter/wrapper.py
Comment thread src/cuda/op/copy_analysis.cc Outdated
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.

[Bug] TMA descriptors ignore Buffer.elem_offset in global base addresses

1 participant