From 487e9aaacaac62097953162f227eaaecdedc7c70 Mon Sep 17 00:00:00 2001 From: Laura Hermanns Date: Sun, 6 Sep 2026 09:23:05 -0400 Subject: [PATCH] [SPIR-V] Fix matrix ordering in `mul()` intrinsic for vertex attributes. Vertex input attributes cannot be decorated with `row_major`/`column_major` type qualifiers, neither in HLSL nor in SPIR-V. Therefore, they cannot be emitted with a flipped matrix ordering in the `mul()` intrinsic that performs an implicit transpose. Instead, they must assume a flipped matrix memory layout in SPIR-V which is canceled out by keeping the intrinsic operands as-is. Note that this only works for square matrices and non-square matrices will need a separate solution. --- docs/ReleaseNotes.md | 1 + tools/clang/lib/SPIRV/SpirvEmitter.cpp | 128 ++++++++++++++++-- ...rtex.attribute.float2x2.mul.intrinsic.hlsl | 38 ++++++ utils/lit/lit/TestingConfig.py | 2 +- 4 files changed, 157 insertions(+), 12 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/vertex.attribute.float2x2.mul.intrinsic.hlsl diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 8ff6ad323c..3a77eba141 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -33,6 +33,7 @@ line upon naming the release. Refer to previous for appropriate section names. - Fix a crash generating DXIL from sources containing a dynamic resource heap access that was discarded. Identified during development of SPIR-V support for [descriptor heaps](https://github.com/microsoft/DirectXShaderCompiler/pull/8517#discussion_r3752113078). +- SPIR-V: Fixed matrix ordering for vertex input attributes of square matrices. #### HLSL Language diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index f05c0d9553..bde30dc44d 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -579,6 +579,82 @@ const StructType *lowerStructType(const SpirvCodeGenOptions &spirvOptions, return output; } +bool hasAnySemantic(const DeclaratorDecl *decl) { + if (!decl) + return false; + + for (auto *annotation : decl->getUnusualAnnotations()) + if (isa(annotation)) + return true; + + return false; +} + +// Walks expression base chain and returns true if it is rooted in an input +// parameter or stage variable. +bool isVertexInputExpr(const Expr *expr) { + if (!expr) + return false; + + expr = expr->IgnoreParenCasts(); + bool hasSemanticField = false; + + while (expr) { + if (const auto *member = dyn_cast(expr)) { + if (const auto *decl = dyn_cast(member->getMemberDecl())) + hasSemanticField |= hasAnySemantic(decl); + expr = member->getBase()->IgnoreParenCasts(); + continue; + } + + if (const auto *subscript = dyn_cast(expr)) { + expr = subscript->getBase()->IgnoreParenCasts(); + continue; + } + + if (const auto *vecElem = dyn_cast(expr)) { + expr = vecElem->getBase()->IgnoreParenCasts(); + continue; + } + + if (const auto *declRef = dyn_cast(expr)) { + if (const auto *parm = dyn_cast(declRef->getDecl())) + return canActAsInParmVar(parm) && + (hasSemanticField || hasAnySemantic(parm)); + return false; + } + + break; + } + + return false; +} + +// Walks instruction provenance to detect stage input origin. +bool originatesFromInputStorage(SpirvInstruction *inst) { + if (!inst) + return false; + + if (inst->getStorageClass() == spv::StorageClass::Input) + return true; + + switch (inst->getKind()) { + case SpirvInstruction::IK_Load: + return originatesFromInputStorage(cast(inst)->getPointer()); + case SpirvInstruction::IK_AccessChain: + return originatesFromInputStorage(cast(inst)->getBase()); + case SpirvInstruction::IK_CopyObject: + return originatesFromInputStorage(cast(inst)->getPointer()); + case SpirvInstruction::IK_CompositeExtract: + return originatesFromInputStorage( + cast(inst)->getComposite()); + case SpirvInstruction::IK_UnaryOp: + return originatesFromInputStorage(cast(inst)->getOperand()); + default: + return false; + } +} + } // namespace SpirvEmitter::SpirvEmitter(CompilerInstance &ci) @@ -12347,36 +12423,66 @@ SpirvInstruction *SpirvEmitter::processIntrinsicMul(const CallExpr *callExpr) { // mul(vector, matrix) { QualType vecElemType = {}, matElemType = {}; - uint32_t elemCount = 0, numRows = 0; + uint32_t elemCount = 0, numRows = 0, numCols = 0; if (isVectorType(arg0Type, &vecElemType, &elemCount) && - isMxNMatrix(arg1Type, &matElemType, &numRows)) { + isMxNMatrix(arg1Type, &matElemType, &numRows, &numCols)) { assert(elemCount == numRows); - if (vecElemType->isFloatingType() && matElemType->isFloatingType()) + if (vecElemType->isFloatingType() && matElemType->isFloatingType()) { + const bool isSquare = (numRows == numCols); + const bool fromVertexInput = + isVertexInputExpr(arg1) || originatesFromInputStorage(arg1Id); + + // Workaround: if matrix originates from vertex input and is square, + // emit OpVectorTimesMatrix without operand swapping. + // Otherwise, row_major cannot be emulated here because + // SPIR-V vertex attributes cannot be decorated with row_major layout. + if (isSquare && fromVertexInput) + return spvBuilder.createBinaryOp(spv::Op::OpVectorTimesMatrix, + returnType, arg0Id, arg1Id, loc, + range); + + // Default path (existing behavior): swap operands and emit MatrixTimesVector. return spvBuilder.createBinaryOp(spv::Op::OpMatrixTimesVector, returnType, arg1Id, arg0Id, loc, range); - else + } else { return processNonFpVectorTimesMatrix(arg0Type, arg0Id, arg1Type, arg1Id, callExpr->getExprLoc(), nullptr, range); + } } } // mul(matrix, vector) { QualType vecElemType = {}, matElemType = {}; - uint32_t elemCount = 0, numCols = 0; - if (isMxNMatrix(arg0Type, &matElemType, nullptr, &numCols) && + uint32_t elemCount = 0, numRows = 0, numCols = 0; + if (isMxNMatrix(arg0Type, &matElemType, &numRows, &numCols) && isVectorType(arg1Type, &vecElemType, &elemCount)) { assert(elemCount == numCols); - if (vecElemType->isFloatingType() && matElemType->isFloatingType()) - return spvBuilder.createBinaryOp(spv::Op::OpVectorTimesMatrix, - returnType, arg1Id, arg0Id, loc, - range); - else + + if (vecElemType->isFloatingType() && matElemType->isFloatingType()) { + const bool isSquare = (numRows == numCols); + const bool fromVertexInput = + isVertexInputExpr(arg0) || originatesFromInputStorage(arg0Id); + + // Workaround: if matrix originates from vertex input and is square, + // emit OpMatrixTimesVector without operand swapping. + // Otherwise, row_major cannot be emulated here because + // SPIR-V vertex attributes cannot be decorated with row_major layout. + if (isSquare && fromVertexInput) + return spvBuilder.createBinaryOp(spv::Op::OpMatrixTimesVector, + returnType, arg0Id, arg1Id, loc, + range); + + // Default path (existing behavior): swap operands and emit VectorTimesMatrix. + return spvBuilder.createBinaryOp(spv::Op::OpVectorTimesMatrix, returnType, + arg1Id, arg0Id, loc, range); + } else { return processNonFpMatrixTimesVector(arg0Type, arg0Id, arg1Type, arg1Id, callExpr->getExprLoc(), range); + } } } diff --git a/tools/clang/test/CodeGenSPIRV/vertex.attribute.float2x2.mul.intrinsic.hlsl b/tools/clang/test/CodeGenSPIRV/vertex.attribute.float2x2.mul.intrinsic.hlsl new file mode 100644 index 0000000000..f3a020630d --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vertex.attribute.float2x2.mul.intrinsic.hlsl @@ -0,0 +1,38 @@ +// RUN: %dxc -T vs_6_0 -E main -fspv-target-env=vulkan1.3 %s -spirv | FileCheck %s + +struct VSIn { + float2x2 rotationA : ROTATIONA; + float2 positionA : POSITIONA; + float2x2 rotationB : ROTATIONB; + float2 positionB : POSITIONB; +}; + +cbuffer SceneInput : register(b0) { + float4x4 projection; +}; + +// Matrix types from vertex input attributes must not be emitted as implicitly transposed in SPIR-V, +// because they cannot be decorated with the row_major/column_major type qualifiers. +// The mul() intrinsic's operands must therefore be emitted as-is, +// which results in a double flip of matrix ordering: (1) in the vertex input attribute and (2) in the mul() intrinsic. +// These cancel each other out and result in the same matrix transformation between DXIL and SPIR-V. +// The second mul() intrinsic in this test must be emitted as before, +// with flipped operands and OpVectorTimesMatrix instruction. +float4 main(VSIn input) : SV_Position { + // CHECK: [[rotationA:%[0-9]+]] = OpLoad %mat2v2float {{%[a-zA-Z0-9_]+}} + // CHECK-NEXT: [[positionA:%[0-9]+]] = OpLoad %v2float {{%[a-zA-Z0-9_]+}} + // CHECK: [[rotationB:%[0-9]+]] = OpLoad %mat2v2float {{%[a-zA-Z0-9_]+}} + // CHECK-NEXT: [[positionB:%[0-9]+]] = OpLoad %v2float {{%[a-zA-Z0-9_]+}} + // CHECK: [[mulA:%[0-9]+]] = OpMatrixTimesVector %v2float [[rotationA]] [[positionA]] + float4 worldSpacePositionA = float4(mul(input.rotationA, input.positionA) + input.positionA, 0.0, 1.0); + + // CHECK: [[worldSpacePositionA:%[0-9]+]] = OpCompositeConstruct %v4float + // CHECK: [[mulB:%[0-9]+]] = OpVectorTimesMatrix %v2float [[positionB]] [[rotationB]] + float4 worldSpacePositionB = float4(mul(input.positionB, input.rotationB) + input.positionB, 0.0, 1.0); + + // CHECK: [[worldSpacePositionB:%[0-9]+]] = OpCompositeConstruct %v4float + // CHECK: [[projection:%[0-9]+]] = OpLoad %mat4v4float {{%[a-zA-Z0-9_]+}} + // CHECK: [[projectionMulA:%[0-9]+]] = OpVectorTimesMatrix %v4float [[worldSpacePositionA]] [[projection]] + // CHECK: [[projectionMulB:%[0-9]+]] = OpVectorTimesMatrix %v4float [[worldSpacePositionB]] [[projection]] + return mul(projection, worldSpacePositionA) + mul(projection, worldSpacePositionB); +} diff --git a/utils/lit/lit/TestingConfig.py b/utils/lit/lit/TestingConfig.py index b2dbcc6a53..651dd50435 100644 --- a/utils/lit/lit/TestingConfig.py +++ b/utils/lit/lit/TestingConfig.py @@ -39,7 +39,7 @@ def _find_git_windows_unix_tools(tools_needed): return lit.util.to_string(candidate_path) except: continue - raise(f"fail to find {tools_needed} which are required for DXC tests") + raise RuntimeError(f"failed to find {tools_needed} which are required for DXC tests") class TestingConfig: """"