Fix: add tile.neg to kRowMajorOps for ACC-to-VEC layout conversion#1308
Fix: add tile.neg to kRowMajorOps for ACC-to-VEC layout conversion#1308bumble0918 wants to merge 1 commit into
Conversation
When infer_tile_memory_space_pass inserts tile.move for a matmul accumulator (col_major, ACC space) feeding into tile.neg, the move previously inherited col_major blayout because tile.neg was not in kRowMajorOps. tile.neg propagates layout via InheritTileViewLayout, so col_major flowed through to downstream ops like tile.exp which require row_major in VEC space, causing PTO verifier failure.
📝 WalkthroughWalkthroughThe PR adds ChangesOperation Registration Configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Code Review
This pull request adds tile.neg to the list of operations requiring a row-major layout in src/backend/common/pto_ops_common.cpp. The reviewer noted that while this change is correct, several other element-wise and tile-scalar operations (such as tile.prelu, tile.subs, and tile.rems) are also missing from the RequiresRowMajorLayout list and should be included to prevent similar layout propagation issues and verifier failures.
| "tile.sqrt", | ||
| "tile.rsqrt", | ||
| "tile.recip", | ||
| "tile.neg", |
There was a problem hiding this comment.
While adding tile.neg correctly addresses the reported issue, several other element-wise operations that likely require row-major layout in the Vector core are still missing from the kRowMajorOps list.
Specifically, the following operations defined in kSimpleOps appear to be missing from kRowMajorOps:
- Binary:
tile.prelu(line 1989) - Tile x Scalar:
tile.subs(line 2005),tile.rems(line 2008),tile.ands(line 2009),tile.ors(line 2010),tile.xors(line 2011),tile.shls(line 2012),tile.shrs(line 2013), andtile.mins(line 2015).
Failure to include these will likely result in similar layout propagation issues and verifier failures when these operations are used in VEC memory space after a matmul, as they also propagate layout via InheritTileViewLayout.
#1309 When infer_tile_memory_space_pass inserts tile.move for a matmul accumulator (col_major, ACC space) feeding into tile.neg, the move previously inherited col_major blayout because tile.neg was not in kRowMajorOps. tile.neg propagates layout via InheritTileViewLayout, so col_major flowed through to downstream ops like tile.exp which require row_major in VEC space, causing PTO verifier failure.