[Feat] Add missing support for uint32x2, add unsigned implicit cast in bitwise op, add T.Ref as macro annotation
#1302
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! 🚀 |
WalkthroughUpdated TVM submodule pointer; added 2-element vector integer dtypes; exported a new Ref symbol and typing support; Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Frontend as User/Frontend
participant Builder as v2.builder.macro_arg
participant Proxy as proxy.Ref/Var
participant Validator as LocalVarValidator
Frontend->>Builder: call macro with annotated arg (Ref or Var)
note right of Builder `#f6f8ff`: Ref imported locally to avoid top-level deps
Builder->>Proxy: inspect annotation type
alt annotation is Ref
Builder->>Validator: validate Ref path as local var buffer
Validator-->>Builder: buffer / valid
else annotation is Var
Builder->>Validator: validate Var path (emit deprecation warning)
Validator-->>Builder: buffer / valid
else invalid
Validator-->>Builder: raise ValueError
end
Builder-->>Frontend: return buffer or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)tilelang/language/v2/builder.py (1)
⏰ 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). (3)
🔇 Additional comments (2)
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 |
kurisu6912
changed the title
T.Ref as macro annotation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tilelang/language/__init__.py (1)
25-25: Consider removing the unusednoqadirective.Static analysis indicates the
noqa: F401directive is not needed. IfRefis properly used or exported, the directive can be removed for cleaner code.Apply this diff:
- Ref, # noqa: F401 + Ref,testing/python/language/test_tilelang_intimm.py (1)
23-23: Inconsistent mask wrapping in bitwise operations.Line 23 wraps the mask in
T.uint64(0xffffffffffffffff), but earlier bitwise operations (lines 14, 17, 20) use bare hex literals. Consider making this consistent across all operations.Either use type constructors consistently:
a = T.int32() - a & 0x7fffffff + a & T.int32(0x7fffffff) a = T.uint32() - a & 0xffffffff + a & T.uint32(0xffffffff) a = T.int64() - a & 0x7fffffffffffffff + a & T.int64(0x7fffffffffffffff)Or omit them consistently if implicit casting is the intended behavior being tested.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
testing/python/language/test_tilelang_intimm.py(1 hunks)testing/python/language/test_tilelang_language_frontend_v2.py(1 hunks)tilelang/language/__init__.py(1 hunks)tilelang/language/proxy.py(2 hunks)tilelang/language/v2/builder.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
testing/python/language/test_tilelang_intimm.py (1)
tilelang/language/v2/dtypes.py (4)
int32(156-156)uint32(184-184)int64(157-157)uint64(185-185)
tilelang/language/v2/builder.py (1)
tilelang/language/proxy.py (2)
Ref(269-270)Ref(278-279)
testing/python/language/test_tilelang_language_frontend_v2.py (2)
tilelang/language/v2/builder.py (4)
macro(159-169)macro(553-590)prim_func(152-156)prim_func(659-752)tilelang/language/proxy.py (2)
Ref(269-270)Ref(278-279)
tilelang/language/__init__.py (1)
tilelang/language/proxy.py (2)
Ref(269-270)Ref(278-279)
🪛 Ruff (0.14.5)
testing/python/language/test_tilelang_language_frontend_v2.py
425-425: Avoid specifying long messages outside the exception class
(TRY003)
tilelang/language/__init__.py
25-25: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (3)
tilelang/language/v2/builder.py (1)
477-492: LGTM!The macro argument handling correctly supports both
VarandRefannotations with appropriate deprecation warnings. The local import pattern avoids circular dependencies, and the validation logic properly ensures that onlylocal.varbuffers are accepted.tilelang/language/proxy.py (1)
4-4: LGTM!The
Refclass implementation properly uses the TYPE_CHECKING pattern to provide generic typing support for type checkers while maintaining a lightweight runtime placeholder. This is a clean approach for adding typing-time reference semantics.Also applies to: 267-270, 278-280
testing/python/language/test_tilelang_language_frontend_v2.py (1)
397-427: LGTM!The new
T.Reftests properly validate macro behavior for reference annotations, mirroring the existingT.Vartests. The tests cover both successful usage and error cases appropriately.Note: Static analysis suggests the error message on line 425 could be shorter or moved to the exception class definition, but this is a minor style consideration that matches the existing pattern in this file.
| def test_tilelang_intimm(): | ||
| T.int32(0x7fffffff) | ||
| T.int32(-0x7fffffff-1) | ||
| T.uint32(0xffffffff) | ||
| T.int64(0x7fffffffffffffff) | ||
| T.int64(-0x7fffffffffffffff-1) | ||
| T.uint64(0xffffffffffffffff) | ||
|
|
||
| a = T.int32() | ||
| a & 0x7fffffff | ||
|
|
||
| a = T.uint32() | ||
| a & 0xffffffff | ||
|
|
||
| a = T.int64() | ||
| a & 0x7fffffffffffffff | ||
|
|
||
| a = T.uint64() | ||
| a & T.uint64(0xffffffffffffffff) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add assertions to validate the test behavior.
The test creates integer immediates and bitwise expressions but doesn't validate that they work correctly. Without assertions, this test cannot detect regressions or verify the intended behavior.
Consider adding assertions to verify:
- Integer immediate values are constructed correctly
- Bitwise operations produce expected results
- Type boundaries are respected
For example:
def test_tilelang_intimm():
# Test integer immediate construction
val = T.int32(0x7fffffff)
assert isinstance(val, T.PrimExpr)
# Test bitwise operations
a = T.uint32()
result = a & 0xffffffff
assert isinstance(result, T.PrimExpr)
# Add more specific validation...
2 participants
This pr adds missing support for uint32x2 and some other details:
T.uint32x2(1.0)T.uint64(0xffffffffffffffff)T.Refin macro annotation to pass reference:Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.