Conversation
_tir_packed_uint_to_uint_to_float masked its field and then subtracted 2^(nbit - 1) - 1, the zero-point of an asymmetric signed decode, even though both the storage and the destination are unsigned. Every decoded value came back shifted by that constant: decoding the nibbles 0..7 of one uint32 gave [-7, -6, -5, -4, -3, -2, -1, 0] instead of [0, 1, 2, 3, 4, 5, 6, 7], with no error. The signed sibling _tir_packed_int_to_int_to_float handles its sign by sign-extending rather than biasing, so the two decoders disagreed on the same payload. Mask and shift, with no bias, is the whole unsigned decode.
|
👋 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! 🚀 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: tile-ai/tilelang/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe packed unsigned decoder now returns extracted fields without subtracting an offset. A CUDA-parametrized test checks the decoded values for 2-, 4-, and 8-bit fields. ChangesUnsigned packed decoding
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change removes the unintended unsigned decoding offset and adds regression coverage for three field widths. No merge-blocking issue is identified; normal checks should pass before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Refs #3301 (F163).
Why
_tir_packed_uint_to_uint_to_floatmasks its field and then subtracts2^(nbit - 1) - 1, the zero-point of an asymmetric signed decode, even though both the storage and the destination are unsigned:Every decoded value comes back shifted by that constant. Decoding the nibbles
0..7of oneuint32:The signed sibling
_tir_packed_int_to_int_to_floathandles its sign by sign-extending rather than biasing, so the two decoders disagreed on the same payload — the bias does not belong in an unsigned decode.What changed
Mask and shift, with no bias. A docstring states the contract, since the name and the behaviour are what disagreed.
The helper is not referenced anywhere in the tree, so nothing that currently runs changes behaviour. It lives in a module that four test files import from, and the same function ships under
tilelang/quantize/quantization.pyin released wheels, so it is worth leaving correct.Verification
testing/python/issue/test_tilelang_u32_signed_decode.py, which already tests a sibling decoder from the same module and whose packing helper and kernel shape are reused here. All three fail on an unmodified tree withMismatched elements: 4 / 4 (100.0%)and similar; the three existing signed cases pass in both states.test_tilelang_u32_signed_decode.py,test_tilelang_issue_2947.py,test_tilelang_bf16x2_pack_special_values.pyandtesting/python/quantize/: 39 passed, 1 skipped.requires_cudaand skip cleanly without a device.pre-commit run --files <changed files>: all 12 applicable hooks pass.Summary
_tir_packed_uint_to_uint_to_floatto decode packed fields as unsigned values without subtracting a zero-point bias.uint32. The tests check that decodedfloat32values match the inputs.Testing
The author reports 39 passed and 1 skipped across the listed test files and quantization tests. The author also reports that all 12 applicable pre-commit hooks passed. These results were not independently verified.
Verified end to end, together with the other contributions from this batch: VERIFICATION.md