fix(TileOps): correct nd2nz/dn2nz src_inner_stride to use src physical stride in bytes.#924
fix(TileOps): correct nd2nz/dn2nz src_inner_stride to use src physical stride in bytes.#924TelGome wants to merge 1 commit into
Conversation
…l stride in bytes
Codex Review该评论由 review 机器人自动更新。
SummaryReview failed at stage Findings未生成结构化 findings,因为 review 过程提前失败。 Log Tail |
There was a problem hiding this comment.
Code Review
This pull request updates lib/TileOps/tload_template.py to use static dst.shape instead of dynamic dst.valid_shape in constraints, and scales the inner stride by the element byte width using the physical strides of the source tensor. The review feedback points out a potential crash where non-5D tensors can bypass the constraints and cause a ValueError during unpacking in the templates, recommending that src.rank == 5 be strictly enforced. Additionally, it suggests simplifying the unpacking of unused shape and stride variables by directly indexing the required strides.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if hasattr(src, 'rank') and src.rank == 5: | ||
| dst_valid_cols = dst.valid_shape[1] if hasattr(dst, 'valid_shape') and dst.valid_shape is not None else None | ||
| if dst_valid_cols is not None and hasattr(src, 'shape') and src.shape is not None: | ||
| dst_cols = dst.shape[1] if hasattr(dst, 'shape') and dst.shape is not None else None |
There was a problem hiding this comment.
If the source tensor does not have rank 5, the outer condition if hasattr(src, 'rank') and src.rank == 5: is skipped, causing the constraint function to return True. This allows non-5D tensors to match this constraint, which will subsequently cause a ValueError during template execution when unpacking src.shape and src.strides into 5 variables.\n\nWe should strictly require src.rank == 5 by returning False if it is not met:\n\npython\nif not hasattr(src, 'rank') or src.rank != 5:\n return False\n
| if hasattr(src, 'rank') and src.rank == 5: | ||
| dst_valid_rows = dst.valid_shape[0] if hasattr(dst, 'valid_shape') and dst.valid_shape is not None else None | ||
| if dst_valid_rows is not None and hasattr(src, 'shape') and src.shape is not None: | ||
| dst_rows = dst.shape[0] if hasattr(dst, 'shape') and dst.shape is not None else None |
There was a problem hiding this comment.
If the source tensor does not have rank 5, the outer condition if hasattr(src, 'rank') and src.rank == 5: is skipped, causing the constraint function to return True. This allows non-5D tensors to match this constraint, which will subsequently cause a ValueError during template execution when unpacking src.shape and src.strides into 5 variables.\n\nWe should strictly require src.rank == 5 by returning False if it is not met:\n\npython\nif not hasattr(src, 'rank') or src.rank != 5:\n return False\n
| # rank-5 partition view metadata (g3 = M rows, g4 = K cols for ND source). | ||
| g0, g1, g2, g3, g4 = src.shape | ||
| s0, s1, s2, s3, s4 = src.strides |
There was a problem hiding this comment.
The variables g0, g1, g2, g3, g4 and s0, s1, s2, s4 are completely unused in this template. Unpacking them is unnecessary and can be simplified by directly accessing src.strides[3].
| # rank-5 partition view metadata (g3 = M rows, g4 = K cols for ND source). | |
| g0, g1, g2, g3, g4 = src.shape | |
| s0, s1, s2, s3, s4 = src.strides | |
| # rank-5 partition view metadata (g3 = M rows, g4 = K cols for ND source).\n s3 = src.strides[3] |
| # rank-5 partition view metadata (g3 = K, g4 = M for DN col-major source). | ||
| g0, g1, g2, g3, g4 = src.shape | ||
| s0, s1, s2, s3, s4 = src.strides |
There was a problem hiding this comment.
The variables g0, g1, g2, g3, g4 and s0, s1, s2, s3 are completely unused in this template. Unpacking them is unnecessary and can be simplified by directly accessing src.strides[4].
| # rank-5 partition view metadata (g3 = K, g4 = M for DN col-major source). | |
| g0, g1, g2, g3, g4 = src.shape | |
| s0, s1, s2, s3, s4 = src.strides | |
| # rank-5 partition view metadata (g3 = K, g4 = M for DN col-major source).\n s4 = src.strides[4] |
Zhendong404
left a comment
There was a problem hiding this comment.
后面需要切到PTODSL,等PR #894 合入再合吧,免得迁移漏掉了
好的 |
Summary
TLOAD.MAT ND2NZ/DN2NZ模板的src_inner_stride改为源 view 物理 stride × 字节宽(s3 * elem_bytes/s4 * elem_bytes),与ND2ND模板写法对齐,匹配硬件loop1_src_stride的字节单位语义s3(对应gStride3)、DN 用s4(对应gStride4),与pto-isaTLoadCubeND2NZ一致dst.shape替代动态dst.valid_shape做模板消歧,避免valid_shape为[null, null]时 ND2NZ/DN2NZ 同时匹配Repro
qwen3_decode_incore_2.pto第一条pto.tload(16×128 bf16,mat)生成的MOV_OUT_TO_L1_MULTI_ND2NZ指令被发射但永不 retire,紧跟 BAR 永久等待,整机死锁;真机报errcode 168 / L0C out of rangesrc_inner_stride = k/m(元素单位 + dst 列行数),硬件按错误步进读 GM(应 16384B/行,实按 128B)Validation
cmake --build build --target check-pto:785 lit 用例全过cube_tile_ops_positive.pto(src 无显式 layout)未回归