SPIR-V `SV_InstanceId` support in pixel shader #4368

ArielG-NV · 2024-06-12T23:34:06Z

Problem:

SV_InstanceID (HLSL) is allowed as an output semantic with HLSL, it is also allowed for input into pixel shader. We need to account for multiple entry points in 1 file using both of these scenarios at once.
SPIRV does not allow SV_InstanceID equivalent as a builtin, SV_InstanceID must be passed as a regular Flat Input from vertex shader.

Solution: Slang needs to treat these SV objects as non built-in's. As a result:

Slang needs to allocate for vertex output and fragment Input binding slots for all SV_InstanceID uses (if the target is SPIRV). This allows Slang to prepare an open slot to bind these SV objects to.
Slang needs to not emit built in modifier for these not actual built in variables (under the specific circumstances described).

Note: we do not need to add Flat ourselves to any fragment Input variables since the parameter being transformed is a uint. This means to avoid validation errors Flat must be added anyways, and Slang implements this behavior for us.

…an SV once diagnosed by slang Problem: 1. SV_InstanceID (HLSL) is allowed as an output semantic with HLSL, it is also allowed for input into pixel shader. We need to account for multiple entry points in 1 file using both of these scenarios at once. 2. SPIRV does not allow `SV_InstanceID` as a builtin, `SV_InstanceID` must be passed as a regular `flat` `Input` from vertex shader. Solution: Slang needs to treat these SV objects as non built-in's. As a result: 1. Slang needs to allocate for vertex output and fragment Input binding slots for all SV_InstanceID uses (if the target is SPIRV). This allows Slang to prepare an open slot to bind these SV objects to. 2. Slang needs to not emit built in modifier for these not actual built in variables (under the specific circumstances described). note: `VarLayout` was made not `HOISTABLE` since I don't believe it needs to be `HOISTABLE`. * The code can be made to work even if `VarLayout` is `HOISTABLE`.

…shader-shader-slang#3087

jkwak-work

Correct me if I am missing something.
But I think the variable needs to be marked as Flat.
I don't see anywhere in your change to make it Flat.
Also make sure you are testing that the keyword Flat is present in the emitted Spirv-asm

tests/bugs/gh-3087.slang

source/slang/slang-ir-inst-defs.h

1. remove an operand by selectively copying operands 2. test to ensure `Flat` is in the test shaders * we do not need to manually add `Flat` in the code since this is done for us in emit-spirv. This is the behavior since `uint` on a fragment `Input` must be `Flat`, else it is a VK validation error.

ArielG-NV · 2024-06-13T04:23:10Z

Also make sure you are testing that the keyword Flat is present in the emitted Spirv-asm

I added to the test, this is a good suggestion (it is a VK validation error to not have Flat)

But I think the variable needs to be marked as Flat.
I don't see anywhere in your change to make it Flat.

We do not need to add Flat ourselves to any fragment Input variables since the parameter being transformed is a uint.
This means to avoid validation errors Flat must be added anyways, and Slang implements this behavior for us.
This is behavior which Slang implements is used in more than 1 instance, so it is likely correct to be using this behavior.

csyonghe · 2024-06-13T06:10:44Z

source/slang/slang-ir-glsl-legalize.cpp

@@ -1285,6 +1285,33 @@ ScalarizedVal createSimpleGLSLGlobalVarying(
        declarator,
        &systemValueInfoStorage);

+    {
+        uint32_t index = 0;
+        IRSystemValueSemanticAttr* systemSemantic = nullptr;


systemSemantic = inVarLayout->findSystemValueSemanticAttr()l

csyonghe · 2024-06-13T06:12:28Z

source/slang/slang-ir-glsl-legalize.cpp

+        if (systemSemantic && isSPIRV(codeGenContext->getTargetFormat()) && systemSemantic->getName().caseInsensitiveEquals(UnownedStringSlice("sv_instanceid"))
+            && ((stage == Stage::Fragment) || (stage == Stage::Vertex && inVarLayout->usesResourceKind(LayoutResourceKind::VaryingOutput))))
+        {
+            IRCloneEnv cloneEnv;


Instead of using IRClone, just use a List to store all the operands you want to retain, and create a new inst with that list of operands. Simply call emitIntrinsicInst to do that. There is no need to use the clone mechanim and the cloneInstExcludingSomeOperands function, since the layout inst will not have any children.

…shader-shader-slang#3087

remove clone logic

…-slang#3087' of https://github.com/ArielG-NV/slang into HLSL-SV_InstanceId-not-supported-in-pixel-shader-shader-slang#3087

csyonghe · 2024-06-13T16:52:26Z

source/slang/slang-ir-glsl-legalize.cpp

+        {
+            ShortList<IRInst*> newOperands;
+            auto opCount = inVarLayout->getOperandCount();
+            newOperands.setCount(opCount);


Just call reserveOverflowBuffer and add in the loop to get rid of the opCount here.

csyonghe · 2024-06-13T16:52:41Z

source/slang/slang-ir-clone.cpp

@@ -100,6 +100,47 @@ IRInst* cloneInstAndOperands(
    return newInst;
 }

+IRInst* cloneInstExcludingSomeOperands(


No longer required?

remove unused function reserve instead of setCount with ShortList

…shader-shader-slang#3087

v2024.1.22 c00f461 remove inline that crashes old glibc version (shader-slang#4398) fdef653 Improve Direct SPIRV Backend Test Coverage (shader-slang#4396) 33e81a0 [Metal] Fix global constant array emit. (shader-slang#4392) a38a4fb Make unknown attributes a warning instead of an error. (shader-slang#4391) a210091 [Metal] Support SV_TargetN. (shader-slang#4390) 2cc9690 Implement for metal `SV_GroupIndex` (shader-slang#4385) a6b8348 fix the clang/gcc warning (shader-slang#4382) cfef0c6 Metal: misc fixes and enable more tests. (shader-slang#4374) 2407966 SPIR-V `SV_InstanceId` support in pixel shader (shader-slang#4368) fba316f Remove `IRHLSLExportDecoration` and `IRKeepAliveDecoration` for non-CUDA/Torch targets (shader-slang#4364) f0d40ad (origin/master, origin/HEAD, master) capture/replay: implement infrastructure for capture (shader-slang#4372) ecc6ecb Fix cuda/cpp/metal crash for when using GLSL style shader inputs (shader-slang#4378) 0bf0bf7 Implement Sampler2D for CPP target (shader-slang#4371) b970b88 Enable full test on macos. (shader-slang#4327) 0574dca Delete glsl_vulkan and glsl_vulkan_one_desc targets. (shader-slang#4361) 085d1a6 Fix emit logic for getElementPtr. (shader-slang#4362) 8813c61 Capability System: Implicit capability upgrade warning/error (shader-slang#4241) 7447fca Add constant folding for % operator. (shader-slang#4359) 8bf7d11 Fix merge error. (shader-slang#4358) b7e8243 Add slangc flag to `-zero-initialize` all variables (shader-slang#3987) ccc26c2 Extend the COM-based API to support whole program compilation. (shader-slang#4355) 318adcc Add compiler option to treat enum types as unscoped. (shader-slang#4354) ec35feb Fix incorrect drop of decoration when translating glsl global var to entrypoint param. (shader-slang#4353) c194af8 Fix crash on invalid entrypoint varying parameter. (shader-slang#4349) 7a4757d Implicit register binding for hlsl to non-hlsl targets (shader-slang#4338) 180d6b1 Fix duplicate SPIRV decorations. (shader-slang#4346) fa8c11e Add option to preserve shader parameter declaration in output SPIRV. (shader-slang#4344) 3fe4a77 Fix crash when using optional type in a generic. (shader-slang#4341) 5da06d4 Fix global value inlining for spirv_asm blocks. (shader-slang#4339) 7e79669 [gfx] Metal improvements (shader-slang#4337) 6909d65 SPIRV backend: add support for tessellation stages, (shader-slang#4336) ef20d93 Test more texture types in Metal (shader-slang#4333) 5a28968 [gfx] Metal texture fixes (shader-slang#4331) df0a201 Support integer typed textures for GLSL (shader-slang#4329) 51d3585 Remove duplicate `VkPhysicalDeviceComputeShaderDerivativesFeaturesNV` extension structure in vk-api.h (shader-slang#4335) 6d5ef9b Fix `GetAttributeAtVertex` for spirv and glsl targets. (shader-slang#4334) 21bbebb Address glslang ordering requirments for 'derivative_group_*NV' (shader-slang#4323) 72016f9 Partial implementation of static_assert (shader-slang#4294) 712ce65 enable more metal tests (shader-slang#4326) 38c0bac Fix SPIRV emit for `Flat` decoration and TessLevel builtin. (shader-slang#4318) b5cdd83 Support all integer typed indices in StructuredBuffer Load/Store/[]. (shader-slang#4311) 6857dd5 [gfx] Metal graphics support (shader-slang#4324) 0974463 Fix typos in the docs (shader-slang#4322) 9a23a9a SPIRV `Block` decoration fixes. (shader-slang#4303) bc680e7 Add initial draft auto-diff basics and IR overview documents (shader-slang#4216) ee812d1 Disallow certain types of decls in `interface` to provide better diagnostic message. (shader-slang#4312) 65928af Metal system value overhaul. (shader-slang#4308) e39ceab Adding functional test for GLSL texture functions (shader-slang#4306) 2a45bc3 Support HLSL `.mips` syntax. (shader-slang#4310) 056a4b9 Small SPIRV emit cleanup around vector element extract. (shader-slang#4309) f83fe55 Make CTS failure report more obvious (shader-slang#4302) 78d34f3 Improve documentation and example formatting consistency (shader-slang#4299) 7c6faf6 Precompute UIntSet from individual capabilities inside generator (shader-slang#4269) 004fe27 Metal compute tests (shader-slang#4292) 72f10a8 Fixed profile string per request in pr#4268 (shader-slang#4297) a709e02 Update capability-generator-main.cpp (shader-slang#4295) d301267 Typo in 06-interfaces-generics.md (shader-slang#4284) fa664d1 Fix build warnings and treat warnings as error on CI (shader-slang#4276) f149052 Remove unnecessary call to __requireComputeDerivative (shader-slang#4283)

ArielG-NV added the pr: non-breaking PRs without breaking changes label Jun 12, 2024

ArielG-NV marked this pull request as draft June 12, 2024 23:53

ArielG-NV added 2 commits June 12, 2024 19:53

Merge branch 'master' into HLSL-SV_InstanceId-not-supported-in-pixel-…

673f64e

…shader-shader-slang#3087

fix compile warnings

6c69fed

jkwak-work reviewed Jun 13, 2024

View reviewed changes

tests/bugs/gh-3087.slang Outdated Show resolved Hide resolved

source/slang/slang-ir-inst-defs.h Outdated Show resolved Hide resolved

ArielG-NV marked this pull request as ready for review June 13, 2024 05:31

csyonghe reviewed Jun 13, 2024

View reviewed changes

ArielG-NV added 3 commits June 13, 2024 09:55

Merge branch 'master' into HLSL-SV_InstanceId-not-supported-in-pixel-…

313e8ec

…shader-shader-slang#3087

address review

25f0b92

remove clone logic

Merge branch 'HLSL-SV_InstanceId-not-supported-in-pixel-shader-shader…

e87b2df

…-slang#3087' of https://github.com/ArielG-NV/slang into HLSL-SV_InstanceId-not-supported-in-pixel-shader-shader-slang#3087

csyonghe reviewed Jun 13, 2024

View reviewed changes

address review

1cb22e4

remove unused function reserve instead of setCount with ShortList

csyonghe approved these changes Jun 13, 2024

View reviewed changes

ArielG-NV added 2 commits June 13, 2024 16:53

Merge branch 'master' into HLSL-SV_InstanceId-not-supported-in-pixel-…

a38ec4a

…shader-shader-slang#3087

Merge branch 'master' into HLSL-SV_InstanceId-not-supported-in-pixel-…

3b6493b

…shader-shader-slang#3087

csyonghe merged commit 2407966 into shader-slang:master Jun 13, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SPIR-V `SV_InstanceId` support in pixel shader #4368

SPIR-V `SV_InstanceId` support in pixel shader #4368

ArielG-NV commented Jun 12, 2024 •

edited

Loading

jkwak-work left a comment

ArielG-NV commented Jun 13, 2024

csyonghe Jun 13, 2024

csyonghe Jun 13, 2024

csyonghe Jun 13, 2024

csyonghe Jun 13, 2024

SPIR-V SV_InstanceId support in pixel shader #4368

SPIR-V SV_InstanceId support in pixel shader #4368

Conversation

ArielG-NV commented Jun 12, 2024 • edited Loading

jkwak-work left a comment

Choose a reason for hiding this comment

ArielG-NV commented Jun 13, 2024

csyonghe Jun 13, 2024

Choose a reason for hiding this comment

csyonghe Jun 13, 2024

Choose a reason for hiding this comment

csyonghe Jun 13, 2024

Choose a reason for hiding this comment

csyonghe Jun 13, 2024

Choose a reason for hiding this comment

SPIR-V `SV_InstanceId` support in pixel shader #4368

SPIR-V `SV_InstanceId` support in pixel shader #4368

ArielG-NV commented Jun 12, 2024 •

edited

Loading