Skip to content

Gate async-copy matmul strategies on real stride alignment - #5370

Open
RAV64 wants to merge 1 commit into
tracel-ai:mainfrom
RAV64:fix/gate-async-copy-matmul-strategies
Open

Gate async-copy matmul strategies on real stride alignment#5370
RAV64 wants to merge 1 commit into
tracel-ai:mainfrom
RAV64:fix/gate-async-copy-matmul-strategies

Conversation

@RAV64

@RAV64 RAV64 commented Aug 15, 2026

Copy link
Copy Markdown

Fixes the kernel-selection half of #5352.

Problem

Strategy::SpecializedCyclicCmma / SpecializedCyclicMma bind L = AsyncPartialCyclicLoading in SpecializedAlgorithm<L, AL>, and that single L serves both operands' partial loads. AsyncPartialCyclicLoading's validation calls validate_async_copy_with_problem, which checks the real strides for 16-byte alignment when the kernel is set up.

The matmul tuner already has a gate for exactly this: the tma group returns PRIORITY_NEVER unless lhs_stride_factor and rhs_stride_factor both clear the threshold. But that group is only attached to the *Tma* strategies. The two specialized cyclic strategies are registered in accelerated with no extra group, and every other entry in that list reads through sync loaders (Simple*, DoubleCyclic*CyclicDoubleBufferingAlgorithm, OrderedDouble*), so nothing else there needs the check.

The result is that an under-aligned problem can hand PRIORITY_MAX to a kernel that cannot launch on it. Selection commits to the winner, setup validation then rejects it, and the error surfaces where it can no longer be handled — .expect("Should run when selected by autotune.") on a cache hit in LocalTuner::execute. On an async device runner that panic is caught and warn-logged while the op's registered outputs stay unwritten, which is how it reaches training as NaN weights or a corrupted fusion stream (#5352 has the full chain and a ~60-line reproduction).

A Linear(126, 7) layer is enough to hit it: the head matmul [512,128] @ [128,7] has rhs strides [7,1], a 28-byte canonical stride.

Fix

Extend the existing threshold to the two async-copy strategies, evaluated from the key like every other priority decision — no fallback, no re-running, and no cloning of real autotune inputs. Under-aligned problems keep the entire sync accelerated pool plus the naive fallback, so the tune plan can never end up empty.

The check is lhs < T || rhs < T rather than per-operand because both operands load through the same L.

Also replaces the two bare 4s in the tma group with the named ASYNC_COPY_STRIDE_FACTOR so the threshold has one definition.

Depends on tracel-ai/cubek#488

This gate is inert without that PR. MatmulAutotuneKey currently derives its stride factors from the anchored dims, so a 7-wide dim (anchor(7) = 8) reports as 32-byte aligned and a 126-wide dim (anchor(126) = 128) as 512-byte aligned. Every under-aligned problem therefore clears any threshold read off the key. cubek#488 makes those fields reflect the real strides; this PR is what consumes them. They need to land together — merging either alone is a no-op for the bug.

That dependency also explains an observation in #5352 that looked contradictory: fixing the key alone did not stop the crash, because splitting the buckets does not help when nothing gates these strategies on alignment in the first place.

Testing

cargo check -p burn-cubecl --features cuda, plus verification on a real CUDA training workload (RTX 4080 SUPER, sm_89) running both fixes backported onto the 0.22.0-pre.2 releases:

  • The gate fires on 14 of 20 matmul autotune keys in that workload — the two strategies are recorded with no benchmark result on those keys and tune normally on the remaining 6, so aligned shapes still see the full candidate pool.
  • The training numerics suites pass on both the cuda and cuda-fusion configurations, where previously the fused one died on the step after the first optimizer update.

Still open from #5352

A task that panics on the device-runner thread is caught and only log::warn!-ed while its outputs stay registered and unwritten. This PR removes the trigger we hit, but any other panic on that path still corrupts state silently; that is worth addressing separately.

The specialized cyclic strategies read through AsyncPartialCyclicLoading,
whose setup validates the real strides for 16-byte alignment, but they are
registered in the `accelerated` group only. The `tma` group already gates
its strategies on that threshold; nothing gated these, so an under-aligned
problem could hand them the maximum priority and select a kernel that
cannot launch on it. The failure then surfaces after autotune has committed
to the winner, where it is no longer recoverable.

Both operands load through the same loader, so either one falling under the
threshold rules the strategy out. Under-aligned problems keep the sync
accelerated kernels and the naive fallback, so the tune plan stays non-empty.

The threshold now has a single named definition shared with the tma group.

Note this depends on the autotune key reporting real stride alignment
(tracel-ai/cubek#488); until then the key derives those factors from the
anchored dims and no threshold read from it can fire.

@nathanielsimard nathanielsimard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though those factors are handled in the kernel selection process @wingertge should they be in the tuner?

@laggui laggui left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think the stride constraint should remain handled by cubek's kernel selection/setup.

I believe the correct fix would be to ensure the autotune cache key distinguishes inputs that pass the kernel's stride-alignment validation from those that do not, so an invalid input can never reuse a kernel selected for a valid one 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants