From c78ae35850e803b875d4f7abeebcb9f0411b783f Mon Sep 17 00:00:00 2001 From: NielsbishereAlt Date: Mon, 12 May 2025 13:03:16 +0200 Subject: [PATCH 01/10] Updated some outdated documentation in the CLI and SPIR-V.rst and lib compiles now also allow fvk-invert-y. --- docs/SPIR-V.rst | 2 +- include/dxc/Support/HLSLOptions.td | 2 +- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index b5e9c05079..f3981ba854 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -4227,7 +4227,7 @@ codegen for Vulkan: - ``-fvk-use-dx-layout``: Uses DirectX layout rules for resources. - ``-fvk-invert-y``: Negates (additively inverts) SV_Position.y before writing to stage output. Used to accommodate the difference between Vulkan's - coordinate system and DirectX's. Only allowed in VS/DS/GS. + coordinate system and DirectX's. Only allowed in VS/DS/GS/MS/Lib. - ``-fvk-use-dx-position-w``: Reciprocates (multiplicatively inverts) SV_Position.w after reading from stage input. Used to accommodate the difference between Vulkan DirectX: the w component of SV_Position in PS is diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index 4d72cb2312..58f6bdfbf3 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -368,7 +368,7 @@ def fvk_bind_register : MultiArg<["-"], "fvk-bind-register", 4>, MetaVarName<"; def vkbr : MultiArg<["-"], "vkbr", 4>, Flags<[CoreOption, DriverOption]>, Alias; def fvk_invert_y: Flag<["-"], "fvk-invert-y">, Group, Flags<[CoreOption, DriverOption]>, - HelpText<"Negate SV_Position.y before writing to stage output in VS/DS/GS to accommodate Vulkan's coordinate system">; + HelpText<"Negate SV_Position.y before writing to stage output in VS/DS/GS/MS/Lib to accommodate Vulkan's coordinate system">; def fvk_use_dx_position_w: Flag<["-"], "fvk-use-dx-position-w">, Group, Flags<[CoreOption, DriverOption]>, HelpText<"Reciprocate SV_Position.w after reading from stage input in PS to accommodate the difference between Vulkan and DirectX">; def fvk_support_nonzero_base_instance: Flag<["-"], "fvk-support-nonzero-base-instance">, Group, Flags<[CoreOption, DriverOption]>, diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 7337a33b01..c794fcfc1a 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -604,8 +604,8 @@ SpirvEmitter::SpirvEmitter(CompilerInstance &ci) emitError("unknown shader module: %0", {}) << shaderModel->GetName(); if (spirvOptions.invertY && !shaderModel->IsVS() && !shaderModel->IsDS() && - !shaderModel->IsGS() && !shaderModel->IsMS()) - emitError("-fvk-invert-y can only be used in VS/DS/GS/MS", {}); + !shaderModel->IsGS() && !shaderModel->IsMS() && !shaderModel->IsLib()) + emitError("-fvk-invert-y can only be used in VS/DS/GS/MS/Lib", {}); if (spirvOptions.useGlLayout && spirvOptions.useDxLayout) emitError("cannot specify both -fvk-use-dx-layout and -fvk-use-gl-layout", From d94759845ce077c24b78a57be0479afb4e022d8c Mon Sep 17 00:00:00 2001 From: NielsbishereAlt Date: Mon, 12 May 2025 13:14:27 +0200 Subject: [PATCH 02/10] Now only inverting Y on SV_POSITION if supported by current entrypoint; avoids pixel shader from inverting SV_POSITION.y in a lib file --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index c794fcfc1a..512b27ebcf 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -14881,8 +14881,12 @@ SpirvEmitter::createSpirvIntrInstExt(llvm::ArrayRef attrs, SpirvInstruction *SpirvEmitter::invertYIfRequested(SpirvInstruction *position, SourceLocation loc, SourceRange range) { - // Negate SV_Position.y if requested - if (spirvOptions.invertY) { + // Negate SV_Position.y if requested and supported + + bool supportsInvertY = spvContext.isVS() || spvContext.isGS() || + spvContext.isGS() || spvContext.isMS(); + + if (spirvOptions.invertY && supportsInvertY) { const auto oldY = spvBuilder.createCompositeExtract( astContext.FloatTy, position, {1}, loc, range); const auto newY = spvBuilder.createUnaryOp( From 70eb03c090f5413d7d04f3815fbd7302cf288c0d Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Mon, 12 May 2025 14:44:13 +0200 Subject: [PATCH 03/10] Fix typo --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 512b27ebcf..7042854504 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -14884,7 +14884,7 @@ SpirvInstruction *SpirvEmitter::invertYIfRequested(SpirvInstruction *position, // Negate SV_Position.y if requested and supported bool supportsInvertY = spvContext.isVS() || spvContext.isGS() || - spvContext.isGS() || spvContext.isMS(); + spvContext.isDS() || spvContext.isMS(); if (spirvOptions.invertY && supportsInvertY) { const auto oldY = spvBuilder.createCompositeExtract( From c6f568446b25a78087e4b5c5b355aaf2fa52f41a Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 14 May 2025 20:43:04 +0200 Subject: [PATCH 04/10] supportsInvertY is now required during invertY since SV_POSITION is only supported on GS/VS/DS/MS already. Added test case for fvk-invert-y for lib files --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 4 +++- .../test/CodeGenSPIRV/vk.cloption.invert-y.lib.hlsl | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 tools/clang/test/CodeGenSPIRV/vk.cloption.invert-y.lib.hlsl diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 7042854504..77101ce8b8 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -14886,7 +14886,9 @@ SpirvInstruction *SpirvEmitter::invertYIfRequested(SpirvInstruction *position, bool supportsInvertY = spvContext.isVS() || spvContext.isGS() || spvContext.isDS() || spvContext.isMS(); - if (spirvOptions.invertY && supportsInvertY) { + assert(supportsInvertY && "invertY is only supported in VS/DS/GS/MS") + + if (spirvOptions.invertY) { const auto oldY = spvBuilder.createCompositeExtract( astContext.FloatTy, position, {1}, loc, range); const auto newY = spvBuilder.createUnaryOp( diff --git a/tools/clang/test/CodeGenSPIRV/vk.cloption.invert-y.lib.hlsl b/tools/clang/test/CodeGenSPIRV/vk.cloption.invert-y.lib.hlsl new file mode 100644 index 0000000000..6dac20fc6f --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.cloption.invert-y.lib.hlsl @@ -0,0 +1,12 @@ +// RUN: %dxc -T lib_6_3 -fvk-invert-y -fcgl %s -spirv | FileCheck %s + +[shader("vertex")] +float4 main(float4 a : A) : SV_Position { + return a; +} + +// CHECK: [[a:%[0-9]+]] = OpFunctionCall %v4float %src_main %param_var_a +// CHECK-NEXT: [[oldY:%[0-9]+]] = OpCompositeExtract %float [[a]] 1 +// CHECK-NEXT: [[newY:%[0-9]+]] = OpFNegate %float [[oldY]] +// CHECK-NEXT: [[pos:%[0-9]+]] = OpCompositeInsert %v4float [[newY]] [[a]] 1 +// CHECK-NEXT: OpStore %gl_Position [[pos]] From 8fd70ec23bc2d47dff0078ccf4be38618f8a1f21 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 14 May 2025 20:48:59 +0200 Subject: [PATCH 05/10] Fix format issues --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 77101ce8b8..dfb453789e 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -14888,7 +14888,7 @@ SpirvInstruction *SpirvEmitter::invertYIfRequested(SpirvInstruction *position, assert(supportsInvertY && "invertY is only supported in VS/DS/GS/MS") - if (spirvOptions.invertY) { + if (spirvOptions.invertY) { const auto oldY = spvBuilder.createCompositeExtract( astContext.FloatTy, position, {1}, loc, range); const auto newY = spvBuilder.createUnaryOp( From 2ef55cbaeeff6fa3bbb4eda91b0443b074f90d36 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 14 May 2025 21:10:27 +0200 Subject: [PATCH 06/10] Fixed a typo --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index dfb453789e..e1dbdba98c 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -14886,7 +14886,7 @@ SpirvInstruction *SpirvEmitter::invertYIfRequested(SpirvInstruction *position, bool supportsInvertY = spvContext.isVS() || spvContext.isGS() || spvContext.isDS() || spvContext.isMS(); - assert(supportsInvertY && "invertY is only supported in VS/DS/GS/MS") + assert(supportsInvertY && "invertY is only supported in VS/DS/GS/MS"); if (spirvOptions.invertY) { const auto oldY = spvBuilder.createCompositeExtract( From 11f7bb2ac3aad86b1e3dde0de2cfe4c1a4466a48 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 14 May 2025 21:12:43 +0200 Subject: [PATCH 07/10] For some reason the formatter wants me to revert back to my old formatting? --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index e1dbdba98c..ea152d4ad9 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -14888,7 +14888,7 @@ SpirvInstruction *SpirvEmitter::invertYIfRequested(SpirvInstruction *position, assert(supportsInvertY && "invertY is only supported in VS/DS/GS/MS"); - if (spirvOptions.invertY) { + if (spirvOptions.invertY) { const auto oldY = spvBuilder.createCompositeExtract( astContext.FloatTy, position, {1}, loc, range); const auto newY = spvBuilder.createUnaryOp( From 2ced6e2203b1b7e51bfcf20e2d0f9051180b6b55 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 14 May 2025 21:48:33 +0200 Subject: [PATCH 08/10] Actually, HsCpOut can contain SV_POSITION too, which would not be allowed to be flipped --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index ea152d4ad9..7042854504 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -14886,9 +14886,7 @@ SpirvInstruction *SpirvEmitter::invertYIfRequested(SpirvInstruction *position, bool supportsInvertY = spvContext.isVS() || spvContext.isGS() || spvContext.isDS() || spvContext.isMS(); - assert(supportsInvertY && "invertY is only supported in VS/DS/GS/MS"); - - if (spirvOptions.invertY) { + if (spirvOptions.invertY && supportsInvertY) { const auto oldY = spvBuilder.createCompositeExtract( astContext.FloatTy, position, {1}, loc, range); const auto newY = spvBuilder.createUnaryOp( From 067b2e61be6ebc51f4d31f3a6c82ec77824edba0 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Thu, 3 Jul 2025 00:21:20 +0200 Subject: [PATCH 09/10] Fix invalid value in ms_SemanticInterpretationTable; it disallows SV_PrimitiveID from Vertex->Geometry shaders (GSVIn) even though according to the spec it should be allowed. This breaks existing shaders but only if they're compiled as lib file (compiling it with gs_ is fine). This is because the value in this table is NA, which results in the semantic becoming invalid and down the road that results in this error: "Semantic 'SV_PrimitiveID' is invalid as gs Input." --- docs/DXIL.rst | 2 +- include/dxc/DXIL/DxilSigPoint.inl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/DXIL.rst b/docs/DXIL.rst index 7532ec3c42..c719cdd034 100644 --- a/docs/DXIL.rst +++ b/docs/DXIL.rst @@ -699,7 +699,7 @@ ClipDistance Arb ClipCull NA NA ClipCull CullDistance Arb ClipCull NA NA ClipCull ClipCull Arb Arb ClipCull ClipCull ClipCull NA ClipCull ClipCull NA NA NA ClipCull NA NA OutputControlPointID NA NA NA NotInSig NA NA NA NA NA NA NA NA NA NA NA NA NA NA NA NA DomainLocation NA NA NA NA NA NA NA NotInSig NA NA NA NA NA NA NA NA NA NA NA NA -PrimitiveID NA NA NotInSig NotInSig NA NA NA NotInSig NA NA NA Shadow SGV SGV NA NA NA NA SV NA +PrimitiveID NA NA NotInSig NotInSig NA NA NA NotInSig NA NA SGV Shadow SGV SGV NA NA NA NA SV NA GSInstanceID NA NA NA NA NA NA NA NA NA NA NA NotInSig NA NA NA NA NA NA NA NA SampleIndex NA NA NA NA NA NA NA NA NA NA NA NA NA Shadow _41 NA NA NA NA NA NA IsFrontFace NA NA NA NA NA NA NA NA NA NA NA NA SGV SGV NA NA NA NA NA NA diff --git a/include/dxc/DXIL/DxilSigPoint.inl b/include/dxc/DXIL/DxilSigPoint.inl index 650eae5fe0..22f2438058 100644 --- a/include/dxc/DXIL/DxilSigPoint.inl +++ b/include/dxc/DXIL/DxilSigPoint.inl @@ -87,7 +87,7 @@ const SigPoint SigPoint::ms_SigPoints[kNumSigPointRecords] = { ROW(DomainLocation, NA, NA, NA, NA, NA, NA, NA, NotInSig, NA, NA, NA, NA, \ NA, NA, NA, NA, NA, NA, NA, NA) \ ROW(PrimitiveID, NA, NA, NotInSig, NotInSig, NA, NA, NA, NotInSig, NA, NA, \ - NA, Shadow, SGV, SGV, NA, NA, NA, NA, SV, NA) \ + SGV, Shadow, SGV, SGV, NA, NA, NA, NA, SV, NA) \ ROW(GSInstanceID, NA, NA, NA, NA, NA, NA, NA, NA, NA, NA, NA, NotInSig, NA, \ NA, NA, NA, NA, NA, NA, NA) \ ROW(SampleIndex, NA, NA, NA, NA, NA, NA, NA, NA, NA, NA, NA, NA, NA, \ From 2df4c346297703f52792092cb1c645eefbbaa4b9 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Thu, 3 Jul 2025 00:53:32 +0200 Subject: [PATCH 10/10] Fixed a missing occurrence of NA->SGV --- utils/hct/hctdb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/hct/hctdb.py b/utils/hct/hctdb.py index 2b94b13134..dd8afb5dd9 100644 --- a/utils/hct/hctdb.py +++ b/utils/hct/hctdb.py @@ -7782,7 +7782,7 @@ def build_semantics(self): CullDistance,Arb,ClipCull,NA,NA,ClipCull,ClipCull,Arb,Arb,ClipCull,ClipCull,ClipCull,NA,ClipCull,ClipCull,NA,NA,NA,ClipCull,NA,NA OutputControlPointID,NA,NA,NA,NotInSig,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA DomainLocation,NA,NA,NA,NA,NA,NA,NA,NotInSig,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA - PrimitiveID,NA,NA,NotInSig,NotInSig,NA,NA,NA,NotInSig,NA,NA,NA,Shadow,SGV,SGV,NA,NA,NA,NA,SV,NA + PrimitiveID,NA,NA,NotInSig,NotInSig,NA,NA,NA,NotInSig,NA,NA,SGV,Shadow,SGV,SGV,NA,NA,NA,NA,SV,NA GSInstanceID,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NotInSig,NA,NA,NA,NA,NA,NA,NA,NA SampleIndex,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,Shadow _41,NA,NA,NA,NA,NA,NA IsFrontFace,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,NA,SGV,SGV,NA,NA,NA,NA,NA,NA