TileOP: fix RedRows per-PE semantics for Local-A/Shared-B groupM - #86
Merged
Conversation
The explicit-groupM TMATMUL overloads computed the reduction/output row count as cooperative_group_m_rows_per_pe(EffectiveM) for every cooperative form. For Local-A/Shared-B, EffectiveM is A::ValidRow which already IS the per-PE shard size (M_per_PE, per ADR-0100) — feeding it back through rows_per_pe() maps M_per_PE=32 (CubeM32, group_M>64) to 16 and fails the D::ValidRow == RedRows assertion, blocking the CubeM32 + group_M=128 configuration entirely. CubeM16 passed only by coincidence (rows_per_pe(16)==16). Now only Shared-A (where EffectiveM is the core-total group_M) derives its per-PE block through rows_per_pe(); Local-A uses the shard size directly. Verified: - CubeM32 Local-A + Shared-B + group_M=128: compiles, LB0 carries the runtime group_M, N/K immediate-form - CubeM16 + group_M=64: unchanged - GroupMatmul/SharedMatmul/TMatmulAcc*/PostProcessCombos/TGEMV* fixtures and unittest 40/40 all clean
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.
Fixes the group_M assertion failure reported for
TMATMUL(d, a, b, options, groupM)with CubeM32 tiles and group_M=128.Root cause
The explicit-groupM overloads computed the reduction/output row count as:
For Local-A/Shared-B,
EffectiveM = A::ValidRowwhich already is the per-PE shard size (M_per_PE per ADR-0100) — each PE holds a full [M_per_PE, K] block. Feeding it back throughrows_per_pe():32 != 16→ the reported compile failureFix
Only Shared-A (where
EffectiveMis the core-total group_M) derives its per-PE block viarows_per_pe(); Local-A uses the shard size directly:Verification
Fixes the assertion analyzed in the linked report (Local-A/Shared-B explicit-groupM path).