fix(ir): Normalize tile/tensor scalar operand dtype - #2132
Merged
lyfne123 merged 3 commits intoJul 27, 2026
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughScalar operand normalization is centralized for tensor and tile operators. Constants adopt compatible element dtypes, non-constant ChangesScalar dtype normalization
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/en/dev/ir/05-operators.md`:
- Line 270: Clarify the scalar normalization contract in the operator-table
notes: document bare integer literals, float literals used with integer
operands, explicitly typed constants, and non-constant expressions, including
the required rejection or cast behavior. Update the English note at
docs/en/dev/ir/05-operators.md:270-270 and mirror the same clarification in
Chinese at docs/zh-cn/dev/ir/05-operators.md:264-264.
In `@python/pypto/ir/op/tile_ops.py`:
- Line 989: Keep shift-count operands as INT32 in both shls/shrs paths by
replacing lhs-based normalization with _normalize_const_to_dtype(rhs,
DataType.INT32, actual_span) at python/pypto/ir/op/tile_ops.py lines 989 and
1028. Add INT8 and INT16 shift-literal test cases in
tests/ut/ir/operators/test_tile_ops.py lines 3294-3306 asserting the normalized
operand uses INT32.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3de78e27-656c-4fbd-9496-0ce1cc79e462
📒 Files selected for processing (14)
docs/en/dev/ir/05-operators.mddocs/zh-cn/dev/ir/05-operators.mdpython/pypto/ir/op/tensor_ops.pypython/pypto/ir/op/tile_ops.pypython/pypto/ir/utils.pytests/ut/codegen/test_pto_codegen.pytests/ut/ir/operators/test_tensor_ops.pytests/ut/ir/operators/test_tile_ops.pytests/ut/ir/transforms/test_convert_tensor_to_tile_ops.pytests/ut/ir/transforms/test_convert_to_ssa_pass.pytests/ut/ir/transforms/test_unroll_loops_pass.pytests/ut/language/parser/test_closure_var_resolution.pytests/ut/language/parser/test_constant_folding.pytests/ut/language/parser/test_control_flow.py
Hzfengsy
added a commit
to Hzfengsy/pypto
that referenced
this pull request
Jul 24, 2026
- Document the scalar normalization exceptions in the operator tables (EN + zh-CN): a float literal on an integer operand keeps FP32, and an explicit pl.const(v, dtype) is left as-is, alongside the existing index-rejection note. - Correct the stale shls/shrs docstrings: the shift count is re-stamped to the lhs element dtype, not fixed at INT32. The IR permits any integer width for the shift operand (DeduceTileOpIntScalarBinaryType) and codegen casts it to i32. - Pin that behaviour with INT8/INT16 shift-literal coverage.
A bare int literal in the DSL is parsed to ConstInt(v, INDEX), but `index` is not a legal operand type for any pto.t*s instruction, so every tile/tensor x integer-literal op failed ptoas parse/verify. On the tensor path it was worse: the index scalar propagated through PromoteDataTypes and silently retyped the *result* tensor to index. Route all tile/tensor scalar wrappers through a shared normalizer: - A constant scalar operand adopts the operand's element dtype. The node kind follows the target dtype (ConstInt vs ConstFloat), since codegen dispatches on it -- an int literal on a float tile must be a ConstFloat or MLIR receives `arith.constant 5 : f32`. - A float literal on an integer operand keeps FP32, preserving existing promotion semantics (int32_tensor * 2.5 -> fp32). - An explicit pl.const(v, dtype) is a deliberate annotation and is left untouched, as are typed pl.Scalar params and any non-constant expr. - A non-constant `index` value (loop var, pl.dim, block idx) is rejected with an actionable pl.cast hint rather than silently miscompiling. Behaviour changes worth noting: tensor.adds(x_i32, 5) now yields an int32 result instead of fp32, and an int literal on a float tile becomes a matching-dtype ConstFloat. Covers 18 tile wrappers and 14 tensor wrappers. Tests that fed a raw index value to a scalar operand were producing uncompilable IR and now cast it explicitly.
- Document the scalar normalization exceptions in the operator tables (EN + zh-CN): a float literal on an integer operand keeps FP32, and an explicit pl.const(v, dtype) is left as-is, alongside the existing index-rejection note. - Correct the stale shls/shrs docstrings: the shift count is re-stamped to the lhs element dtype, not fixed at INT32. The IR permits any integer width for the shift operand (DeduceTileOpIntScalarBinaryType) and codegen casts it to i32. - Pin that behaviour with INT8/INT16 shift-literal coverage.
A non-constant index scalar is rejected with a pl.cast hint. An integer scalar operand is accepted against a float tile (codegen narrows it), so the hint can uniformly suggest pl.cast(<value>, pl.INT32) rather than the two-step index->int->float form for float operands.
Hzfengsy
force-pushed
the
fix/scalar-operand-index-dtype
branch
from
July 25, 2026 05:01
8769cef to
d726627
Compare
lyfne123
approved these changes
Jul 27, 2026
This was referenced Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A bare int literal in the DSL is parsed to
ConstInt(v, INDEX), butindexis not a legal operand type for anypto.t*sinstruction — so every tile/tensor × integer-literal op failed ptoas parse/verify:The tile wrappers already tried to fix this via
_normalize_expr(..., int_dtype=INT32), but that call was a no-op:_normalize_exprreturns early when the value is already anExpr, which it always is coming from the parser.On the tensor path it was worse — the
indexscalar propagated throughPromoteDataTypesand silently retyped the result tensor toindex, sopl.add(x_i32, 5)produced anindextensor that then failed to rebind to itsOutparam.Float literals were unaffected (
5.0→ConstFloat(FP32)), which is why this stayed hidden.Approach
All tile/tensor scalar wrappers now route through one shared normalizer in
python/pypto/ir/utils.py(18 tile wrappers, 14 tensor wrappers):ConstIntvsConstFloat) because codegen dispatches on it — an int literal on a float tile must become aConstFloat, or MLIR receivesarith.constant 5 : f32.int32_tensor * 2.5 -> fp32).pl.const(v, dtype)is left untouched — it is a deliberate user annotation, not a placeholder. Same for typedpl.Scalarparams and any non-constant expression.indexvalue (loop var,pl.dim, block idx) is rejected with an actionablepl.casthint instead of silently miscompiling.pl.add(tile_i32, 5)index→ ptoas rejecti32✅pl.tensor.adds(x_i32, 5)indexint32✅pl.add(tile_i32, 2.5)f32f32(unchanged)pl.const(42, pl.INT32)on i16 tilei32i32(unchanged)pl.add(acc, i)(i= loop var)ValueError+pl.casthintBehaviour changes worth noting
tensor.adds(x_i32, 5)now yields an int32 result instead offp32.ConstFloat(e.g.fp16) instead offp32.indexvalue to a scalar operand now raises. Tests that did this were producing uncompilable IR and now cast explicitly.Testing
TestTileScalarOperandDtype(15) andTestTensorScalarOperandDtype(11) unit tests.test_pto_codegen.pyasserting the emittedpto.taddsoperand isi32, neverindex— every IR-construction test stops before codegen, which is exactly why this class of bug shipped.indexvalue to a scalar operand (unroll / SSA / tensor→tile lowering / parser control-flow, folding, closure) to cast it explicitly.ruff,ruff format,pyright, and all pre-commit hooks clean.docs/enanddocs/zh-cn.