[µTLX] fix h100 persistent matmul - #63
Open
plotfi wants to merge 1 commit into
Open
Conversation
(cherry picked from commit 2df86e3)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This makes TLX H100 persistent matmul for µTLX, but there was quite a lot of vibe coding involved so this must be audited somewhat.
This makes the following example notebook work:
https://colab.research.google.com/drive/1zANW7SP8dG9I7SXvWkH_9pYFRZYPAu_y?usp=sharing
Claude Summary of what it did:
Legality Check Fixes for ConvertTritonToTritonGPU
Problem
The uTLX plugin creates TTNG ops (
WarpGroupDotOp,WarpGroupDotWaitOp) and TLX ops (RequireLayoutOp,ReleaseLayoutOp) during the builder phase with unencoded tensor types. TheConvertTritonToTritonGPUpass must convert these to use encoded types (e.g.,#blocked,#nvidia_mma), but several issues prevented this.Fix 1: Custom patterns for TTNG ops with
InferTypeOpInterfaceuTLXConversionPatterns.cpplines 128-158 —WarpGroupDotPatternandWarpGroupDotWaitPatternGenericOpPatterncallsreplaceOpWithNewOp<Op>(op, retTypes, adaptor.getOperands(), op->getAttrs()), passing explicit result types. ButWarpGroupDotOphasTypesMatchWithandWarpGroupDotWaitOphasAllTypesMatch— both implementInferTypeOpInterface, meaning result types are inferred from operands, not accepted as parameters. The builder ignores the explicit types, so the result stays unencoded.Fix: Custom patterns that call the op's named-parameter constructor (no explicit result types), letting type inference derive encoded results from converted operands:
WarpGroupDotPattern: passesadaptor.getA(), adaptor.getB(), adaptor.getC(), adaptor.getUseC()+ attribute argsWarpGroupDotWaitPattern: passesadaptor.getInputs(), op.getPendings()Fix 2: Dynamic legality for TTNG ops
uTLXConversionPatterns.cpplines 966-972The TTNG dialect is blanket-legal (
addLegalDialect<TritonNvidiaGPUDialect>()), so the conversion framework skips TTNG ops even when they have unencoded tensors.Fix: Override with
addDynamicallyLegalOpforWarpGroupDotOpandWarpGroupDotWaitOpusingallTensorsEncoded— these ops are only legal when all their tensor operands/results have encodings.Fix 3: RequireLayoutOp/ReleaseLayoutOp to
ttg.convert_layoutuTLXConversionPatterns.cpplines 86-122 —RequireLayoutToConvertPatternandReleaseLayoutToConvertPatternThese TLX ops change tensor encodings (e.g.,
#blockedto#nvidia_mma). They sit betweenWarpGroupDotOpandWarpGroupDotWaitOpin the SSA chain, creating unencoded/encoded boundaries that cause source materializations.Using
GenericOpPatternwould create newRequireLayoutOp/ReleaseLayoutOpinstances (which are also illegal, causing an infinite conversion loop). Instead, these patterns replace them withttg.convert_layout, which is legal in the TritonGPU dialect.Fix 4: Dynamic legality for TLX ops
uTLXConversionPatterns.cpplines 976-986TLX RequireLayout/ReleaseLayout ops that operate on
MemDescType(notRankedTensorType) should pass through unchanged. Only tensor-typed instances need conversion.Fix:
addDynamicallyLegalOpreturnsfalse(illegal) when any operand or result isRankedTensorType,trueotherwise.Fix 5: Post-conversion cleanup walk
uTLXConversionPatterns.cpplines 1040-1060If any tensor-typed
RequireLayoutOp/ReleaseLayoutOpsurviveapplyPartialConversion(e.g., because the TLXPropagateLayoutpass is not in the pipeline), they would reach LLVM translation and causeunrealized_conversion_castfailures.Fix: A post-conversion
mod.walklowers surviving tensor-typed instances tottg.convert_layoutas a safety net.Fix 6:
ensureModuleAttrsinNewOps.cppNewOps.cpplines 17-38 —ensureModuleAttrs()helperNvidiaMmaEncodinglayout verifiers requirettg.num-warps,ttg.threads-per-warp, andttg.num-ctasmodule attributes. These weren't being set early enough during the builder phase.Fix:
ensureModuleAttrs()eagerly sets these attributes on the parentModuleOpwhencreateRequireNvMmaLayoutis called.Pattern registration
uTLXConversionPatterns.cpplines 1020-1023 — All four custom patterns are registered:patterns.add<RequireLayoutToConvertPattern, ReleaseLayoutToConvertPattern, WarpGroupDotPattern, WarpGroupDotWaitPattern>(typeConverter, context);Remaining issue
The
failed to legalize unresolved materializationerror may still persist fortlx.require_layoutinside SCF for-loop bodies. The MLIR conversion framework inserts source materializations when it converts SCF block args from unencoded to#blocked, but theRequireLayoutOpthat uses those block args may not get its pattern applied at the right time. This is a framework-level timing issue withapplyPartialConversion.