Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions docs/DXIL.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3211,6 +3211,9 @@ INSTR.IMMBIASFORSAMPLEB bias amount for sample_b m
INSTR.INBOUNDSACCESS Access to out-of-bounds memory is disallowed.
INSTR.LINALGILLEGALCOMPONENTTYPE Matrix Component Type '%0' not allowed in LinAlg Matrix.
INSTR.LINALGILLEGALKDIM Matrix K Dimension out of bounds. K=%0 must be >= %1 and <= %2.
INSTR.LINALGMATRIXDIMMISMATCH Matrix Dimension '%0x%1' does not match expected dimension %2x%3.
INSTR.LINALGMATRIXSCOPEMISMATCH Matrix Scope '%0' does not match expected scope %1.
INSTR.LINALGMATRIXSCOPENOTALLOWED Matrix Scope '%0' not allowed in %1 operation.
INSTR.MAYREORDERTHREADUNDEFCOHERENCEHINTPARAM Use of undef coherence hint or num coherence hint bits in MaybeReorderThread.
INSTR.MINPRECISIONNOTPRECISE Instructions marked precise may not refer to minprecision values.
INSTR.MINPRECISONBITCAST Bitcast on minprecison types is not allowed.
Expand Down
59 changes: 58 additions & 1 deletion lib/DxilValidation/DxilValidation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2269,7 +2269,6 @@ static void ValidateDxilOperationCallInProfile(CallInst *CI,
break;
}
case DXIL::OpCode::LinAlgFillMatrix:
case DXIL::OpCode::LinAlgCopyConvertMatrix:
case DXIL::OpCode::LinAlgMatrixLoadFromDescriptor:
case DXIL::OpCode::LinAlgMatrixLoadFromMemory:
case DXIL::OpCode::LinAlgMatrixSetElement:
Expand All @@ -2281,6 +2280,64 @@ static void ValidateDxilOperationCallInProfile(CallInst *CI,
ValidateLinAlgOpParameters(CI, ValCtx);
break;
}
case DXIL::OpCode::LinAlgCopyConvertMatrix: {
ValidateLinAlgOpReturnMatrix(CI, ValCtx);
ValidateLinAlgOpParameters(CI, ValCtx);

Type *DstMatTy = CI->getType();
Type *SrcMatTy = CI->getArgOperand(1)->getType();
assert(dxilutil::IsHLSLLinAlgMatrixType(DstMatTy) &&
dxilutil::IsHLSLLinAlgMatrixType(SrcMatTy) &&
"Must be LinAlg types");

Value *TransposeOp = CI->getArgOperand(2);
ConstantInt *TransposeCI = dyn_cast<ConstantInt>(TransposeOp);
bool Transpose = false;

if (TransposeCI)
Transpose = TransposeCI->isOne();
else
ValCtx.EmitInstrFormatError(CI, ValidationRule::InstrOpConst,
{"Transpose", "LinAlgCopyConvertMatrix"});

auto DstIt = ValCtx.LinAlgTargetTypeMap.find(DstMatTy);
auto SrcIt = ValCtx.LinAlgTargetTypeMap.find(SrcMatTy);
if (DstIt == ValCtx.LinAlgTargetTypeMap.end())
break;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Am I missing something here, I would have thought these should be an error condition. But the break seems to imply we do nothing?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My friend copilot pointed out that they are already validated via the ValidateLinAlg* calls at the start of the switch case. So maybe these should be asserts instead?

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.

They are validated via those calls! We don't want to assert here though as the validation raises validation errors that should be pretty printed and all that jazz. Instead we just stop processing knowing that an error has already been raised and will be reported to the user (along with any other errors seen along the way)

if (SrcIt == ValCtx.LinAlgTargetTypeMap.end())
break;
LinAlgTargetType DstLATT = DstIt->second;
LinAlgTargetType SrcLATT = SrcIt->second;

if (DstLATT.Scope == DXIL::MatrixScope::Thread ||
SrcLATT.Scope == DXIL::MatrixScope::Thread)
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrLinAlgMatrixScopeNotAllowed,
{"Thread", "LinAlgCopyConvertMatrix"});

if (DstLATT.Scope != SrcLATT.Scope)
ValCtx.EmitInstrFormatError(
CI, ValidationRule::InstrLinAlgMatrixScopeMismatch,
{MatrixScopeToString(DstLATT.Scope),
MatrixScopeToString(SrcLATT.Scope)});

unsigned DstM = DstLATT.M;
unsigned DstN = DstLATT.N;
unsigned SrcM = SrcLATT.M;
unsigned SrcN = SrcLATT.N;
if (Transpose) {
SrcM = SrcLATT.N;
SrcN = SrcLATT.M;
}

if (DstM != SrcM || DstN != SrcN)
ValCtx.EmitInstrFormatError(CI,
ValidationRule::InstrLinAlgMatrixDimMismatch,
{std::to_string(DstM), std::to_string(DstN),
std::to_string(SrcM), std::to_string(SrcN)});

break;
}

default:
// TODO: make sure every Opcode is checked.
Expand Down
14 changes: 14 additions & 0 deletions lib/DxilValidation/DxilValidationUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <limits>
#include <optional>

#include "dxc/DXIL/DxilConstants.h"
#include "dxc/DXIL/DxilEntryProps.h"
#include "dxc/DXIL/DxilInstructions.h"
#include "dxc/DXIL/DxilModule.h"
Expand Down Expand Up @@ -669,4 +670,17 @@ llvm::StringRef ComponentTypeToString(DXIL::ComponentType CT) {
}
}

llvm::StringRef MatrixScopeToString(DXIL::MatrixScope MS) {
switch (MS) {
case DXIL::MatrixScope::Thread:
return "Thread";
case DXIL::MatrixScope::Wave:
return "Wave";
case DXIL::MatrixScope::ThreadGroup:
return "ThreadGroup";
default:
return "Unknown MatrixScope";
}
}

} // namespace hlsl
2 changes: 2 additions & 0 deletions lib/DxilValidation/DxilValidationUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,4 +157,6 @@ struct ValidationContext {
uint32_t ValidateDxilModule(llvm::Module *pModule, llvm::Module *pDebugModule);

llvm::StringRef ComponentTypeToString(DXIL::ComponentType CT);

llvm::StringRef MatrixScopeToString(DXIL::MatrixScope MS);
} // namespace hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
; REQUIRES: dxil-1-10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we plan on adding passing test cases for this later? I noticed that some test files have a filename-passing.ll variant. Looks like all of the existing linalg tests right now are just failing cases?

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.

We already have passing tests! They are under tools/clang/test/CodeGenDXIL/hlsl/linalg/builtins/<builtin name>/nominal.hlsl

My understanding is that LitDxilValidation is for testing things that should raise validation failures

; RUN: not %dxv %s 2>&1 | FileCheck %s
target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
target triple = "dxil-ms-dx"

%dx.types.LinAlgMatrixC2M5N4U1S2 = type { i8* }
%dx.types.LinAlgMatrixC4M8N4U1S2 = type { i8* }
%dx.types.LinAlgMatrixC4M5N8U1S2 = type { i8* }
%dx.types.LinAlgMatrixC4M5N4U1S2 = type { i8* }
%dx.types.LinAlgMatrixC2M4N5U1S1 = type { i8* }
%dx.types.LinAlgMatrixC2M4N5U1S0 = type { i8* }
define void @main() {
%1 = call %dx.types.LinAlgMatrixC2M5N4U1S2 @dx.op.linAlgFillMatrix.mC2M5N4U1S2.i32(i32 -2147483636, i32 1) ; LinAlgFillMatrix(value)

; CHECK: Function: main: error: Matrix Dimension '8x4' does not match expected dimension 5x4.
; CHECK-NEXT: note: at {{.*}} @dx.op.linAlgCopyConvertMatrix.mC4M8N4U1S2.mC2M5N4U1S2
%2 = call %dx.types.LinAlgMatrixC4M8N4U1S2 @dx.op.linAlgCopyConvertMatrix.mC4M8N4U1S2.mC2M5N4U1S2(i32 -2147483635, %dx.types.LinAlgMatrixC2M5N4U1S2 %1, i1 false) ; LinAlgCopyConvertMatrix(srcMatrix,transpose)

; CHECK-NEXT: Function: main: error: Matrix Dimension '5x8' does not match expected dimension 5x4.
; CHECK-NEXT: note: at {{.*}} @dx.op.linAlgCopyConvertMatrix.mC4M5N8U1S2.mC2M5N4U1S2
%3 = call %dx.types.LinAlgMatrixC4M5N8U1S2 @dx.op.linAlgCopyConvertMatrix.mC4M5N8U1S2.mC2M5N4U1S2(i32 -2147483635, %dx.types.LinAlgMatrixC2M5N4U1S2 %1, i1 false) ; LinAlgCopyConvertMatrix(srcMatrix,transpose)

; CHECK-NEXT: Function: main: error: Matrix Dimension '5x4' does not match expected dimension 4x5.
; CHECK-NEXT: note: at {{.*}} @dx.op.linAlgCopyConvertMatrix.mC4M5N4U1S2.mC2M5N4U1S2
%4 = call %dx.types.LinAlgMatrixC4M5N4U1S2 @dx.op.linAlgCopyConvertMatrix.mC4M5N4U1S2.mC2M5N4U1S2(i32 -2147483635, %dx.types.LinAlgMatrixC2M5N4U1S2 %1, i1 true) ; LinAlgCopyConvertMatrix(srcMatrix,transpose)

; CHECK-NEXT: Function: main: error: Matrix Scope 'Wave' does not match expected scope ThreadGroup.
; CHECK-NEXT: note: at {{.*}} @dx.op.linAlgCopyConvertMatrix.mC2M4N5U1S1.mC2M5N4U1S2
%5 = call %dx.types.LinAlgMatrixC2M4N5U1S1 @dx.op.linAlgCopyConvertMatrix.mC2M4N5U1S1.mC2M5N4U1S2(i32 -2147483635, %dx.types.LinAlgMatrixC2M5N4U1S2 %1, i1 true) ; LinAlgCopyConvertMatrix(srcMatrix,transpose)

%6 = call %dx.types.LinAlgMatrixC2M4N5U1S0 @dx.op.linAlgFillMatrix.mC2M4N5U1S0.i32(i32 -2147483636, i32 1) ; LinAlgFillMatrix(value)

; CHECK-NEXT: Function: main: error: Matrix Scope 'Thread' not allowed in LinAlgCopyConvertMatrix operation.
; CHECK-NEXT: note: at {{.*}} @dx.op.linAlgCopyConvertMatrix.mC2M4N5U1S0.mC2M4N5U1S0
%7 = call %dx.types.LinAlgMatrixC2M4N5U1S0 @dx.op.linAlgCopyConvertMatrix.mC2M4N5U1S0.mC2M4N5U1S0(i32 -2147483635, %dx.types.LinAlgMatrixC2M4N5U1S0 %6, i1 false) ; LinAlgCopyConvertMatrix(srcMatrix,transpose)
; CHECK-NEXT: Validation failed.
ret void
}

; Function Attrs: nounwind
declare %dx.types.LinAlgMatrixC2M5N4U1S2 @dx.op.linAlgFillMatrix.mC2M5N4U1S2.i32(i32, i32) #0

; Function Attrs: nounwind
declare %dx.types.LinAlgMatrixC4M8N4U1S2 @dx.op.linAlgCopyConvertMatrix.mC4M8N4U1S2.mC2M5N4U1S2(i32, %dx.types.LinAlgMatrixC2M5N4U1S2, i1) #0

; Function Attrs: nounwind
declare %dx.types.LinAlgMatrixC4M5N8U1S2 @dx.op.linAlgCopyConvertMatrix.mC4M5N8U1S2.mC2M5N4U1S2(i32, %dx.types.LinAlgMatrixC2M5N4U1S2, i1) #0

; Function Attrs: nounwind
declare %dx.types.LinAlgMatrixC4M5N4U1S2 @dx.op.linAlgCopyConvertMatrix.mC4M5N4U1S2.mC2M5N4U1S2(i32, %dx.types.LinAlgMatrixC2M5N4U1S2, i1) #0

; Function Attrs: nounwind
declare %dx.types.LinAlgMatrixC2M4N5U1S1 @dx.op.linAlgCopyConvertMatrix.mC2M4N5U1S1.mC2M5N4U1S2(i32, %dx.types.LinAlgMatrixC2M5N4U1S2, i1) #0

; Function Attrs: nounwind
declare %dx.types.LinAlgMatrixC2M4N5U1S0 @dx.op.linAlgFillMatrix.mC2M4N5U1S0.i32(i32, i32) #0

; Function Attrs: nounwind
declare %dx.types.LinAlgMatrixC2M4N5U1S0 @dx.op.linAlgCopyConvertMatrix.mC2M4N5U1S0.mC2M4N5U1S0(i32, %dx.types.LinAlgMatrixC2M4N5U1S0, i1) #0

attributes #0 = { nounwind }

!dx.targetTypes = !{!0, !1, !2, !3, !4, !5}
!llvm.ident = !{!6}
!dx.version = !{!7}
!dx.valver = !{!7}
!dx.shaderModel = !{!8}
!dx.entryPoints = !{!9}

!0 = !{%dx.types.LinAlgMatrixC2M5N4U1S2 undef, i32 2, i32 5, i32 4, i32 1, i32 2}
!1 = !{%dx.types.LinAlgMatrixC4M8N4U1S2 undef, i32 4, i32 8, i32 4, i32 1, i32 2}
!2 = !{%dx.types.LinAlgMatrixC4M5N8U1S2 undef, i32 4, i32 5, i32 8, i32 1, i32 2}
!3 = !{%dx.types.LinAlgMatrixC4M5N4U1S2 undef, i32 4, i32 5, i32 4, i32 1, i32 2}
!4 = !{%dx.types.LinAlgMatrixC2M4N5U1S1 undef, i32 2, i32 4, i32 5, i32 1, i32 1}
!5 = !{%dx.types.LinAlgMatrixC2M4N5U1S0 undef, i32 2, i32 4, i32 5, i32 1, i32 0}
!6 = !{!"dxc(private) 1.9.0.5389 (linalg-validation-component-type, b8b639b7d-dirty)"}
!7 = !{i32 1, i32 10}
!8 = !{!"cs", i32 6, i32 10}
!9 = !{void ()* @main, !"main", null, null, !10}
!10 = !{i32 4, !11}
!11 = !{i32 1, i32 1, i32 1}
13 changes: 12 additions & 1 deletion utils/hct/hctdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -8644,7 +8644,18 @@ def build_valrules(self):
"Instr.LinAlgIllegalComponentType",
"Matrix Component Type '%0' not allowed in LinAlg Matrix.",
)

self.add_valrule(
"Instr.LinAlgMatrixScopeNotAllowed",
"Matrix Scope '%0' not allowed in %1 operation.",
)
self.add_valrule(
"Instr.LinAlgMatrixScopeMismatch",
"Matrix Scope '%0' does not match expected scope %1.",
)
self.add_valrule(
"Instr.LinAlgMatrixDimMismatch",
"Matrix Dimension '%0x%1' does not match expected dimension %2x%3.",
)
# Some legacy rules:
# - space is only supported for shader targets 5.1 and higher
# - multiple rules regarding derivatives, which isn't a supported feature for DXIL
Expand Down
Loading