[comgr][hotswap] Preserve VGPR-MSB mode when splitting gfx1250 WMMAs#3428
[comgr][hotswap] Preserve VGPR-MSB mode when splitting gfx1250 WMMAs#3428YashDeshpande25 wants to merge 1 commit into
Conversation
9b631a4 to
fb68ee8
Compare
YashDeshpande25
changed the title
github-actions
Bot
added
comgr
| if ((!LS.MIA->isBranch(DI.Inst) && !LS.MIA->isCall(DI.Inst)) || | ||
| LS.MIA->isIndirectBranch(DI.Inst) || LS.MIA->isReturn(DI.Inst)) | ||
| continue; | ||
| std::optional<uint64_t> Target = evaluateDirectControlFlowTarget(DI, LS); |
There was a problem hiding this comment.
I do not think this uses the same control-flow surface as the rewrite. This only asks evaluateDirectControlFlowTarget(), so it misses the absolute and PC-materialized s_swap_pc_i64 targets that collectDirectBranchTargets() already treats as known text entries. A caller can enter another function at an interior WMMA after skipping the normal s_set_vgpr_msb prefix; then this analysis seeds from the symbol entry and can emit brackets for the wrong incoming mode instead of failing closed.
Can we share the known-target logic from collectDirectBranchTargets() here, or otherwise make absolute/materialized/unresolved call targets force the affected function to stay unanalyzed? Please add a lit case for an interior cross-function s_swap_pc_i64 target, ideally both materialized-PC and absolute-immediate forms.
| In[I] = mergeVgprMsbState(In[I], vgprMsbStateFromMode(0)); | ||
| Worklist.push_back(I); | ||
| }; | ||
| SeedAbiEntry(0); |
There was a problem hiding this comment.
I think the VGPR-MSB analysis also needs to seed declared text entries, not just the function symbol start and pc-relative callable entries. collectDeclaredTextEntries() already treats kernel descriptor entries as real entry points, but computeVgprMsbModes() only seeds SeedAbiEntry(0) plus CallableEntries found via evaluateDirectControlFlowTarget().
That can mark an interior KD entry block as unreachable and then findActiveVgprMsbMode() returns mode 0 for a WMMA there, even if the real entry path executes a local s_set_vgpr_msb first. Can we pass the declared entries into this analysis and seed any entry inside the current range? A lit test with a KD entry offset to an interior block would cover this.
|
Non-blocking AGENT_CONVENTIONS cleanup before merge: please spell out the new |
973e8f3 to
6068a5d
Compare
|
needs to be rebased on latest amd-staging to unbreak spirv |
The WMMA split pass rewrites gfx1250 B0-only WMMAs into pairs of narrower WMMAs that also run on A0. On gfx1250, VGPR operands past v255 are reached through a persistent VGPR-MSB bank mode, so a split whose upper half crosses v255 (or a K-split whose second half reuses dst as src2) must execute that half under a different bank. Make the pass VGPR-MSB aware: - Recover the live VGPR-MSB mode at each WMMA via a whole-function CFG fixed point, fail-closed: an ambiguous or unprovable mode declines the split. - Bracket the upper half with s_set_vgpr_msb (switch before, restore after), and emit nothing when no bank change is needed. - Fail closed on every matched-but-unsplittable path (RequiredPatchFailed) instead of silently leaving an A0-illegal opcode in .text. Use the same control-flow surface as the rewrite so the mode proof cannot be seeded from a wrong incoming mode: - Decline the analysis when collectDirectBranchTargets() reports an unresolved call target, since such a call may enter any function at an interior offset. - Resolve absolute-immediate and PC-materialized s_swap_pc_i64 / s_set_pc_i64 targets in the interior-entry pre-pass so a cross-function interior entry declines the split rather than seeding the symbol-start mode. - Seed declared kernel-descriptor entries so an interior KD-entry block is analyzed from the real entry mode rather than misread as unreachable. Add lit coverage for the bank-crossing, fail-closed, and loop-carried mode cases, plus interior cross-function s_swap_pc_i64 (materialized and absolute), an interior s_set_pc_i64 jump, and an interior kernel-descriptor entry.
6068a5d to
d79c0d1
Compare
3 participants
Summary
The WMMA split pass rewrites gfx1250 B0-only WMMAs into pairs of narrower WMMAs that also run on A0. On gfx1250, VGPR operands past v255 are reached through a persistent VGPR-MSB bank mode, so a split whose upper half crosses
v255 (or a K-split whose second half reuses
dstassrc2) must execute that half under a different bank.This makes the pass VGPR-MSB aware:
s_set_vgpr_msb(switch before, restore after), and emits nothing when no bank change is needed.RequiredPatchFailed) instead of silently leaving an A0-illegal opcode in.text.s_swap_pc_i64calls ands_set_pc_i64jumps so the CFG can follow materialized-PC jumps.