fix(stinkytofu): represent exec-masked spans as atomic DAG nodes#9004
Open
KKyang wants to merge 6 commits into
Open
fix(stinkytofu): represent exec-masked spans as atomic DAG nodes#9004KKyang wants to merge 6 commits into
KKyang wants to merge 6 commits into
Conversation
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (76.84%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #9004 +/- ##
========================================
Coverage 65.36% 65.36%
========================================
Files 2685 2685
Lines 422293 422293
Branches 62747 62747
========================================
Hits 276009 276009
Misses 125493 125493
Partials 20791 20791
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
KKyang
force-pushed
the
users/kkyang/stinkytofu-exec-mask-group
branch
from
20bb3a8 to
03a7728
Compare
added 5 commits
July 13, 2026 11:18
Collapse a narrow-write..full-mask-reset exec span into one opaque ExecMaskGroup pseudo-instruction before DAG scheduling and restore it afterward, so the existing RAW/WAW/WAR graph builder orders it correctly without teaching every vector instruction class about exec. Closes #9003
- Forward-declare StinkyInstruction as struct, not class, in StinkyModifiers.hpp -- matches its actual definition and clears -Werror,-Wmismatched-tags under the Microsoft C++ ABI (clang-cl/MSVC). - Add missing ArchHelper.hpp include for getGfxArchID() in StinkyDAGSchedulerPass.cpp.
runPass() always runs expandExecMaskedGroups() on every BB, so a hand-built EXEC_GROUP without a real ExecGroupData modifier hit its assertion and aborted the whole test binary. Give both tests a minimal but real ExecGroupData so unzip has something valid to do, and adjust their expected output accordingly. Verified locally: full build + unit_tests.exe now 804/804 passing.
Replace all "See docs/developer/exec-mask-grouping.md" references with short inline descriptions at each declaration site. Remove the doc file.
KKyang
force-pushed
the
users/kkyang/stinkytofu-exec-mask-group
branch
from
03a7728 to
5ca4af4
Compare
cycheng
approved these changes
Jul 13, 2026
Replace exact register comparison and opcode allowlist with two semantic predicates: isFullMaskReset: any exec write whose sole source is literal -1. Single-source constraint covers all sane reset idioms without needing to evaluate multi-operand expressions. isExecNarrowWrite: any exec write that is not a full-mask reset. Opcode-agnostic — unlike LLVM where codegen is machine-generated, StinkyTofu IR is written by hand, so we match semantic intent (exec != -1) rather than an opcode allowlist. overlapsExec: alias-aware register check so s_mov_b64 exec, -1 is recognized as a reset in wave32 (EXEC aliases EXEC_LO). Tests added: B64ResetClosesWave32Span: s_mov_b64 exec, -1 closes a wave32 span. NonMovExecWriteOpensSpan: s_or_b32 exec_lo opens a span.
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
3 participants
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
Collapses each
exec-narrowing span into a single opaqueExecMaskGrouppseudo-instruction before DAG scheduling, then expands it back afterward. This prevents the scheduler from reordering instructions into or out of the lane-masked region.The pattern matched is the canonical
if() {}shape:-1(opcode-agnostic — StinkyTofu IR is written by hand, not machine-generated, so we match semantic intent rather than an opcode allowlist)-1(single-source constraint covers all sane reset idioms without needing to evaluate multi-operand expressions)Register aliasing is handled explicitly:
EXEC(64-bit) aliases bothEXEC_LOandEXEC_HI, sos_mov_b64 exec, -1is correctly recognized as a reset in wave32.EXEC_LOandEXEC_HIdo not alias each other, so anEXEC_HIwrite inside a wave32 span is a no-op to the depth counter.Unmatched spans (no matching
-1reset before end of basic block) are left ungrouped — silent fallback to pre-fix scheduling behavior rather than a hard error.Changes
ExecMaskGrouping.hpp/.cpp— newcollapseExecMaskedRegions()/expandExecMaskedGroups()passStinkyAsmIR.hpp—createExecMaskGroup(),isExecMaskGroup()StinkyModifiers.hpp—ExecGroupData(non-owning child pointer list)StinkyDAGSchedulerPass.cpp— zip → schedule → unzip call siteGenInstructions.cpp—EXEC_GROUPunified opcodeExecMaskGroupingTest.cpp— unit tests: round-trip, nesting, unmatched narrow, sibling spans, b64 reset in wave32, non-mov openerDAGSchedulerPassTest.cpp— scheduler treatsExecMaskGroupas atomic nodeTest plan
ExecMaskGroupingTest— collapse/expand unit tests passDAGSchedulerPassTest—ExecMaskGroup_*tests pass