Skip to content

[SM6.10] LinAlg Validation: MatrixLength #8787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Ashley Coleman (V-FEXrt) merged 1 commit into from
Aug 17, 2026
+18 −0
Merged

[SM6.10] LinAlg Validation: MatrixLength #8787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions lib/DxilValidation/DxilValidation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,17 @@ static void ValidateLinAlgOpReturnMatrix(CallInst *CI,
static void ValidateLinAlgMatrixLength(CallInst *CI,
ValidationContext &ValCtx) {
ValidateLinAlgOpParameters(CI, ValCtx);
DxilInst_LinAlgMatrixLength Op(CI);
std::optional<LinAlgTargetType> Mat =
GetCheckedLATT(Op.get_matrix()->getType(), ValCtx);
if (!Mat)
return;
Comment on lines 1126 to +1131

@V-FEXrt Ashley Coleman (V-FEXrt) Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is already ensured by the type system and an extremely common pattern. Generally we should be able to assume well-formed IR from the frontend.

@damyanp Damyan Pepper (damyanp) Aug 17, 2026

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'm not sure I agree with "we should be able to assume well-formed IR from the frontend." - the whole point of the validator is to ensure that code from other frontends is valid.

At the same time, I don't think it matters so much about hitting an assert in debug builds, as long as a non-assert enabled build rejects the invalid dxil.

@V-FEXrt Ashley Coleman (V-FEXrt) Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Well-formed and valid aren't the same thing :)

A proper frontend will not generate IR that calls a function with the wrong types for example. If we can't assume that the frontend generated a call to foo(i32, float) with parameters of i32, float then pretty much all bets are off and the backend needs to have essentially another copy of the frontend in it. (or at least the type checker)

Separately, if we say it's only valid to call foo with even integers then that's where we can't necessarily trust the frontend to not generate IR with odd integers so we validate.

In this specific case, we only have overloads for linAlgMatrixLength with a matrix in param slot 1, if the frontend ever generates anything different then the type system is very broken and the IR is not well-formed

@damyanp Damyan Pepper (damyanp) Aug 17, 2026

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.

Isn't well-formedness validated elsewhere already before we get to this point?

Anyway, to be clear, I don't think this impacts this PR, but alarm bells go off if I hear anything suggesting that the validator should lean on the frontend, because the frontend isn't always there and is not something that's in the control of the validator.

@V-FEXrt Ashley Coleman (V-FEXrt) Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

LLVM IR well-formedness is already validated elsewhere yeah but DXIL is a bit weird in this regard (in fact DXIL makes a lot of this weird since other folks generate it directly and its not really an IR anymore).

As long as the function is declared correctly then we should expect a type error to already be generated if the wrong type is passed in, but I think copilot is commenting here because nothing stops someone from declaring an illegal overload of matrixlength that doesn't have a matrix as arg 1. That would be well-formed LLVM IR that passes the IR checker but not well-formed DXIL since its an illegal overload.

Out of curiosity I'll hand build both of those rq and report back.

I suppose the question we need to answer more generally is: what is our responsibility to users who forgo the safety checks of the frontend and generate DXIL directly? Should we gracefully fail for all generatable DXIL thats well-formed enough to pass llvmir checks yet not well-formed DXIL? There is also certainly a gray area here on whether we should call illegal type overload for DXIL operations as "invalid" or "not well-formed" and I suppose I'm slowly talking myself into the first

aside: I do have to admit I found myself a bit snippy with copilot for calling out a pretty mundane thing as a "HIGH" priority issue after having seen the pattern at least 10 other times without saying anything about it. It reads a bit as trying to invent a comment when it had nothing else to say

@V-FEXrt Ashley Coleman (V-FEXrt) Aug 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Out of curiosity I'll hand build both of those rq and report back.

Calling with correct declaration but wrong parameter type errors as expected

shader: invalid forward reference to function 'dx.op.linAlgMatrixLength.mC8M4N4U2S2' with wrong type!
  %2 = call i32 @dx.op.linAlgMatrixLength.mC8M4N4U2S2(i32 -2147483632, i32 1)  ; LinAlgMatrixLength(matrix)

Declaring a wrong overload for the operation (but calling it correctly) errors with an invalid overload error

Function: main: error: 'dx.op.linAlgMatrixLength.mC8M4N4U2S2' is not a DXILOpFuncition for DXILOpcode 'LinAlgMatrixLength'.
note: at '%2 = call i32 @dx.op.linAlgMatrixLength.mC8M4N4U2S2(i32 -2147483632, i32 1)' in block '#0' of function 'main'.
Function: main: error: DXIL intrinsic overload must be valid.
note: at '%2 = call i32 @dx.op.linAlgMatrixLength.mC8M4N4U2S2(i32 -2147483632, i32 1)' in block '#0' of function 'main'.
Function: dx.op.linAlgMatrixLength.i32: error: External function 'dx.op.linAlgMatrixLength.i32' is unused.

So we properly catch both cases already and copilots comment was actually just superfluous. That said, my reason for rejecting the comment was also incorrect so I guess its a learning moment:)


if (Mat->Scope != DXIL::MatrixScope::Wave &&
Mat->Scope != DXIL::MatrixScope::ThreadGroup)
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrLinAlgMatrixScopeMismatch2,
{"Input", MatrixScopeToString(Mat->Scope), "Wave", "ThreadGroup"});
}

static void ValidateLinAlgMatrixGetCoordinate(CallInst *CI,
Expand Down
7 changes: 7 additions & 0 deletions tools/clang/test/LitDXILValidation/LinAlgMatrix/linalgmatrix-non-thread-ops.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ define void @main() {
; CHECK-NEXT: note: at {{.*}} @dx.op.linAlgMatrixGetCoordinate.mC8M4N4U2S0
%4 = call <2 x i32> @dx.op.linAlgMatrixGetCoordinate.mC8M4N4U2S0(i32 -2147483631, %dx.types.LinAlgMatrixC8M4N4U2S0 %3, i32 0) ; LinAlgMatrixGetCoordinate(matrix,threadLocalIndex)

; CHECK-NEXT: Function: main: error: Input matrix scope 'Thread' does not match expected scope Wave or ThreadGroup.
; CHECK-NEXT: note: at {{.*}} @dx.op.linAlgMatrixLength.mC8M4N4U2S0
%5 = call i32 @dx.op.linAlgMatrixLength.mC8M4N4U2S0(i32 -2147483632, %dx.types.LinAlgMatrixC8M4N4U2S0 %3) ; LinAlgMatrixLength(matrix)

; CHECK-NEXT: Validation failed.
ret void
}
Expand All @@ -41,6 +45,9 @@ declare %dx.types.LinAlgMatrixC8M4N4U2S0 @dx.op.linAlgMatrixSetElement.mC8M4N4U2
; Function Attrs: nounwind
declare <2 x i32> @dx.op.linAlgMatrixGetCoordinate.mC8M4N4U2S0(i32, %dx.types.LinAlgMatrixC8M4N4U2S0, i32) #0

; Function Attrs: nounwind
declare i32 @dx.op.linAlgMatrixLength.mC8M4N4U2S0(i32, %dx.types.LinAlgMatrixC8M4N4U2S0) #0

attributes #0 = { nounwind }

!dx.targetTypes = !{!0}
Expand Down
Loading