From c78ae35850e803b875d4f7abeebcb9f0411b783f Mon Sep 17 00:00:00 2001 From: NielsbishereAlt Date: Mon, 12 May 2025 13:03:16 +0200 Subject: [PATCH 1/8] 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 2/8] 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 3/8] 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 4/8] 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 5/8] 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 6/8] 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 7/8] 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 8/8] 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(