From 9cd2a0d9b3bc5f5a58b52da8988a8146c754504b Mon Sep 17 00:00:00 2001 From: hcman2 Date: Fri, 30 Oct 2026 00:59:31 +0000 Subject: [PATCH 1/6] Preserve barrier threshold with dependency --- .../src/transforms/asm/dag/CDNA5.hpp | 47 ++++++--- .../tests/unit/asm/DAGSchedulerPassTest.cpp | 98 +++++++++++++++++++ 2 files changed, 133 insertions(+), 12 deletions(-) diff --git a/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp b/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp index 9264c9854a0c..c0ba99a8c640 100644 --- a/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp +++ b/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp @@ -702,6 +702,11 @@ class CDNA5ReadyQueue : public ReadyQueue { int latencyWmmaBudget = 0; int wmmaWindowsNeeded = 0; int dsLoadCount = 0; + // Step 4b parent-WMMA drain (maxParentWmmaIdx + budget). The overlap + // afterThreshold can sit later than this. Averaging with a before + // threshold may move earlier than that after value, but not earlier + // than this dependency. + int depThreshold = 0; }; int wmmaIssuedCountThisRegion_ = 0; @@ -2005,6 +2010,7 @@ CDNA5ReadyQueue::computeBarrierAfterThresholds(IRList::iterator regionStart, int wmmaWindowsNeeded; int latencyWmmaBudget; int dsLoadCount; + int depThreshold; }; // Step 1a: collect all movable barriers with their PSEUDO src token sets, @@ -2151,11 +2157,12 @@ CDNA5ReadyQueue::computeBarrierAfterThresholds(IRList::iterator regionStart, } for (StinkyInstruction* barrier : group.barriers) { barrierWmmaThresholds_[barrier] = afterThreshold; - result[barrier] = {afterThreshold, afterThreshold, latencyWmmaBudget, wmmaWindowsNeeded, - matchingDsLoadCount}; + result[barrier] = {afterThreshold, afterThreshold, latencyWmmaBudget, + wmmaWindowsNeeded, matchingDsLoadCount, depThreshold}; } overlapChecks.push_back({groupBarrier, group.barriers, afterThreshold, lastOverlap, - wmmaWindowsNeeded, latencyWmmaBudget, matchingDsLoadCount}); + wmmaWindowsNeeded, latencyWmmaBudget, matchingDsLoadCount, + depThreshold}); PASS_DEBUG(std::cerr << "[CDNA5 computeBarrierAfterThresholds] barrier=" << groupBarrier << " barrierGroupSize=" << group.barriers.size() << " afterThreshold=" << afterThreshold << " matchingDsLoadCount=" << matchingDsLoadCount @@ -2220,9 +2227,9 @@ CDNA5ReadyQueue::computeBarrierAfterThresholds(IRList::iterator regionStart, for (StinkyInstruction* barrier : summary.barriers) { barrierWmmaThresholds_[barrier] = adjustedAfterThreshold; - result[barrier] = {adjustedAfterThreshold, summary.afterThreshold, + result[barrier] = {adjustedAfterThreshold, summary.afterThreshold, summary.latencyWmmaBudget, summary.wmmaWindowsNeeded, - summary.dsLoadCount}; + summary.dsLoadCount, summary.depThreshold}; } PASS_DEBUG( std::cerr << "[CDNA5 computeBarrierAfterThresholds overlap] barrier=" @@ -2993,10 +3000,16 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato if (hasWMMAInRegion_) { // Layer 1/3 (base merge): // Build one threshold map from two independent estimators: - // - afterThresholds : "barrier should be after at least N WMMA" - // - beforeThresholds: "barrier should be before/around WMMA N" - // If a barrier appears in both maps, average them to get a single - // neutral placement point in barrierWmmaThresholds_. + // - afterThresholds : step-4 overlap/window placement. Step 4b may + // replace it when the parent-WMMA drain is later. + // - beforeThresholds: where the barrier should sit so the + // post-barrier ds_loads can issue and be consumed. + // A barrier in both maps averages the two. The average may pull the + // step-4 after earlier, but not earlier than depThreshold (the + // parent-WMMA index plus the same drain budget). That dependency is + // the floor even when it did not replace the overlap after, because + // overlap can sit later than the parent drain. Example: overlap after + // 50, dep 45, before 30 averages to 40, and the barrier keeps 45. auto afterThresholds = computeBarrierAfterThresholds(regionStart, regionEnd); auto beforeThresholds = computeBarrierBeforeThresholds(regionStart, regionEnd); for (auto& [barrier, afterOutput] : afterThresholds) { @@ -3004,10 +3017,20 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato } for (auto& [barrier, beforeOutput] : beforeThresholds) { auto it = barrierWmmaThresholds_.find(barrier); - if (it != barrierWmmaThresholds_.end()) - it->second = (it->second + beforeOutput.beforeThreshold) / 2; - else + if (it != barrierWmmaThresholds_.end()) { + const int afterThreshold = it->second; + const int beforeThreshold = beforeOutput.beforeThreshold; + const int averaged = (afterThreshold + beforeThreshold) / 2; + const int depThreshold = afterThresholds.find(barrier)->second.depThreshold; + it->second = std::max(depThreshold, averaged); + PASS_DEBUG(std::cerr << "[CDNA5 onInitRegion after-before floor] barrier=" + << barrier << " afterThreshold=" << afterThreshold + << " beforeThreshold=" << beforeThreshold + << " depThreshold=" << depThreshold << " averaged=" << averaged + << " merged=" << it->second << "\n"); + } else { barrierWmmaThresholds_[barrier] = beforeOutput.beforeThreshold; + } } // Layer 2/3 (exclusive overlap reconcile): // Some signal/wait-like pairs are split: one member only lands in the diff --git a/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp b/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp index dfbe229364be..d9cb7b1b9e5d 100644 --- a/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp +++ b/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp @@ -851,6 +851,104 @@ TEST_F(DAGSchedulerPassTest, BarrierAfterUsesParentWmmaWhenItExceedsDrainThresho EXPECT_LT(positionOf(*bb, afterParentWmma), positionOf(*bb, lateSignal)) << scheduleOrder(*bb); } +// One stage barrier with ds_loads on both sides is in both threshold maps. +// The pre-barrier ds_load overwrites a VGPR read by WMMA 16, so the after +// threshold is that parent plus the single-load drain budget +// (16 + 56/8 + 1 = 24). The post-barrier ds_load pulls the before threshold +// below that (the issue window is subtracted from the region), and averaging +// the two promotes the barrier at 19. One of the twelve competing WMMAs +// issues before the ds_load, so a threshold of 24 leaves seven WMMAs between +// the load and the barrier (7 * 8 cycles covers the 56-cycle return). The +// average leaves two. DsSpread is off so the drain is the instruction +// latency. The trailing compare keeps the loop tail from being a WMMA, which +// would otherwise defer the first WMMA and hide the gap. +TEST_F(DAGSchedulerPassTest, BothSidedBarrierKeepsTheAfterThresholdAsAFloor) { + bb->addSuccessor(bb); + + StinkyInstruction* parentWmma = nullptr; + for (int i = 0; i < 16; ++i) { + const int src = (i == 15) ? 0 : 3000 + i * 16; + StinkyInstruction* wmma = + createWmmaF32_16x16x16_bf16(/*destStart=*/1100 + i * 16, /*src0Start=*/src); + if (i == 15) parentWmma = wmma; + } + StinkyInstruction* preBarrierDs = + createMovableDsLoad(/*destReg=*/0, /*addrReg=*/810, /*ldsToken=*/3); + for (int i = 0; i < 12; ++i) + createWmmaF32_16x16x16_bf16(/*destStart=*/2000 + i * 16, /*src0Start=*/4000 + i * 16); + auto [signal, wait] = createMovableWorkgroupBarrier(bb, /*ldsToken=*/3); + (void)wait; + StinkyInstruction* postBarrierDs = + createMovableDsLoad(/*destReg=*/500, /*addrReg=*/820, /*ldsToken=*/3); + createWmmaF32_16x16x16_bf16(/*destStart=*/800, /*src0Start=*/500); + createSCmpWritingScc(bb, /*srcSgpr=*/90); + + runPassWithUnrollGemm(/*dsSpread=*/false); + + auto wmmasBetween = [&](const StinkyInstruction* from, const StinkyInstruction* to) { + int count = 0; + bool started = false; + for (const IRBase& ir : *bb) { + const auto* inst = dyn_cast(&ir); + if (inst == nullptr) continue; + if (inst == to) return started ? count : -1; + if (started && isMatrixInstruction(*inst)) ++count; + if (inst == from) started = true; + } + return -1; + }; + + ASSERT_NE(parentWmma, nullptr); + EXPECT_LT(positionOf(*bb, parentWmma), positionOf(*bb, preBarrierDs)) << scheduleOrder(*bb); + EXPECT_LT(positionOf(*bb, signal), positionOf(*bb, postBarrierDs)) << scheduleOrder(*bb); + EXPECT_GE(wmmasBetween(preBarrierDs, signal), 7) << scheduleOrder(*bb); +} + +// The overlap after can sit later than the parent-WMMA drain. WMMA 10 reads +// the VGPR the pre-barrier ds_load overwrites, so depThreshold is 10 + 56/8 + 1 +// = 18. One matching ds_load over 35 WMMAs and 2 ds_loads needs 18 windows, so +// the overlap after is 18 + 8 = 26. The post-barrier ds_load's before threshold +// is 4, and (26 + 4) / 2 = 15, which is under the parent drain. The barrier +// keeps 18: later than the average, earlier than the overlap after. The +// trailing compare keeps head balance off. The WMMAs after the consumer are +// the work that would fill a later overlap placement. +TEST_F(DAGSchedulerPassTest, BothSidedBarrierStopsAtTheParentDependencyNotTheOverlapAfter) { + bb->addSuccessor(bb); + + StinkyInstruction* parentWmma = nullptr; + for (int i = 0; i < 10; ++i) { + const int src = (i == 9) ? 0 : 3000 + i * 16; + StinkyInstruction* wmma = + createWmmaF32_16x16x16_bf16(/*destStart=*/1100 + i * 16, /*src0Start=*/src); + if (i == 9) parentWmma = wmma; + } + StinkyInstruction* preBarrierDs = + createMovableDsLoad(/*destReg=*/0, /*addrReg=*/810, /*ldsToken=*/3); + auto [signal, wait] = createMovableWorkgroupBarrier(bb, /*ldsToken=*/3); + (void)wait; + StinkyInstruction* postBarrierDs = + createMovableDsLoad(/*destReg=*/500, /*addrReg=*/820, /*ldsToken=*/3); + createWmmaF32_16x16x16_bf16(/*destStart=*/800, /*src0Start=*/500); + for (int i = 0; i < 24; ++i) + createWmmaF32_16x16x16_bf16(/*destStart=*/5000 + i * 16, /*src0Start=*/6000 + i * 16); + createSCmpWritingScc(bb, /*srcSgpr=*/90); + + runPassWithUnrollGemm(/*dsSpread=*/false); + + int wmmasBeforeBarrier = 0; + for (const IRBase& ir : *bb) { + const auto* inst = dyn_cast(&ir); + if (inst == nullptr) continue; + if (inst == signal) break; + if (isMatrixInstruction(*inst)) ++wmmasBeforeBarrier; + } + + ASSERT_NE(parentWmma, nullptr); + EXPECT_LT(positionOf(*bb, parentWmma), positionOf(*bb, preBarrierDs)) << scheduleOrder(*bb); + EXPECT_LT(positionOf(*bb, signal), positionOf(*bb, postBarrierDs)) << scheduleOrder(*bb); + EXPECT_EQ(wmmasBeforeBarrier, 18) << scheduleOrder(*bb); +} + TEST_F(DAGSchedulerPassTest, Layer2DoesNotPublishWithoutBeforeGroup) { bb->addSuccessor(bb); From 5b629c3ee056ded3facba7af88e198ec81611e41 Mon Sep 17 00:00:00 2001 From: hcman2 Date: Fri, 30 Oct 2026 01:26:22 +0000 Subject: [PATCH 2/6] Issue a barrier once its threshold is met The threshold already includes the last WMMA a feeding ds_load depends on and that load's drain. Holding a met barrier for another tail of WMMAs skipped it and let a later barrier be selected. --- .../docs/user/scheduler-tuning-parameters.md | 6 +-- .../stinkytofu/bindings/python/Module.hpp | 1 - .../include/stinkytofu/core/Types.hpp | 6 --- .../conversion/rocisa/ToStinkyTofuUtils.cpp | 6 ++- .../src/pipeline/backend/Gfx1250Backend.cpp | 2 - .../src/transforms/asm/dag/AGENTS.md | 19 ++++----- .../src/transforms/asm/dag/CDNA5.hpp | 42 ++----------------- .../tools/stinkytofu-opt/stinkytofu-opt.cpp | 7 ++-- 8 files changed, 24 insertions(+), 65 deletions(-) diff --git a/shared/stinkytofu/docs/user/scheduler-tuning-parameters.md b/shared/stinkytofu/docs/user/scheduler-tuning-parameters.md index 59eb9cc8aff0..47703c36fed6 100644 --- a/shared/stinkytofu/docs/user/scheduler-tuning-parameters.md +++ b/shared/stinkytofu/docs/user/scheduler-tuning-parameters.md @@ -99,8 +99,7 @@ the region's WMMAs. This replaces the old cap / throttle / slot-first / hide-bud |---|---|---|---| | `DsSpread` | true | `--ds-spread` / `--no-ds-spread` | Pace the region's ds_loads by WMMA batch (`DsBatchWmmas` WMMAs). Each batch takes the region's even share `ceil(ds_loads / usable batches)`, where the last `ceil(LDS return time / WMMA window) + kDrainSlackWmmas` WMMAs of the region are kept free for the last load to return. A sparse region therefore spreads its loads, a dense one gets a share above what the LDS rate fits (the credits throttle it), and a batch that began behind the even line takes `DsCatchUpPace` more. Nothing is held once the region's last WMMA has issued; a held ds_load never holds a WMMA. false = a ds_load issues as soon as it is ready and a credit is free (the credit queue is the only pacing). | | `DsBatchWmmas` | 4 | `--ds-batch-wmmas=N` | WMMAs per ds_load pacing batch (independent of the queue refill batch `WmmaBatchSize`). | -| `DsReadCyclesPerLoad` | 6 (`stinkytofu-opt`: 0 = off) | `--ds-read-cycles-per-load=N` | LDS return rate: cycles per ds_load. Capped at the region's even spread (`WMMAs x WMMA latency / ds_loads`). Decides how long the stage's ds_loads take to issue: `ceil(ds_loads x N / WMMA window)` WMMA windows from the region start. The barrier is placed after the last of them; the gap from the last ds_load to the barrier is the drain, set by the barrier cover. Replaces the old throttle latency T (one ds_load per `T / DsReadQueueDepth` cycles). Used only with `DsSpread`. | -| `WmmaBarrierCoverCycles` | -1 (automatic) | `--wmma-barrier-cover-cycles=N` | Queued WMMA cover a ready barrier waits for before it issues. -1 = a tail of 4 WMMAs (needs `DsSpread`); 0 = off; always capped at a full queue. Needs the queue model on. | +| `DsReadCyclesPerLoad` | 6 (`stinkytofu-opt`: 0 = off) | `--ds-read-cycles-per-load=N` | LDS return rate: cycles per ds_load. Capped at the region's even spread (`WMMAs x WMMA latency / ds_loads`). Decides how long the stage's ds_loads take to issue: `ceil(ds_loads x N / WMMA window)` WMMA windows from the region start. The barrier is placed after the last of them; the gap from the last ds_load to the barrier is the drain already inside the barrier threshold. Replaces the old throttle latency T (one ds_load per `T / DsReadQueueDepth` cycles). Used only with `DsSpread`. | | `DsCatchUpPace` | 2 | `--ds-catch-up-pace=N` | ds_loads a batch may take above the even share when it began behind the even line. 0 = no catching up. | | `DsReadQueueDepth` | 0 → HW 16 | `--ds-read-queue-depth=N` | In-flight ds_load credits modeled for the LDS return queue. A credit lives `HWModel::lds.readCreditLifetime` cycles. | | `DsReadDrainLatency` | 0 → dynamic | `--ds-read-drain-latency=N` | Barrier timing only: cycles a barrier waits for its ds_loads to return. 0 derives it from the matching loads. | @@ -122,9 +121,10 @@ loses its tuning unless it is translated: | `DsReadPerCap` A, `DsIssueCapSpanCycles` X, `DsIssueCapMode` | burst cap: at most A ds_loads per span X (default one WMMA window) | no equivalent: the per-batch share (`DsBatchWmmas`, `DsCatchUpPace`) replaces it | | `DsReadPerWmma`, `DsReadThrottleTransitionFactor`, `DsReadThrottleTransitionEntries`, `DsSlotFirst` | per-WMMA ds count, loads past the queue depth, ds before fillers | removed | | `BarrierHalfSlack` | WMMA windows kept inside a barrier signal/wait pair (0 = together) | removed: the separation slack is the constant 1 | +| `WmmaBarrierCoverCycles` | extra WMMAs a ready barrier waited for after its threshold (`-1` = 4) | removed: the threshold already includes the last WMMA to ds dependency and its drain | | `DsReadDrainLatency` | barrier timing only | unchanged | -The matching `stinkytofu-opt` flags and `--enable-wmma-hide-budget-prescan` are gone too. +The matching `stinkytofu-opt` flags, `--enable-wmma-hide-budget-prescan`, and `--wmma-barrier-cover-cycles` are gone too. The last is still accepted and ignored. TensileLite's `StinkyTofuDsIssueCapMode` and `StinkyTofuDsIssueCapSpanCycles` global parameters are gone. diff --git a/shared/stinkytofu/include/stinkytofu/bindings/python/Module.hpp b/shared/stinkytofu/include/stinkytofu/bindings/python/Module.hpp index e4a505b5aa6e..a4d4ef791fe4 100644 --- a/shared/stinkytofu/include/stinkytofu/bindings/python/Module.hpp +++ b/shared/stinkytofu/include/stinkytofu/bindings/python/Module.hpp @@ -145,7 +145,6 @@ X(WarGateWmmas, int, -1) /* WMMA src -> ds_load overwrite gap; -1 = derived */ \ X(PrefetchLeadWmmas, int, 25) /* 0 = prefetch issues when ready */ \ X(PrefetchLeadMinStageWmmas, int, 64) /* shorter stages run with no lead */ \ - X(WmmaBarrierCoverCycles, int, -1) /* cover kept for a barrier; < 0 = automatic */ \ X(DsSpread, bool, true) /* even ds_load spread over the WMMAs */ \ X(DsBatchWmmas, int, 4) /* WMMAs per ds_load pacing batch */ \ X(DsCatchUpPace, int, 2) /* extra ds_loads per batch when behind the even line */ \ diff --git a/shared/stinkytofu/include/stinkytofu/core/Types.hpp b/shared/stinkytofu/include/stinkytofu/core/Types.hpp index a5c79552b1d5..c21cbbf76fa9 100644 --- a/shared/stinkytofu/include/stinkytofu/core/Types.hpp +++ b/shared/stinkytofu/include/stinkytofu/core/Types.hpp @@ -121,12 +121,6 @@ struct PassFeatureConfig { /// (see DsSpreadPacer), and a load issues only while the ds queue has a free credit. Off = /// ds_loads issue as soon as they are ready and a credit is free. bool dsSpread = true; - /// Queued WMMA cover (cycles) to have in the pipe when a barrier issues, so the WMMAs hide - /// its stall. The WMMAs before the barrier's threshold are reserved for it: the queue is - /// refilled to this cover ahead of other work, and ds_loads the barrier waits on are - /// released that many WMMAs earlier. Applies with dsSpread (or when set > 0) and the - /// queue model; < 0 = automatic (72 cycles, at most a full queue), 0 = off. - int wmmaBarrierCoverCycles = -1; /// Cycles the LDS needs to return one ds_load (its return rate, the old throttle latency / /// queue depth). The windows the ds_loads in front of a barrier need are at least /// loads * cycles / WMMA window. 0 = off (the even-spread density only). diff --git a/shared/stinkytofu/src/conversion/rocisa/ToStinkyTofuUtils.cpp b/shared/stinkytofu/src/conversion/rocisa/ToStinkyTofuUtils.cpp index 1d5d501d32da..2aa9091bc2ea 100644 --- a/shared/stinkytofu/src/conversion/rocisa/ToStinkyTofuUtils.cpp +++ b/shared/stinkytofu/src/conversion/rocisa/ToStinkyTofuUtils.cpp @@ -1760,7 +1760,11 @@ void init_stinkytofu(nb::module_ m) { // NOLINT(misc-use-internal-linkage) #undef DEBUG_SET_MODULE_OPTION for (auto kv : options) { const std::string key = nb::cast(kv.first); - if (isRemovedDsPacingOption(key)) + if (key == "WmmaBarrierCoverCycles") + std::cerr << "[StinkyTofu] WARNING: module option '" << key + << "' was removed; a barrier issues when its threshold is met, " + << "and the option is ignored\n"; + else if (isRemovedDsPacingOption(key)) std::cerr << "[StinkyTofu] WARNING: module option '" << key << "' was removed with the legacy ds_load pacing and is ignored\n"; diff --git a/shared/stinkytofu/src/pipeline/backend/Gfx1250Backend.cpp b/shared/stinkytofu/src/pipeline/backend/Gfx1250Backend.cpp index 1ed87f493d8e..345cc634fb40 100644 --- a/shared/stinkytofu/src/pipeline/backend/Gfx1250Backend.cpp +++ b/shared/stinkytofu/src/pipeline/backend/Gfx1250Backend.cpp @@ -198,8 +198,6 @@ bool buildGfx1250Pipeline(ModulePassManager& mpm, StinkyAsmModule& module, const passFeatureConfig.dagFeatures.lockDsReadOrder = moduleOptions.LockDsReadOrder; passFeatureConfig.dagFeatures.evenSpreadFillers = moduleOptions.EvenSpreadFillers; passFeatureConfig.dagFeatures.dsSpread = moduleOptions.DsSpread; - passFeatureConfig.dagFeatures.wmmaBarrierCoverCycles = - moduleOptions.WmmaBarrierCoverCycles; passFeatureConfig.dagFeatures.dsCatchUpPace = moduleOptions.DsCatchUpPace; passFeatureConfig.dagFeatures.dsBatchWmmas = moduleOptions.DsBatchWmmas; passFeatureConfig.dagFeatures.dsReadCyclesPerLoad = diff --git a/shared/stinkytofu/src/transforms/asm/dag/AGENTS.md b/shared/stinkytofu/src/transforms/asm/dag/AGENTS.md index ba4578464e5e..13a8530997ae 100644 --- a/shared/stinkytofu/src/transforms/asm/dag/AGENTS.md +++ b/shared/stinkytofu/src/transforms/asm/dag/AGENTS.md @@ -37,20 +37,17 @@ When two rules want different things, the higher level wins. `PlacementLevel` in | L1 | `Barrier` | The barrier lands where it is planned. | | L2 | `LastDs` | The last ds before a barrier issues in time for that barrier. | | L3 | `WmmaQueue` | The WMMA queue stays busy: no window is wasted (the refill). | -| L4 | `WmmaTail` | Back-to-back WMMAs before a barrier cover its stall (`kBarrierTailWmmas`, `wmmaBarrierCover`). | -| L5 | `Prefetch` | Prefetches and global loads issue at their planned window. | -| L6 | `DsPacer` | ds loads are spread over the region (`DsSpreadPacer`). | -| L7 | `Filler` | SALU and VALU fill whatever slots are left. | +| L4 | `Prefetch` | Prefetches and global loads issue at their planned window. | +| L5 | `DsPacer` | ds loads are spread over the region (`DsSpreadPacer`). | +| L6 | `Filler` | SALU and VALU fill whatever slots are left. | Consequences: -- A lower level never vetoes a higher one. The ds pacer (L6) must not delay the last ds - before a barrier (L2). A filler (L7) must not take a prefetch's window (L5). -- Cover is measured, not counted. `wmma ds×8` does not cover a barrier; `wmma wmma ds×8` - does. A rule that asks "is the barrier covered" uses `queuedCoverCycles()`, not the number - of instructions picked. -- The WMMA tail (L4) exists to provide cover. VALU and SALU may co-issue inside it. Do not - block them there. +- A lower level never vetoes a higher one. The ds pacer (L5) must not delay the last ds + before a barrier (L2). A filler (L6) must not take a prefetch's window (L4). +- A barrier issues when its WMMA threshold is met. That threshold already includes the last + WMMA a feeding ds_load depends on and that load's drain. Do not hold the barrier for an + extra tail of WMMAs after the threshold. ## Rules diff --git a/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp b/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp index c0ba99a8c640..30bde3ad2dc6 100644 --- a/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp +++ b/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp @@ -473,20 +473,6 @@ class CDNA5ReadyQueue : public ReadyQueue { return std::max(0, getPassContext().getPassFeatureConfig().dagFeatures.wmmaQueueCoverCycles); } - // A barrier is preceded by a tail of kBarrierTailWmmas WMMAs issued back to back: the wave - // waits for the stage's last ds_loads while the tail runs, so the tail hides that much of the - // drain. 4 is the best batch size measured on gfx1250 (never above 8, the useful queue depth). - static constexpr int kBarrierTailWmmas = 4; - // Queued cover to hold for a barrier (0 = off); see DagFeatures::wmmaBarrierCoverCycles. - int wmmaBarrierCover(int wmmaLatency) const { - const auto& f = getPassContext().getPassFeatureConfig().dagFeatures; - if (wmmaQueueCover() <= 0 || f.wmmaBarrierCoverCycles == 0) return 0; - if (f.wmmaBarrierCoverCycles < 0 && !f.dsSpread) return 0; - const int fullQueue = wmmaQueueDepth() * std::max(1, wmmaLatency); - const int want = f.wmmaBarrierCoverCycles < 0 ? kBarrierTailWmmas * std::max(1, wmmaLatency) - : f.wmmaBarrierCoverCycles; - return std::min(want, fullQueue); - } // WMMA refill (see PassFeatureConfig::DagFeatures::wmmaRefillMode). A refill starts when the // queued cover drops below the refill line (wmmaQueueCover()) with room in the queue, and // keeps issuing WMMAs ahead of other work until it is done: Fixed after wmmaBatchSize @@ -496,7 +482,6 @@ class CDNA5ReadyQueue : public ReadyQueue { if (wmmaQueueCover() <= 0) return false; const auto& f = getPassContext().getPassFeatureConfig().dagFeatures; const bool room = outstandingWmmas() < wmmaQueueDepth(); - if (barrierWaitsForCover_) return true; if (wmmaRefilling_) { const int target = f.wmmaRefillToCycles > 0 ? f.wmmaRefillToCycles @@ -716,11 +701,6 @@ class CDNA5ReadyQueue : public ReadyQueue { // The region's ds_loads cost more issue cycles than its WMMAs take, so they cannot all hide // under a co-issue window: a ds_load the pace has released may issue outside one. bool dsOverflow_ = false; - // decidePromote() held a ready barrier back: the queued WMMA cover is below the barrier cover. - bool barrierWaitsForCover_ = false; - // WMMA count at which a ready barrier was first held for its tail (the hold lasts - // kBarrierTailWmmas). - std::unordered_map barrierCoverHoldStart_; BasicBlock* currentBB_ = nullptr; std::vector layer2BarrierOverlapCandidates_; @@ -969,7 +949,7 @@ class CDNA5ReadyQueue : public ReadyQueue { // new rule is a promoteDue*() function plus one row in decidePromote()'s rule // table at its level; a rule at an in-between level is a new enumerator. // A rule returns true once it has set promotedNode_. - enum class PlacementLevel { Barrier, LastDs, WmmaQueue, WmmaTail, Prefetch, DsPacer, Filler }; + enum class PlacementLevel { Barrier, LastDs, WmmaQueue, Prefetch, DsPacer, Filler }; void decidePromote(); void promote(PromotePhase phase, DAGNode* node, int kind = -1); bool promoteDueBarrier(); @@ -1798,7 +1778,6 @@ void CDNA5ReadyQueue::noteBarrierPairIssue(const DAGNode* node) { void CDNA5ReadyQueue::decidePromote() { promotedPhase_ = PromotePhase::None; promotedNode_ = nullptr; - barrierWaitsForCover_ = false; promotedKind_ = -1; using Rule = bool (CDNA5ReadyQueue::*)(); @@ -1821,7 +1800,9 @@ void CDNA5ReadyQueue::promote(PromotePhase phase, DAGNode* node, int kind) { promotedKind_ = kind; } -// A barrier whose WMMA-issued threshold is met, unless it is held for its WMMA tail. +// A barrier whose WMMA threshold is met. The threshold already waits out the last +// WMMA a feeding ds_load depends on, plus that load's drain, so a met barrier +// issues on this pick. bool CDNA5ReadyQueue::promoteDueBarrier() { if (barrierQueue.empty() || barrierWmmaThresholds_.empty()) return false; for (DAGNode* node : barrierQueue) { @@ -1829,19 +1810,6 @@ bool CDNA5ReadyQueue::promoteDueBarrier() { auto thIt = barrierWmmaThresholds_.find(node->inst); if (thIt == barrierWmmaThresholds_.end() || wmmaIssuedCountThisRegion_ < thIt->second) continue; - // The tail hold counts from the WMMA the threshold was met at, full queue or not. - const int heldSince = - barrierCoverHoldStart_.emplace(node, wmmaIssuedCountThisRegion_).first->second; - // The WMMAs queued behind the barrier hide its stall: refill first. - if (!wmmaQueue.empty() && outstandingWmmas() < wmmaQueueDepth()) { - const int latency = wmmaQueue.top()->inst->latencyCycles; - if (!isBarrierWait(*node->inst) && - wmmaIssuedCountThisRegion_ - heldSince < kBarrierTailWmmas && - queuedCoverCycles() < wmmaBarrierCover(latency)) { - barrierWaitsForCover_ = true; - continue; - } - } promote(PromotePhase::Barrier, node); PASS_DEBUG(std::cerr << "[CDNA5 decidePromote] promote barrier=" << node->inst << " wmmaIssuedCountThisRegion_=" << wmmaIssuedCountThisRegion_ @@ -2922,8 +2890,6 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato wmmaIssuedCountThisRegion_ = 0; wmmaRefilling_ = false; wmmaRefillLeft_ = 0; - barrierWaitsForCover_ = false; - barrierCoverHoldStart_.clear(); lastPickedNode_ = nullptr; // SCC chain locks are per-region: chain ids index the prior region's // DAGNodeList, and region boundaries are side-effect cuts no reordering diff --git a/shared/stinkytofu/tools/stinkytofu-opt/stinkytofu-opt.cpp b/shared/stinkytofu/tools/stinkytofu-opt/stinkytofu-opt.cpp index ccc23153775e..6ec7a4d5852d 100644 --- a/shared/stinkytofu/tools/stinkytofu-opt/stinkytofu-opt.cpp +++ b/shared/stinkytofu/tools/stinkytofu-opt/stinkytofu-opt.cpp @@ -559,9 +559,10 @@ int main(int argc, char** argv) { } else if (a.starts_with("--wmma-queue-cover-cycles=")) { passFeatureConfig.dagFeatures.wmmaQueueCoverCycles = std::stoi(a.substr(std::string("--wmma-queue-cover-cycles=").size())); - } else if (a.starts_with("--wmma-barrier-cover-cycles=")) { - passFeatureConfig.dagFeatures.wmmaBarrierCoverCycles = - std::stoi(a.substr(std::string("--wmma-barrier-cover-cycles=").size())); + } else if (a == "--wmma-barrier-cover-cycles" || + a.starts_with("--wmma-barrier-cover-cycles=")) { + std::cerr << "warning: --wmma-barrier-cover-cycles was removed; a barrier issues " + << "when its threshold is met, and the flag is ignored\n"; } else if (a == "--ds-spread") { passFeatureConfig.dagFeatures.dsSpread = true; } else if (a == "--no-ds-spread") { From 447a397c82f3d881c8df890b83284c090e66ca56 Mon Sep 17 00:00:00 2001 From: hcman2 Date: Fri, 30 Oct 2026 03:06:30 +0000 Subject: [PATCH 3/6] Force tensorload with barrier order --- .../src/transforms/asm/dag/CDNA5.hpp | 101 ++++++++++++++---- .../tests/unit/asm/DAGSchedulerPassTest.cpp | 19 +++- 2 files changed, 96 insertions(+), 24 deletions(-) diff --git a/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp b/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp index 30bde3ad2dc6..37630b81dd42 100644 --- a/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp +++ b/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp @@ -3152,10 +3152,48 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato return result; }; + auto pseudoTokens = [](const StinkyInstruction& inst) { + std::unordered_set tokens; + for (const auto* regs : {&inst.getSrcRegs(), &inst.getDestRegs()}) { + for (const StinkyRegister& reg : *regs) + if (isPseudoReg(reg)) tokens.insert(reg.reg.idx); + } + return tokens; + }; // Computed once per group (not once per after x before pair). + // Register descendants miss the real shape: a tensor_load is the next + // LDS producer, so it defines the token instead of using the barrier's + // definition. Take the same-token tensor_loads that follow the after + // group until the next barrier. for (auto& afterGroup : exclusiveAfterGroups) { afterGroup.descendantLoads = collectDescendants( afterGroup.barriers, [](StinkyInstruction& inst) { return isTensorLoad(inst); }); + std::unordered_set tokens; + std::unordered_set groupBarriers(afterGroup.barriers.begin(), + afterGroup.barriers.end()); + for (StinkyInstruction* barrier : afterGroup.barriers) { + auto barrierTokens = pseudoTokens(*barrier); + tokens.insert(barrierTokens.begin(), barrierTokens.end()); + } + std::unordered_set seen(afterGroup.descendantLoads.begin(), + afterGroup.descendantLoads.end()); + bool passedGroup = false; + for (IRList::iterator it = regionStart; it != regionEnd; ++it) { + auto* inst = dyn_cast(it.getNodePtr()); + if (inst == nullptr) continue; + if (groupBarriers.count(inst) != 0) { + passedGroup = true; + continue; + } + if (!passedGroup) continue; + if (isBarrier(*inst)) break; + if (!isTensorLoad(*inst)) continue; + auto loadTokens = pseudoTokens(*inst); + if (std::none_of(loadTokens.begin(), loadTokens.end(), + [&](uint32_t token) { return tokens.count(token) != 0; })) + continue; + if (seen.insert(inst).second) afterGroup.descendantLoads.push_back(inst); + } } for (auto& beforeGroup : exclusiveBeforeGroups) { beforeGroup.descendantLoads = collectDescendants( @@ -3197,8 +3235,38 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato const int baseBeforeEnd = baseBeforeBegin + beforeGroup.claimWindow; const bool overlap = (baseAfterBegin < baseBeforeEnd) && (baseBeforeBegin < baseAfterEnd + separationSlack); + // Even when the two claimed windows do not overlap, an after + // group whose threshold is earlier than the before group has + // room for its tensor load. Keep that load in the gap instead + // of allowing the later barrier to overtake it. + const bool tensorLoadFitsBetween = afterGroup.threshold < beforeGroup.threshold; int proportionalSplit = -1; const char* placement = "none"; + if (overlap || tensorLoadFitsBetween) { + auto requireOrdering = [&](StinkyInstruction* predecessor, + StinkyInstruction* successor) { + deps.requestedConstraints.emplace_back(predecessor, successor); + }; + + // barrierAfter -> tensor_load -> barrierBefore. The + // tensor_load is the next producer of the after token, so + // def-use does not put it after the barrier; request that + // edge explicitly. + for (StinkyInstruction* barrierAfter : afterGroup.barriers) { + for (StinkyInstruction* barrierBefore : beforeGroup.barriers) + requireOrdering(barrierAfter, barrierBefore); + for (StinkyInstruction* tensorLoad : afterGroup.descendantLoads) + requireOrdering(barrierAfter, tensorLoad); + } + for (StinkyInstruction* tensorLoad : afterGroup.descendantLoads) { + for (StinkyInstruction* barrierBefore : beforeGroup.barriers) { + requireOrdering(tensorLoad, barrierBefore); + } + for (StinkyInstruction* dsLoad : beforeGroup.descendantLoads) { + requireOrdering(tensorLoad, dsLoad); + } + } + } if (overlap) { // afterNeeded is issue-only (no drain); beforeNeeded is the // before claim window. @@ -3217,32 +3285,25 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato // every requested ordering against final instruction order first. Layer2BarrierOverlapCandidate overlapCandidate{ afterGroup.barriers, beforeGroup.barriers, {}}; - auto requireOrdering = [&](StinkyInstruction* predecessor, - StinkyInstruction* successor) { - deps.requestedConstraints.emplace_back(predecessor, successor); + auto requireOverlapOrdering = [&](StinkyInstruction* predecessor, + StinkyInstruction* successor) { overlapCandidate.requiredConstraints.emplace_back(predecessor, successor); }; - // Every overlapping pair needs structural ordering, independent of - // which threshold-adjustment branch above was taken. + // The same hard orderings were requested above. Record + // their provenance only for overlap candidates, which are + // the only pairs eligible for barrier merging. for (StinkyInstruction* barrierAfter : afterGroup.barriers) { - for (StinkyInstruction* barrierBefore : beforeGroup.barriers) { - requireOrdering(barrierAfter, barrierBefore); - } + for (StinkyInstruction* barrierBefore : beforeGroup.barriers) + requireOverlapOrdering(barrierAfter, barrierBefore); + for (StinkyInstruction* tensorLoad : afterGroup.descendantLoads) + requireOverlapOrdering(barrierAfter, tensorLoad); } - - // Also keep tensor_load descendants of barrierAfter ahead of the - // barrierBefore group, and prevent them from interleaving with - // ds_load descendants of barrierBefore: all matching tensor_loads - // must issue first. Both descendant sets were precomputed once per - // group above. for (StinkyInstruction* tensorLoad : afterGroup.descendantLoads) { - for (StinkyInstruction* barrierBefore : beforeGroup.barriers) { - requireOrdering(tensorLoad, barrierBefore); - } - for (StinkyInstruction* dsLoad : beforeGroup.descendantLoads) { - requireOrdering(tensorLoad, dsLoad); - } + for (StinkyInstruction* barrierBefore : beforeGroup.barriers) + requireOverlapOrdering(tensorLoad, barrierBefore); + for (StinkyInstruction* dsLoad : beforeGroup.descendantLoads) + requireOverlapOrdering(tensorLoad, dsLoad); } layer2BarrierOverlapCandidates_.push_back(std::move(overlapCandidate)); diff --git a/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp b/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp index d9cb7b1b9e5d..baefe838b03d 100644 --- a/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp +++ b/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp @@ -783,17 +783,22 @@ TEST_F(DAGSchedulerPassTest, Layer2DoesNotPublishWhenPerWmmaBudgetsSeparateWindo EXPECT_EQ(waits, 2); } -// A non-overlapping pair stays on one threshold and issues together. -TEST_F(DAGSchedulerPassTest, NonOverlappingBarrierPairStaysTogether) { +// Non-overlapping after/before groups still keep the after token's next +// tensor_load between them when the after threshold is earlier. The load +// defines that token, so it is not a register user of the barrier. The before +// barrier starts ahead of the load, so only the new hard edges can move it +// behind the load. +TEST_F(DAGSchedulerPassTest, NonOverlappingBarrierPairKeepsTensorLoadBetweenGroups) { bb->addSuccessor(bb); createMovableDsLoad(/*destReg=*/0, /*addrReg=*/200, /*ldsToken=*/0); createWmmaF32_16x16x16_bf16(/*destStart=*/100, /*src0Start=*/0); for (int i = 0; i < 22; ++i) createWmmaF32_16x16x16_bf16(/*destStart=*/400 + i * 16, /*src0Start=*/64 + i * 16); - auto [afterSignal, afterWait] = createMovableWorkgroupBarrier(bb, /*ldsToken=*/0); - createMovableTensorLoad(bb, /*src0Reg=*/220, /*src1Reg=*/224, /*ldsToken=*/0); auto [beforeSignal, beforeWait] = createMovableWorkgroupBarrier(bb, /*ldsToken=*/1); + auto [afterSignal, afterWait] = createMovableWorkgroupBarrier(bb, /*ldsToken=*/0); + StinkyInstruction* tensorLoad = + createMovableTensorLoad(bb, /*src0Reg=*/220, /*src1Reg=*/224, /*ldsToken=*/0); createMovableDsLoad(/*destReg=*/500, /*addrReg=*/204, /*ldsToken=*/1); createWmmaF32_16x16x16_bf16(/*destStart=*/800, /*src0Start=*/500); @@ -816,6 +821,12 @@ TEST_F(DAGSchedulerPassTest, NonOverlappingBarrierPairStaysTogether) { EXPECT_EQ(wmmasBetween(afterSignal, afterWait), 0) << scheduleOrder(*bb); EXPECT_EQ(positionOf(*bb, beforeWait), positionOf(*bb, beforeSignal) + 1) << scheduleOrder(*bb); EXPECT_EQ(wmmasBetween(beforeSignal, beforeWait), 0) << scheduleOrder(*bb); + EXPECT_LT(positionOf(*bb, afterWait), positionOf(*bb, tensorLoad)) << scheduleOrder(*bb); + EXPECT_LT(positionOf(*bb, tensorLoad), positionOf(*bb, beforeSignal)) << scheduleOrder(*bb); + + const auto* overlaps = am.getCachedResult(); + ASSERT_NE(overlaps, nullptr); + EXPECT_FALSE(overlaps->contains(afterSignal, beforeSignal)); } // The latest matching ds_load does not overlap any WMMA, so lastOverlap is 0. From ce978b60d275b94e924143171dd064552ca023dd Mon Sep 17 00:00:00 2001 From: hcman2 Date: Fri, 30 Oct 2026 06:40:31 +0000 Subject: [PATCH 4/6] Fix merge barrier bug. --- .../docs/user/scheduler-tuning-parameters.md | 2 +- .../include/stinkytofu/core/Types.hpp | 10 +- .../transforms/asm/StinkyMergeBarrierPass.hpp | 10 +- .../transforms/asm/StinkyMergeBarrierPass.cpp | 83 +++++-------- .../tests/unit/asm/MergeBarrierPassTest.cpp | 116 ++++++++++-------- 5 files changed, 102 insertions(+), 119 deletions(-) diff --git a/shared/stinkytofu/docs/user/scheduler-tuning-parameters.md b/shared/stinkytofu/docs/user/scheduler-tuning-parameters.md index 47703c36fed6..71daf340492c 100644 --- a/shared/stinkytofu/docs/user/scheduler-tuning-parameters.md +++ b/shared/stinkytofu/docs/user/scheduler-tuning-parameters.md @@ -159,7 +159,7 @@ Per WMMA window, at most `globalReadPerWmma` (arch default 1) tensor_loads issue | `WarGateWmmas` | -1 → derived `L / I` | `--war-gate-wmmas=N` | WMMAs a ds_load waits before overwriting a vgpr a WMMA read. Active only with `EnableESM2` + `EnableESM2TrackValuVsrc`. | | `ClusterBarrierRule3SignalLeadCycles` | -1 → heuristic | – | Cluster-barrier Rule 3 signal lead: 200 if the main loop's WMMA latency sum > 500, else 100. See [Insert cluster barrier pass](../developer/cluster-barrier.md). | | `ReduceClusterBarrierPerLoop` | -1 → heuristic | – | `1`: first Rule 3 handshake of each enclosing loop, then another after each 2000 estimated cycles. Nearby HalfPLR / ExpandPointerSwap copies still share one handshake. `0`: every loop trigger handshakes. Unset turns on when the main loop's WMMA latency sum > 2500 and that sum / ds_load count >= 5. See [Insert cluster barrier pass](../developer/cluster-barrier.md). | -| *CLI only* `mergeBarrierThreshold` | 0 → 11 | `--merge-barrier-threshold=N` | Max cycle distance for `StinkyMergeBarrierPass` to merge adjacent barrier groups. | +| *CLI only* `mergeBarrierThreshold` | unused | `--merge-barrier-threshold=N` | Parsed but ignored. `StinkyMergeBarrierPass` merges adjacent groups only when no matrix instruction is issued between them. | ## Arch defaults and hardware facts diff --git a/shared/stinkytofu/include/stinkytofu/core/Types.hpp b/shared/stinkytofu/include/stinkytofu/core/Types.hpp index c21cbbf76fa9..7754d6494b14 100644 --- a/shared/stinkytofu/include/stinkytofu/core/Types.hpp +++ b/shared/stinkytofu/include/stinkytofu/core/Types.hpp @@ -136,11 +136,11 @@ struct PassFeatureConfig { /// WMMA windows by the region's matrix latency. 0 disables the extra /// gap. Mirrors ModuleOptions::TensorLoadDsLoadGapCycles. int tensorLoadDsLoadGapCycles = 64; - /// Max cycle-distance between two adjacent barrier groups for - /// StinkyMergeBarrierPass to merge them into a single multi-token - /// barrier group. 0 = use the CDNA5 default (kCdna5MergeBarrierThreshold). - /// Internal tuning knob only — deliberately not surfaced as a module - /// option, so TensileLite cannot set it. + /// Unused by StinkyMergeBarrierPass. Adjacent groups merge only when no + /// matrix instruction is issued between them. Kept so existing callers + /// and --merge-barrier-threshold= still parse. Internal knob only — + /// deliberately not surfaced as a module option, so TensileLite cannot + /// set it. int mergeBarrierThreshold = 0; /// Mirrors ModuleOptions::ClusterBarrier: InsertClusterBarrierPass will run /// after the scheduler and plant SCC-clobbering handshakes around workgroup diff --git a/shared/stinkytofu/include/stinkytofu/transforms/asm/StinkyMergeBarrierPass.hpp b/shared/stinkytofu/include/stinkytofu/transforms/asm/StinkyMergeBarrierPass.hpp index cb354e803cce..8d58ec045db2 100644 --- a/shared/stinkytofu/include/stinkytofu/transforms/asm/StinkyMergeBarrierPass.hpp +++ b/shared/stinkytofu/include/stinkytofu/transforms/asm/StinkyMergeBarrierPass.hpp @@ -36,11 +36,11 @@ class Pass; /// exactly an adjacent s_barrier_signal / s_barrier_wait pair with the same /// non-empty memory-token set. Two consecutive legal groups may be fused only /// when Layer2BarrierOverlapAnalysis contains the final-order-validated, -/// directional earlier-to-later pair, their token sets differ, and their -/// modeled cycle-distance is below the merge threshold -/// (PassFeatureConfig::DagFeatures::mergeBarrierThreshold, or the CDNA5 default -/// when unset): the earlier pair absorbs the later tokens and the redundant -/// second signal/wait pair is removed. +/// directional earlier-to-later pair, their token sets differ, and no matrix +/// instruction is issued between them. One issued WMMA occupies the WMMA queue +/// long enough to hide the later barrier, so cycle distance is not a gate. The +/// earlier pair absorbs the later tokens and the redundant second signal/wait +/// pair is removed. STINKYTOFU_EXPORT std::unique_ptr createStinkyMergeBarrierPass(); } // namespace stinkytofu diff --git a/shared/stinkytofu/src/transforms/asm/StinkyMergeBarrierPass.cpp b/shared/stinkytofu/src/transforms/asm/StinkyMergeBarrierPass.cpp index aa0f19116c44..fda36be4c8c5 100644 --- a/shared/stinkytofu/src/transforms/asm/StinkyMergeBarrierPass.cpp +++ b/shared/stinkytofu/src/transforms/asm/StinkyMergeBarrierPass.cpp @@ -42,9 +42,9 @@ // ====================== // Runs immediately after StinkyDAGSchedulerPass. Within loop bodies it looks // for two (or more) barrier groups that Layer 2 identified as a directional -// overlapping pair and the scheduler placed only a few cycles apart. It fuses -// them into a single group carrying the union of their memory tokens, dropping -// the redundant second signal/wait pair. +// overlapping pair and the scheduler placed with no matrix instruction between +// them. It fuses them into a single group carrying the union of their memory +// tokens, dropping the redundant second signal/wait pair. // // A legal "barrier group" is exactly one adjacent s_barrier_signal / // s_barrier_wait pair that shares the same non-empty LDS token set, e.g.: @@ -53,23 +53,19 @@ // Anything else (lone signal/wait, signal/.../wait with filler between, mismatched // tokens) is ignored. Two consecutive legal groups G1 (tokens T1) and G2 (tokens // T2) may merge only when Layer2BarrierOverlapAnalysis contains G1 -> G2, -// T1 ≠ T2, and the modeled cycle-distance is below the configured threshold. +// T1 ≠ T2, and no matrix instruction is issued between them. Issuing a WMMA +// occupies the WMMA queue long enough to hide the later barrier, so cycle +// distance is not a merge gate. // -// The merge never moves instructions: it only unions G2's tokens into G1's -// barriers and drops G2's redundant signal/wait pair. That is correct precisely -// when nothing between the two groups is ordered against the merged barrier — no -// other barrier and no producer/consumer of a merged token (T1 ∪ T2) sits in -// between. When such an instruction exists the merge is skipped, because keeping -// the barrier in place would move it to the wrong side of that dependency. +// The merge never moves ordinary instructions: it unions G2's tokens into G1 +// and drops G2. G1 stays where it is, so a producer or consumer of T1 may sit +// between the groups. G2 is the barrier that moves earlier. A producer or +// consumer of T2 between the groups would end up on the wrong side of it, and +// another barrier between them would too, so those pairs are left unmerged. namespace { using namespace stinkytofu; -// CDNA5 (Gfx1250) default merge distance, in cycles. Used when -// dagFeatures.mergeBarrierThreshold holds the sentinel value (0). Explicit -// non-sentinel config wins. Mirrors the kCdna5* tunables in dag/CDNA5.hpp. -constexpr int kCdna5MergeBarrierThreshold = 11; - // An adjacent s_barrier_signal / s_barrier_wait pair with one shared token set. struct BarrierGroup { std::vector barriers; // [signal, wait] @@ -161,21 +157,6 @@ std::vector collectBarrierGroups(BasicBlock& bb) { return groups; } -// Sum of issueCycles over the instructions strictly between \p afterIt and -// \p beforeIt (both exclusive). Non-StinkyInstruction IR contributes nothing. -int cycleDistance(IRList::iterator afterIt, IRList::iterator beforeIt) { - int cycles = 0; - for (auto it = std::next(afterIt); it != beforeIt; ++it) { - auto* inst = dyn_cast(it.getNodePtr()); - if (inst == nullptr) continue; - if (isMatrixInstruction(*inst)) - cycles += inst->latencyCycles; - else - cycles += inst->issueCycles; - } - return cycles; -} - // Add \p tokenId as an LDS pseudo register to \p regs if not already present. void addUniqueLds(std::vector& regs, uint32_t tokenId, bool /*isDest*/) { for (const StinkyRegister& r : regs) @@ -227,8 +208,8 @@ void setMergedBarrierComment(StinkyInstruction* barrier, } // Attempt to merge the two consecutive groups g1 (earlier) and g2 (later) inside -// \p bb. Returns true on success (IR mutated). \p threshold is in cycles. -bool tryMergePair(BasicBlock& bb, const BarrierGroup& g1, const BarrierGroup& g2, int threshold, +// \p bb. Returns true on success (IR mutated). +bool tryMergePair(BasicBlock& bb, const BarrierGroup& g1, const BarrierGroup& g2, Layer2OverlapResult& overlaps) { if (!isLayer2OverlapPair(g1, g2, overlaps)) return false; @@ -236,24 +217,25 @@ bool tryMergePair(BasicBlock& bb, const BarrierGroup& g1, const BarrierGroup& g2 // legal groups are successive syncs of one token and must both remain. if (g1.tokens == g2.tokens) return false; - const int dist = cycleDistance(g1.lastIt, g2.firstIt); - if (dist >= threshold) return false; - - // Merged barrier will guard the union of both token sets. - std::unordered_set mergedTokens = g1.tokens; - mergedTokens.insert(g2.tokens.begin(), g2.tokens.end()); - - // Merge without moving instructions. This is only correct when nothing - // strictly between the two groups is ordered against the merged barrier: - // no other barrier, and no producer/consumer of a merged token. Otherwise - // folding G2 back onto G1's position would cross that dependency — bail. + // G1 stays put and absorbs G2's tokens; G2 is the barrier that moves + // earlier. An instruction between the groups may touch G1's tokens. It + // must not touch G2's tokens, and another barrier must not sit between + // them: either one would be crossed by folding G2 back onto G1. + // A matrix instruction between them also blocks the merge. Issuing one + // WMMA, including scaled WMMA, fills the WMMA queue for long enough to + // hide G2, so the groups stay separate no matter how few cycles the + // modeled latency reports. for (auto it = std::next(g1.lastIt); it != g2.firstIt; ++it) { auto* inst = dyn_cast(it.getNodePtr()); if (inst == nullptr) continue; if (isBarrier(*inst)) return false; - if (touchesTokens(*inst, mergedTokens)) return false; + if (isMatrixInstruction(*inst)) return false; + if (touchesTokens(*inst, g2.tokens)) return false; } + std::unordered_set mergedTokens = g1.tokens; + mergedTokens.insert(g2.tokens.begin(), g2.tokens.end()); + // Safe: fold g2's tokens into every barrier of g1, then drop g2's redundant // signal/wait pair. No instruction is moved. for (StinkyInstruction* barrier : g1.barriers) { @@ -268,13 +250,13 @@ bool tryMergePair(BasicBlock& bb, const BarrierGroup& g1, const BarrierGroup& g2 // Repeatedly merge mergeable adjacent barrier-group pairs in \p bb until a fixed // point. Chained close groups collapse into one multi-token group. -void mergeBarriersInBlock(BasicBlock& bb, int threshold, Layer2OverlapResult& overlaps) { +void mergeBarriersInBlock(BasicBlock& bb, Layer2OverlapResult& overlaps) { bool changed = true; while (changed) { changed = false; std::vector groups = collectBarrierGroups(bb); for (size_t i = 0; i + 1 < groups.size(); ++i) { - if (tryMergePair(bb, groups[i], groups[i + 1], threshold, overlaps)) { + if (tryMergePair(bb, groups[i], groups[i + 1], overlaps)) { changed = true; break; // group layout changed; rebuild before continuing } @@ -299,13 +281,6 @@ class StinkyMergeBarrierPass : public StinkyInstPass { if (passCtx.getGemmTileConfig().arch[0] != 12 || passCtx.getGemmTileConfig().arch[1] != 5) return preserveCFGAnalyses(); - const int cfg = passCtx.getPassFeatureConfig().dagFeatures.mergeBarrierThreshold; - const int threshold = cfg > 0 ? cfg : kCdna5MergeBarrierThreshold; - - if (threshold == -1) { - return preserveCFGAnalyses(); - } - // Copy because chained merges redirect relations away from removed barriers. Layer2OverlapResult layer2Overlaps = AM.getResult(func); @@ -321,7 +296,7 @@ class StinkyMergeBarrierPass : public StinkyInstPass { for (BasicBlock& bb : func) { if (!loopBodyBBs.count(&bb)) continue; if (!passCtx.shouldProcessBasicBlock(bb)) continue; - mergeBarriersInBlock(bb, threshold, layer2Overlaps); + mergeBarriersInBlock(bb, layer2Overlaps); } return preserveCFGAnalyses(); } diff --git a/shared/stinkytofu/tests/unit/asm/MergeBarrierPassTest.cpp b/shared/stinkytofu/tests/unit/asm/MergeBarrierPassTest.cpp index 7032fc0a0ccb..5a51e0e174a9 100644 --- a/shared/stinkytofu/tests/unit/asm/MergeBarrierPassTest.cpp +++ b/shared/stinkytofu/tests/unit/asm/MergeBarrierPassTest.cpp @@ -23,9 +23,9 @@ // Unit tests for StinkyMergeBarrierPass. // // The pass runs after StinkyDAGSchedulerPass and fuses Layer 2 overlapping -// barrier groups that sit closer than DagFeatures::mergeBarrierThreshold cycles -// apart into a single multi-token group. These tests build barrier groups -// directly in a self-loop body block and publish the scheduler analysis result. +// barrier groups when no matrix instruction is issued between them. Cycle +// distance is not a gate. These tests build barrier groups directly in a +// self-loop body block and publish the scheduler analysis result. #include #include @@ -45,9 +45,6 @@ using namespace stinkytofu; using namespace stinkytofu::test; -// Default used by the pass when mergeBarrierThreshold is left at the sentinel 0. -static constexpr int kCdna5MergeBarrierThreshold = 11; - class MergeBarrierPassTest : public ::testing::Test { protected: GfxArchID arch = GfxArchID::Gfx1250; @@ -89,6 +86,20 @@ class MergeBarrierPassTest : public ::testing::Test { return inst; } + // Scaled WMMA used by mxf8. It carries IF_MXWMMA, not only IF_WMMA, and + // isMatrixInstruction must still see it as a matrix issue. + StinkyInstruction* createScaleWmma() { + AsmIRBuilder builder(*bb, arch); + const HwInstDesc* desc = getMCIDByUOp(GFX::v_wmma_scale_f32_16x16x128_f8f6f4, arch); + if (desc == nullptr) return nullptr; + StinkyInstruction* inst = builder.create(desc); + inst->addDestReg(StinkyRegister("v", 0, 8)); + inst->addSrcReg(StinkyRegister("v", 16, 8)); + inst->addSrcReg(StinkyRegister("v", 16, 8)); + inst->addSrcReg(StinkyRegister("v", 0, 8)); + return inst; + } + std::vector> legalBarrierGroups() { std::vector> groups; for (auto it = bb->begin(); it != bb->end(); ++it) { @@ -103,9 +114,10 @@ class MergeBarrierPassTest : public ::testing::Test { } // Run BuildImplicitDependency (to attach LDS pseudo tokens to barriers) then - // MergeBarrier with the given cycle threshold (0 => pass default). Most - // tests publish the forward Layer 2 relation for each consecutive group; - // dedicated gate tests can omit or reverse it. + // MergeBarrier. mergeThreshold is accepted so existing call sites stay + // stable; the pass no longer consults it. Most tests publish the forward + // Layer 2 relation for each consecutive group; dedicated gate tests can + // omit or reverse it. void runPasses(int mergeThreshold, bool publishOverlap = true, bool reverseOverlap = false) { PassContext ctx; ctx.setGemmTileConfig(config); @@ -175,9 +187,9 @@ class MergeBarrierPassTest : public ::testing::Test { } }; -// Two directly-adjacent barrier groups (distance 0) merge under the default -// threshold, collapsing to a single signal/wait pair carrying both tokens. -TEST_F(MergeBarrierPassTest, AdjacentGroupsMergeWithDefaultThreshold) { +// Two directly-adjacent barrier groups merge, collapsing to a single +// signal/wait pair carrying both tokens. +TEST_F(MergeBarrierPassTest, AdjacentGroupsMerge) { createSignal(0); createWait(0); createSignal(1); @@ -186,7 +198,7 @@ TEST_F(MergeBarrierPassTest, AdjacentGroupsMergeWithDefaultThreshold) { EXPECT_EQ(countByMnemonic("s_barrier_signal"), 2); EXPECT_EQ(countByMnemonic("s_barrier_wait"), 2); - runPasses(/*mergeThreshold=*/0); // 0 => default kCdna5MergeBarrierThreshold + runPasses(/*mergeThreshold=*/0); EXPECT_EQ(countByMnemonic("s_barrier_signal"), 1); EXPECT_EQ(countByMnemonic("s_barrier_wait"), 1); @@ -221,46 +233,48 @@ TEST_F(MergeBarrierPassTest, ReverseLayer2OverlapPairPreventsMerge) { EXPECT_EQ(countByMnemonic("s_barrier_wait"), 2); } -// Distance strictly below the threshold merges; equal-or-above does not. -TEST_F(MergeBarrierPassTest, ThresholdBoundaryControlsMerge) { +// Non-matrix instructions between the groups do not hide a barrier, so the +// groups still merge even when their issue cycles exceed the old threshold. +TEST_F(MergeBarrierPassTest, NonMatrixInstructionsBetweenStillMerge) { createSignal(0); createWait(0); - // Neutral VALU filler (neither token producer nor consumer). - StinkyInstruction* f0 = createVAddInBlock(bb, arch, /*dest=*/40, /*s0=*/0, /*s1=*/1); - StinkyInstruction* f1 = createVAddInBlock(bb, arch, /*dest=*/41, /*s0=*/2, /*s1=*/3); + int cycles = 0; + for (int i = 0; i < 20; ++i) { + StinkyInstruction* filler = + createVAddInBlock(bb, arch, /*dest=*/40 + i, /*s0=*/0, /*s1=*/1); + cycles += filler->issueCycles; + } createSignal(1); createWait(1); - const int distance = f0->issueCycles + f1->issueCycles; - ASSERT_GT(distance, 0) << "filler must contribute a positive cycle distance"; - - // Threshold equal to the distance: dist >= threshold, so no merge. - runPasses(/*mergeThreshold=*/distance); - EXPECT_EQ(countByMnemonic("s_barrier_signal"), 2); - EXPECT_EQ(countByMnemonic("s_barrier_wait"), 2); + ASSERT_GT(cycles, 11); + // The old cycle gate would reject this. A tiny threshold must not matter. + runPasses(/*mergeThreshold=*/1); + EXPECT_EQ(countByMnemonic("s_barrier_signal"), 1); + EXPECT_EQ(countByMnemonic("s_barrier_wait"), 1); + EXPECT_EQ(tokensOf(firstBarrier("s_barrier_signal")), (std::vector{0, 1})); } -TEST_F(MergeBarrierPassTest, DistanceBelowThresholdMerges) { +// One issued WMMA is enough queued latency to hide the later barrier. +TEST_F(MergeBarrierPassTest, MatrixInstructionBetweenPreventsMerge) { createSignal(0); createWait(0); - StinkyInstruction* f0 = createVAddInBlock(bb, arch, /*dest=*/40, /*s0=*/0, /*s1=*/1); - StinkyInstruction* f1 = createVAddInBlock(bb, arch, /*dest=*/41, /*s0=*/2, /*s1=*/3); + StinkyInstruction* wmma = createScaleWmma(); + ASSERT_NE(wmma, nullptr); + ASSERT_TRUE(isMatrixInstruction(*wmma)); createSignal(1); createWait(1); - const int distance = f0->issueCycles + f1->issueCycles; + runPasses(/*mergeThreshold=*/100000); - // Threshold one above the distance: dist < threshold, so merge. - runPasses(/*mergeThreshold=*/distance + 1); - EXPECT_EQ(countByMnemonic("s_barrier_signal"), 1); - EXPECT_EQ(countByMnemonic("s_barrier_wait"), 1); - EXPECT_EQ(tokensOf(firstBarrier("s_barrier_signal")), (std::vector{0, 1})); + EXPECT_EQ(countByMnemonic("s_barrier_signal"), 2); + EXPECT_EQ(countByMnemonic("s_barrier_wait"), 2); + EXPECT_EQ(countByMnemonic("v_wmma_scale_f32_16x16x128_f8f6f4"), 1); } -// A producer of a merged token sitting between the two groups is ordered against -// the merged barrier. Since the pass never moves instructions, the merge must be -// skipped even when the distance is below threshold. -TEST_F(MergeBarrierPassTest, TokenProducerBetweenPreventsMerge) { +// A producer of the later group's token sits before that barrier. Folding the +// later barrier back would put the producer after it, so the merge is skipped. +TEST_F(MergeBarrierPassTest, LaterTokenProducerBetweenPreventsMerge) { createSignal(0); createWait(0); // Producer of token 1 (writes LDS token 1). @@ -277,22 +291,21 @@ TEST_F(MergeBarrierPassTest, TokenProducerBetweenPreventsMerge) { EXPECT_EQ(countByMnemonic("tensor_load_to_lds"), 1); } -// A consumer of a merged token (ds_read of token 0, guarded by the first group) -// sitting between the groups is likewise ordered against the merged barrier, so -// the merge is skipped without moving anything. -TEST_F(MergeBarrierPassTest, TokenConsumerBetweenPreventsMerge) { +// The earlier barrier does not move. A producer of its token may stay between +// the groups while the later barrier folds back onto it. +TEST_F(MergeBarrierPassTest, EarlierTokenProducerBetweenStillMerges) { createSignal(0); createWait(0); - // Consumer of token 0 (reads LDS token 0). - createDSLoadInBlock(bb, arch, /*destReg=*/20, /*addrReg=*/30, /*memTokens=*/{0}); + createTensorLoadInBlock(bb, arch, /*s0=*/8, /*s1=*/16, /*memTokens=*/{0}); createSignal(1); createWait(1); runPasses(/*mergeThreshold=*/100000); - // No merge: both barrier groups remain intact. - EXPECT_EQ(countByMnemonic("s_barrier_signal"), 2); - EXPECT_EQ(countByMnemonic("s_barrier_wait"), 2); + EXPECT_EQ(countByMnemonic("s_barrier_signal"), 1); + EXPECT_EQ(countByMnemonic("s_barrier_wait"), 1); + EXPECT_EQ(countByMnemonic("tensor_load_to_lds"), 1); + EXPECT_EQ(tokensOf(firstBarrier("s_barrier_signal")), (std::vector{0, 1})); } // signal / filler / wait is NOT a legal group (must be adjacent signal+wait), @@ -345,7 +358,7 @@ TEST_F(MergeBarrierPassTest, LegalDifferentTokenGroupsMerge) { // An instruction between the groups that touches an *unrelated* token (not in // either group's set) does not constrain the merged barrier, so the merge still -// happens when the distance is below threshold. +// happens. TEST_F(MergeBarrierPassTest, UnrelatedTokenBetweenStillMerges) { createSignal(0); createWait(0); @@ -371,7 +384,7 @@ TEST_F(MergeBarrierPassTest, ChainedGroupsCollapse) { createSignal(2); createWait(2); - runPasses(/*mergeThreshold=*/0); // default threshold, all adjacent + runPasses(/*mergeThreshold=*/0); EXPECT_EQ(countByMnemonic("s_barrier_signal"), 1); EXPECT_EQ(countByMnemonic("s_barrier_wait"), 1); @@ -397,8 +410,3 @@ TEST_F(MergeBarrierPassTest, NonLoopBlockUntouched) { EXPECT_EQ(countByMnemonic("s_barrier_signal"), 2); EXPECT_EQ(countByMnemonic("s_barrier_wait"), 2); } - -// The compile-time default mirrors the documented value. -TEST(MergeBarrierConstants, KnownDefault) { - EXPECT_EQ(kCdna5MergeBarrierThreshold, 11); -} From 57bf1b34e6402fd7d57a1e1ab75990c2996557f0 Mon Sep 17 00:00:00 2001 From: hcman2 Date: Fri, 30 Oct 2026 08:20:32 +0000 Subject: [PATCH 5/6] Fix overlap rule in barrier --- .../src/transforms/asm/dag/AGENTS.md | 10 +- .../src/transforms/asm/dag/CDNA5.hpp | 109 ++++++++++++------ .../tests/unit/asm/DAGSchedulerPassTest.cpp | 57 +++++++++ 3 files changed, 139 insertions(+), 37 deletions(-) diff --git a/shared/stinkytofu/src/transforms/asm/dag/AGENTS.md b/shared/stinkytofu/src/transforms/asm/dag/AGENTS.md index 13a8530997ae..148531f75cbe 100644 --- a/shared/stinkytofu/src/transforms/asm/dag/AGENTS.md +++ b/shared/stinkytofu/src/transforms/asm/dag/AGENTS.md @@ -45,9 +45,13 @@ Consequences: - A lower level never vetoes a higher one. The ds pacer (L5) must not delay the last ds before a barrier (L2). A filler (L6) must not take a prefetch's window (L4). -- A barrier issues when its WMMA threshold is met. That threshold already includes the last - WMMA a feeding ds_load depends on and that load's drain. Do not hold the barrier for an - extra tail of WMMAs after the threshold. +- A barrier issues when its WMMA threshold is met. Do not hold it for an extra tail of + WMMAs after the threshold. The threshold includes the last WMMA a feeding ds_load depends + on and that load's drain (`depThreshold`). Two exclusive groups whose claim windows + overlap are the exception: that drain does not fit, so the after group stops at + `maxParentWmmaIdx + 1` and the separation that does not fit is added to the before + group's threshold. A barrier that is in both the after and before maps is still averaged + no earlier than `depThreshold`. ## Rules diff --git a/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp b/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp index 37630b81dd42..156a2a1dab82 100644 --- a/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp +++ b/shared/stinkytofu/src/transforms/asm/dag/CDNA5.hpp @@ -692,6 +692,11 @@ class CDNA5ReadyQueue : public ReadyQueue { // threshold may move earlier than that after value, but not earlier // than this dependency. int depThreshold = 0; + // Parent WMMA index after the WmmaVgprSrcToDsWrite gap. 0 means the + // ds_loads have no WMMA parent. Overlapping exclusive groups may pull + // the after threshold down to this index + 1; the full drain in + // depThreshold does not fit once the claim windows overlap. + int maxParentWmmaIdx = 0; }; int wmmaIssuedCountThisRegion_ = 0; @@ -1979,6 +1984,7 @@ CDNA5ReadyQueue::computeBarrierAfterThresholds(IRList::iterator regionStart, int latencyWmmaBudget; int dsLoadCount; int depThreshold; + int maxParentWmmaIdx; }; // Step 1a: collect all movable barriers with their PSEUDO src token sets, @@ -2126,11 +2132,12 @@ CDNA5ReadyQueue::computeBarrierAfterThresholds(IRList::iterator regionStart, for (StinkyInstruction* barrier : group.barriers) { barrierWmmaThresholds_[barrier] = afterThreshold; result[barrier] = {afterThreshold, afterThreshold, latencyWmmaBudget, - wmmaWindowsNeeded, matchingDsLoadCount, depThreshold}; + wmmaWindowsNeeded, matchingDsLoadCount, depThreshold, + maxParentWmmaIdx}; } overlapChecks.push_back({groupBarrier, group.barriers, afterThreshold, lastOverlap, wmmaWindowsNeeded, latencyWmmaBudget, matchingDsLoadCount, - depThreshold}); + depThreshold, maxParentWmmaIdx}); PASS_DEBUG(std::cerr << "[CDNA5 computeBarrierAfterThresholds] barrier=" << groupBarrier << " barrierGroupSize=" << group.barriers.size() << " afterThreshold=" << afterThreshold << " matchingDsLoadCount=" << matchingDsLoadCount @@ -2197,7 +2204,8 @@ CDNA5ReadyQueue::computeBarrierAfterThresholds(IRList::iterator regionStart, barrierWmmaThresholds_[barrier] = adjustedAfterThreshold; result[barrier] = {adjustedAfterThreshold, summary.afterThreshold, summary.latencyWmmaBudget, summary.wmmaWindowsNeeded, - summary.dsLoadCount, summary.depThreshold}; + summary.dsLoadCount, summary.depThreshold, + summary.maxParentWmmaIdx}; } PASS_DEBUG( std::cerr << "[CDNA5 computeBarrierAfterThresholds overlap] barrier=" @@ -3002,11 +3010,13 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato // Some signal/wait-like pairs are split: one member only lands in the // "after" model while its counterpart only lands in the "before" model. // For those cross-map-only pairs, compare their implied WMMA ranges. - // On overlap, leave before in place and pull after to before minus the - // separation slack when both issue demands plus that slack fit in the - // region and in front of the before threshold. Otherwise split - // totalWmma proportionally: pull after earlier (min) and push before - // later (max). + // On overlap, pull after toward before minus the separation slack when + // both issue demands plus that slack fit in the region and in front of + // the before threshold. Otherwise split totalWmma proportionally: pull + // after earlier (min) and push before later (max). Overlap means the + // full parent drain does not fit, so neither path may pull after + // earlier than maxParentWmmaIdx + 1. Separation that after cannot give + // up past that parent is added to before. struct BarrierGroupThresholdSummary { StinkyInstruction* anchor = nullptr; std::vector barriers; @@ -3020,9 +3030,13 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato // windows (no drain); before uses the same value as claimWindow. int splitNeeded = 0; // Layer-2 target across overlapping pairs. After keeps the earliest - // target (min). Before moves later only on the proportional-split - // path. + // target (min) that is still after its parent WMMA. Before moves + // later on the proportional-split path, and also when a gap cannot + // be opened by pulling after earlier than maxParentWmmaIdx + 1. int pendingThreshold = 0; + // Latest parent WMMA among the group's after-map barriers. 0 when + // none of those ds_loads has a WMMA parent. + int maxParentWmmaIdx = 0; // Descendants used when publishing hard orderings on overlap. std::vector descendantLoads; // A proportional placement keeps the signal/wait pair together for @@ -3047,6 +3061,7 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato int baseThresholdCount = 0; int maxClaimWindow = 0; int maxSplitNeeded = 0; + int maxParentWmmaIdx = 0; bool hasPrimary = false; bool hasCross = false; for (StinkyInstruction* barrier : group.barriers) { @@ -3065,6 +3080,7 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato pIt->second.latencyWmmaBudget)); maxSplitNeeded = std::max(maxSplitNeeded, std::max(0, pIt->second.wmmaWindowsNeeded)); + maxParentWmmaIdx = std::max(maxParentWmmaIdx, pIt->second.maxParentWmmaIdx); } if (beforeThresholds.find(barrier) != beforeThresholds.end()) hasCross = true; } @@ -3077,6 +3093,7 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato summary.baseThreshold = baseThresholdSum / baseThresholdCount; summary.claimWindow = maxClaimWindow; summary.splitNeeded = maxSplitNeeded; + summary.maxParentWmmaIdx = maxParentWmmaIdx; summary.pendingThreshold = summary.threshold; groups.push_back(std::move(summary)); } @@ -3210,9 +3227,11 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato // threshold would choose. // TODO: multiple after or before groups. Compute each pair from the // unmodified group threshold, then combine candidates after the loop. - // When the issue demands plus the separation slack fit, leave before - // in place and pull after to before minus that slack. Otherwise split - // the WMMA windows in proportion to each side's demand. + // When the issue demands plus the separation slack fit, pull after to + // before minus that slack. Otherwise split the WMMA windows in + // proportion to each side's demand. Overlap cannot fit the parent + // drain, so after cannot move earlier than maxParentWmmaIdx + 1. + // before moves later by whatever separation is left. // The two groups keep one window apart for the tensor load. const int separationSlack = 1; for (auto& afterGroup : exclusiveAfterGroups) { @@ -3310,32 +3329,53 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato const bool demandFits = totalNeeded + separationSlack < totalWmma && beforeGroup.pendingThreshold >= afterNeeded + separationSlack; + // Issue windows are a floor. A parent WMMA is the other floor: + // overlap does not leave room for the full drain in + // depThreshold, so after only has to stay one WMMA after + // maxParentWmmaIdx. No parent means that floor is absent. + const int issueFloor = std::clamp(afterGroup.splitNeeded, 0, totalWmma); + const int parentFloor = + afterGroup.maxParentWmmaIdx > 0 + ? std::clamp(afterGroup.maxParentWmmaIdx + 1, 0, totalWmma) + : 0; if (demandFits) { - // Room for both issue windows and the slack: keep before, - // and pull after earlier by the slack plus the configured + // Room for both issue windows and the slack: pull after + // earlier by the slack plus the configured // TensorLoadDsLoadGapCycles (default 64) worth of WMMA // windows so tensor loads stay off the ds_loads. placement = "gap"; - // Do not pull the after barrier earlier than the issue - // windows it needs (splitNeeded is wmmaWindowsNeeded, no - // drain). The configured gap (TensorLoadDsLoadGapCycles, - // default 64) is extra separation, not a reason to drop - // below that floor. - const int afterFloor = std::clamp(afterGroup.splitNeeded, 0, totalWmma); - const int afterAtGap = - std::clamp(beforeGroup.pendingThreshold - separationSlack - - tensorLoadDsLoadGapWmma, - afterFloor, totalWmma); - afterGroup.pendingThreshold = - std::max(afterFloor, std::min(afterGroup.pendingThreshold, afterAtGap)); + const int separation = separationSlack + tensorLoadDsLoadGapWmma; + const int afterAtGap = std::clamp(beforeGroup.pendingThreshold - separation, + issueFloor, totalWmma); + const int pulledByIssueFloor = + std::max(issueFloor, std::min(afterGroup.pendingThreshold, afterAtGap)); + const int pulledAfter = std::max(pulledByIssueFloor, parentFloor); + afterGroup.pendingThreshold = pulledAfter; + // after is already one WMMA after its parent, so it + // cannot open the rest of the gap. Push before later + // by that remainder. + if (pulledAfter > pulledByIssueFloor) { + const int beforeAtGap = + std::clamp(pulledAfter + separation, 0, totalWmma); + beforeGroup.pendingThreshold = + std::max(beforeGroup.pendingThreshold, beforeAtGap); + } } else { placement = "proportional"; afterGroup.sawProportionalPlacement = true; beforeGroup.sawProportionalPlacement = true; - afterGroup.pendingThreshold = - std::min(afterGroup.pendingThreshold, afterTarget); + const int pulledAfter = std::max( + parentFloor, std::min(afterGroup.pendingThreshold, afterTarget)); + afterGroup.pendingThreshold = pulledAfter; + // Keep the split's after/before spacing when the + // dependency stops after short of afterTarget. + const int splitSeparation = std::max(0, beforeTarget - afterTarget); + const int beforeGoal = + pulledAfter > afterTarget + ? std::clamp(pulledAfter + splitSeparation, 0, totalWmma) + : beforeTarget; beforeGroup.pendingThreshold = - std::max(beforeGroup.pendingThreshold, beforeTarget); + std::max(beforeGroup.pendingThreshold, beforeGoal); } } @@ -3354,10 +3394,11 @@ void CDNA5ReadyQueue::onInitRegion(IRList::iterator regionStart, IRList::iterato << " afterWmmaWindow=" << afterGroup.claimWindow << " beforeWmmaWindow=" << beforeGroup.claimWindow << " overlap=" << overlap << " baseAfterEnd=" << baseAfterEnd - << " baseBeforeBegin=" << baseBeforeBegin << " proportionalSplit=" - << proportionalSplit << " separationSlack=" << separationSlack - << " tensorLoadDsLoadGapWmma=" << tensorLoadDsLoadGapWmma - << " placement=" << placement + << " baseBeforeBegin=" << baseBeforeBegin + << " maxParentWmmaIdx=" << afterGroup.maxParentWmmaIdx + << " proportionalSplit=" << proportionalSplit + << " separationSlack=" << separationSlack << " tensorLoadDsLoadGapWmma=" + << tensorLoadDsLoadGapWmma << " placement=" << placement << " pendingAfterThreshold=" << afterGroup.pendingThreshold << " pendingBeforeThreshold=" << beforeGroup.pendingThreshold << "\n"); } diff --git a/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp b/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp index baefe838b03d..dcda2f0da56a 100644 --- a/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp +++ b/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp @@ -960,6 +960,63 @@ TEST_F(DAGSchedulerPassTest, BothSidedBarrierStopsAtTheParentDependencyNotTheOve EXPECT_EQ(wmmasBeforeBarrier, 18) << scheduleOrder(*bb); } +// Exclusive after/before windows overlap, so the full parent drain does not +// fit. Gap placement may pull the after threshold down to maxParentWmmaIdx + 1, +// and the 9-window separation (1 slack + ceil(64/8) tensor-load gap) that does +// not fit in front of that index is supplied by pushing before later. +// +// 33 WMMAs, 16 ds_loads, dsSpread off. The 16th WMMA reads the token-0 ds dest, +// so the overlap floor is 17. depThreshold would be 16 + 56/8 + 1 = 24, and one +// token-0 ds_load needs only 3 issue windows. Eight token-1 ds_loads put the +// before window across the after window, and demandFits selects gap. before is +// pushed to min(33, 17 + 9) = 26. +TEST_F(DAGSchedulerPassTest, OverlapGapStopsAfterAtParentWmmaAndPushesBefore) { + bb->addSuccessor(bb); + + StinkyInstruction* parentWmma = nullptr; + for (int i = 0; i < 16; ++i) { + const int src = (i == 15) ? 0 : 3000 + i * 16; + StinkyInstruction* wmma = + createWmmaF32_16x16x16_bf16(/*destStart=*/1100 + i * 16, /*src0Start=*/src); + if (i == 15) parentWmma = wmma; + } + StinkyInstruction* preBarrierDs = + createMovableDsLoad(/*destReg=*/0, /*addrReg=*/810, /*ldsToken=*/0); + createUnrelatedDsLoads(7); + auto [afterSignal, afterWait] = createMovableWorkgroupBarrier(bb, /*ldsToken=*/0); + auto [beforeSignal, beforeWait] = createMovableWorkgroupBarrier(bb, /*ldsToken=*/1); + (void)afterWait; + (void)beforeWait; + createMovableDsLoad(/*destReg=*/500, /*addrReg=*/820, /*ldsToken=*/1); + for (int i = 1; i < 8; ++i) + createMovableDsLoad(/*destReg=*/8000 + i * 4, /*addrReg=*/830 + i, /*ldsToken=*/1); + createWmmaF32_16x16x16_bf16(/*destStart=*/800, /*src0Start=*/500); + for (int i = 0; i < 16; ++i) + createWmmaF32_16x16x16_bf16(/*destStart=*/4000 + i * 16, /*src0Start=*/5000 + i * 16); + createSCmpWritingScc(bb, /*srcSgpr=*/90); + + runPassWithUnrollGemm(/*dsSpread=*/false); + + auto wmmasBefore = [&](const StinkyInstruction* target) { + int count = 0; + for (const IRBase& ir : *bb) { + const auto* inst = dyn_cast(&ir); + if (inst == nullptr) continue; + if (inst == target) break; + if (isMatrixInstruction(*inst)) ++count; + } + return count; + }; + + ASSERT_NE(parentWmma, nullptr); + EXPECT_LT(positionOf(*bb, parentWmma), positionOf(*bb, preBarrierDs)) << scheduleOrder(*bb); + EXPECT_LT(positionOf(*bb, afterSignal), positionOf(*bb, beforeSignal)) << scheduleOrder(*bb); + EXPECT_GE(wmmasBefore(afterSignal), 17) << scheduleOrder(*bb); + EXPECT_LT(wmmasBefore(afterSignal), 24) << scheduleOrder(*bb); + EXPECT_GE(wmmasBefore(beforeSignal), 26) << scheduleOrder(*bb); + EXPECT_LT(wmmasBefore(beforeSignal), 33) << scheduleOrder(*bb); +} + TEST_F(DAGSchedulerPassTest, Layer2DoesNotPublishWithoutBeforeGroup) { bb->addSuccessor(bb); From 042115cbfe62511a2b6162deffe001e7e3f81841 Mon Sep 17 00:00:00 2001 From: hcman2 Date: Fri, 30 Oct 2026 09:47:38 +0000 Subject: [PATCH 6/6] Fix test case bug. --- .../filecheck/dag_wmma_barrier_cover.stir | 20 +++++++++---------- .../tests/unit/asm/DAGSchedulerPassTest.cpp | 10 +++++++--- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/shared/stinkytofu/tests/filecheck/dag_wmma_barrier_cover.stir b/shared/stinkytofu/tests/filecheck/dag_wmma_barrier_cover.stir index 1041035eaf4b..a97c06a6f9e2 100644 --- a/shared/stinkytofu/tests/filecheck/dag_wmma_barrier_cover.stir +++ b/shared/stinkytofu/tests/filecheck/dag_wmma_barrier_cover.stir @@ -1,18 +1,18 @@ # RUN: %stinkytofu-opt --arch gfx1250 %s --StinkyBuildImplicitDependencyPass --StinkyDAGSchedulerPass --wmma-queue-depth=8 --wmma-queue-cover-cycles=16 --wmma-batch-size=4 --ds-spread --debug-pass StinkyDAGSchedulerPass --print-output 2>&1 # -# A ready barrier waits for the WMMA queue to cover its stall (WmmaBarrierCoverCycles, automatic: -# a tail of 4 WMMAs, 32 cycles) instead of issuing the moment its threshold is met, so the queued -# WMMAs hide the barrier stall: +# A barrier issues as soon as its threshold is met and its ds_stores are done. It does not wait +# for an extra tail of WMMAs; the WMMAs already queued hide the barrier stall: # -# wmma wmma ds ds wmma wmma wmma barrier +# wmma wmma ds_store wmma ds_store barrier # -# 24 independent WMMAs are available and the barrier threshold is 18. The barrier issues after 22 -# WMMAs with 37 cycles queued, the cover met. The wait half of the pair follows its signal at once -# and the model takes the WMMA queue as empty after it (queuedCover=0). The 25th WMMA reads the -# post-barrier ds_loads, so it cannot be used for cover. +# 24 independent WMMAs are available and the barrier threshold is 0, so the last ds_store sets +# when it can issue. That ds_store waits on its buffer_load and lands after 21 WMMAs, and the +# barrier follows right away with 30 cycles still queued. The wait half of the pair follows its +# signal at once and the model takes the WMMA queue as empty after it (queuedCover=0). The 25th +# WMMA reads the post-barrier ds_loads, so it cannot be used for cover. # -# CHECK: [CDNA5 barrierCover] wmmaIssued=22 queuedCover=37 -# CHECK: [CDNA5 barrierCover] wmmaIssued=22 queuedCover=0 +# CHECK: [CDNA5 barrierCover] wmmaIssued=21 queuedCover=30 +# CHECK: [CDNA5 barrierCover] wmmaIssued=21 queuedCover=0 # # The barrier pair still precedes the ds_loads that read the data it protects. # CHECK-LABEL: @barrier_cover diff --git a/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp b/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp index dcda2f0da56a..01d25050c6c1 100644 --- a/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp +++ b/shared/stinkytofu/tests/unit/asm/DAGSchedulerPassTest.cpp @@ -786,8 +786,9 @@ TEST_F(DAGSchedulerPassTest, Layer2DoesNotPublishWhenPerWmmaBudgetsSeparateWindo // Non-overlapping after/before groups still keep the after token's next // tensor_load between them when the after threshold is earlier. The load // defines that token, so it is not a register user of the barrier. The before -// barrier starts ahead of the load, so only the new hard edges can move it -// behind the load. +// barrier starts ahead of the load, so only the hard edges can move it behind +// the load. Unrelated ds_loads shrink each claim window to one WMMA, so the +// pair is not a Layer 2 merge candidate. TEST_F(DAGSchedulerPassTest, NonOverlappingBarrierPairKeepsTensorLoadBetweenGroups) { bb->addSuccessor(bb); @@ -801,6 +802,9 @@ TEST_F(DAGSchedulerPassTest, NonOverlappingBarrierPairKeepsTensorLoadBetweenGrou createMovableTensorLoad(bb, /*src0Reg=*/220, /*src1Reg=*/224, /*ldsToken=*/0); createMovableDsLoad(/*destReg=*/500, /*addrReg=*/204, /*ldsToken=*/1); createWmmaF32_16x16x16_bf16(/*destStart=*/800, /*src0Start=*/500); + // 24 ds_loads over 24 WMMAs: one window per ds_load, so the after claim + // (window + drain) ends before the before group starts. + createUnrelatedDsLoads(22); runPassWithUnrollGemm(); @@ -826,7 +830,7 @@ TEST_F(DAGSchedulerPassTest, NonOverlappingBarrierPairKeepsTensorLoadBetweenGrou const auto* overlaps = am.getCachedResult(); ASSERT_NE(overlaps, nullptr); - EXPECT_FALSE(overlaps->contains(afterSignal, beforeSignal)); + EXPECT_FALSE(overlaps->contains(afterSignal, beforeSignal)) << scheduleOrder(*bb); } // The latest matching ds_load does not overlap any WMMA, so lastOverlap is 0.